Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enhancement(#2351):Adopt uPortal CSS Variables for Web Components to Simplify Skin Management #2860

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

555vedant
Copy link
Contributor

currently, web components require individual CSS variiables to be defined separately, which adds complexity when integrating with uPortal skins. To streamline styling and ensure consistency, uPortal CSS variables (like those used for portlets) should be directly exposed and defined. This will allow web components to inherit styles such as header text color, content background color, and more, from the uPortal skin without requiring redundant definitions. The solution involves defining key CSS variables in the :root scope that web components can automatically use, improving maintainability and reducing duplication

@555vedant 555vedant changed the title Adopt uPortal CSS Variables for Web Components to Simplify Skin Management enhancement(#2351):Adopt uPortal CSS Variables for Web Components to Simplify Skin Management Oct 3, 2024
@555vedant
Copy link
Contributor Author

555vedant commented Oct 5, 2024

@jonathanmtran sir please tell me is there any improvement or suggestion for this PR because from 10th onwards i have MIDSEM exam of college.

@jonathanmtran jonathanmtran linked an issue Oct 5, 2024 that may be closed by this pull request
Copy link
Contributor

@jonathanmtran jonathanmtran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the enhancement request two examples were given.

:root {
   --portlet-header-text-color: #000000
   --portlet-content-bg-color: #d0d0d0
...
}

--portlet-header-text-color would correspond to @portlet-titlebar-link-color found in https://github.com/uPortal-Project/uPortal/blob/HEAD/uPortal-webapp/src/main/webapp/media/skins/respondr/common/less/content.less#L165

    > a {
      color: @portlet-titlebar-link-color;

      &:hover {
        color: darken(@portlet-titlebar-link-color, 20%);
      }
    }

The end result would be an appropriate modification that would include something like the following:
NOTE: I did not try this so I am not sure if this would even compile/work

:root {
  --uP-portlet-header-text-color: @portlet-titlebar-link-color;
}

@555vedant
Copy link
Contributor Author

@jonathanmtran is it fine ?

@555vedant
Copy link
Contributor Author

@jonathanmtran PR ready to merge ??

@jonathanmtran
Copy link
Contributor

I do not feel that this PR is ready to merge.

  • The modifications were made to autosuggest.css which is NOT a file that is related to what I feel the enhancement would be
  • The UNTESTED fragment I provided contains a LESS variable. Since the modified file is not a .less file, it would never be compiled into CSS at build time. Therefore, the changes need to be made to the appropriate file, pre-existing or new

@555vedant
Copy link
Contributor Author

@jonathanmtran i also noticed the same problem with less variables we cant do without less , now whats the solution for this PR ??

@jonathanmtran
Copy link
Contributor

jonathanmtran commented Oct 11, 2024

I have already described my guess as to what an implementation could look like.

In order to make meaningful progress on implementing this enhancement and if you have not set up an instance of uPortal: you should set up an instance of uPortal using uPortal-start and work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants