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

Better support for theming using CSS global vars #5105

Closed
Tracked by #5413
mcoker opened this issue Sep 20, 2022 · 3 comments
Closed
Tracked by #5413

Better support for theming using CSS global vars #5105

mcoker opened this issue Sep 20, 2022 · 3 comments
Labels
breaking change 💥 DevX enhances the consumer developer experience but does not change the experience for the end-user Epic PF5

Comments

@mcoker
Copy link
Contributor

mcoker commented Sep 20, 2022

Taking the learnings from dark theme, let's examine the current global variables and see where we can make improvements specifically for theming. A couple of examples are:

As close we can, a user should be able to go in and tweak a global variable to theme some concept and it change - to the best of our ability - that thing. If we use --pf-global--disabled-color--400 for text, backgrounds, and borders, and a user wants to theme --pf-global--disabled-color--400 for use in one context, it's probably going to change a bunch of stuff they don't want in other contexts.

As part of this, I think we should be able to document the intended use of every global variable so there is no confusion as to when to use what var for what purpose, and if a user wants to theme their app, that documentation can be their reference guide.

Then we should audit each component and make sure they are referencing the correct global vars, which can be a follow up task/epic.

cc @mceledonia @mmenestr @mcarrano


Other ideas:

Status colors

  • The pf-color-name-50 colors we use as the background for statuses should be global vars
  • Make it clear what --100, --200, etc are for, or give those more meaningful names as text, icons, backgrounds, hover/focus backgrounds, etc
  • We should probably have a generic scale for the existing status colors that are not tied to status names, as they don't always have to be used for a particular status - labels and banners as an example - Colors - PF colors for severity levels patternfly-design#1219
    • Would we want to have both status names and the generic scale? I would say so:
      • Users can theme their "red" (but not danger) component things uniquely from their "danger" component things
      • We could probably get rid of the "default" status and make it part of the generic scale? Or do we want a "default" status that can be themed separately from a "cyan" status? We probably need to rename that regardless since it's causing a namespace conflict in react components - Potential namespace conflict with "default" in status prop enums #5175

Icons

  • Sort out icon color and icon font size global vars - the color should likely be whatever the text color is by default, as in it should probably inherit it so they stay in sync, but maintain separate vars for theming.

Spacers

  • Provide more meaningful description for the form spacer
  • Introduce spacers for shared concepts. eg,
    • helper text
    • icon + text
    • document blocks/gaps for things like stacked components or content like paragraph + paragraph + list + heading + paragraph (seen in the content component)
    • gutters and insets - both block (vertical) and inline (horiz)
  • Consider logical property names where appropriate - like the form spacer. It's block (vertical) padding for a form element. Or text/content's heading - there is a unique block-start and block-end
    --pf-c-content--h1--MarginTop: var(--pf-global--spacer--lg);
    --pf-c-content--h1--MarginBottom: var(--pf-global--spacer--sm);
  • Consider global var for form element height (it's currently 36px) - or look at removing this,
    --pf-c-form-control--Height: calc(var(--pf-c-form-control--FontSize) * var(--pf-c-form-control--LineHeight) + var(--pf-c-form-control--BorderWidth) * 2 + var(--pf-c-form-control--PaddingTop) + var(--pf-c-form-control--PaddingBottom)); // Needed for various browsers that compute height of form elements differently

Borders

  • Have more robust and well-documented border and disabled colors. We often get a color from design to use for, say, a disabled box's border, and we just use whatever global var is available - that may be a text color, a global disabled color, or one of the border colors.

Box shadows

  • Consider variations for shadows that are inset vs go outside of their container.

Backgrounds

Colors

  • HSLa - more intuitive, better for theming and generating palettes in general, support for the alpha channel*
    • * we can add support for that now with hex colors using #RRGGBBAA, which I think we should add if we don't use HSLa with this update.
@mcoker mcoker added Feature end user feature that requires design to define this, i.e. changes or augments the user experience. needs triage Needs to be reviewed and prioritized breaking change 💥 labels Sep 20, 2022
@nicolethoen nicolethoen removed the needs triage Needs to be reviewed and prioritized label Sep 23, 2022
@nicolethoen nicolethoen added this to the Breaking Changes milestone Sep 23, 2022
@mcarrano mcarrano modified the milestones: Breaking Changes, 2022.14 Sep 28, 2022
@mcarrano mcarrano added this to 2022.15 November 18 2022 in PatternFly Feature Roadmap Sep 28, 2022
@mcarrano mcarrano added the PF5 label Sep 28, 2022
@mcarrano mcarrano moved this from 2022.15 November 18 2022 to 2022.16 (No release) in PatternFly Feature Roadmap Sep 28, 2022
@mcarrano mcarrano moved this from 2022.16 (No release) to 2023.01 PatternFly 5 - Date TBD in PatternFly Feature Roadmap Sep 30, 2022
@mcarrano mcarrano removed this from 2023.01 PatternFly 5 - Date TBD in PatternFly Feature Roadmap Sep 30, 2022
@mcoker mcoker self-assigned this Oct 4, 2022
@mcarrano mcarrano modified the milestones: 2022.14, 2022.15 Oct 21, 2022
@mcarrano mcarrano added the Epic label Oct 25, 2022
@nicolethoen nicolethoen modified the milestones: 2022.15, 2022.16 Oct 25, 2022
@mcoker mcoker modified the milestones: 2022.16, 2023.01 Dec 13, 2022
@mcarrano mcarrano modified the milestones: 2023.01, 2023.02 Jan 20, 2023
@nicolethoen nicolethoen modified the milestones: 2023.02, 2023.03 Feb 15, 2023
@nicolethoen nicolethoen modified the milestones: 2023.03, 2023.04 Mar 3, 2023
@evwilkin evwilkin modified the milestones: 2023.04, Breaking Changes Mar 14, 2023
@mcoker mcoker removed their assignment Jun 8, 2023
@mcarrano mcarrano added DevX enhances the consumer developer experience but does not change the experience for the end-user and removed Feature end user feature that requires design to define this, i.e. changes or augments the user experience. labels Jun 12, 2023
@mcarrano
Copy link
Member

@mcoker @srambach Just doing some housekeeping. Do we need to keep this issue open?

@srambach
Copy link
Member

Looks to me like it could be closed and we'll open more specific issues around tokens and theming, but I'll defer to @mcoker - there's a lot of good info here.

@mcoker
Copy link
Contributor Author

mcoker commented Jun 14, 2023

Agree, some of this is already done in the penta work, but everything I see here is already being considered. Closing for now and we can reference if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change 💥 DevX enhances the consumer developer experience but does not change the experience for the end-user Epic PF5
Projects
Archived in project
Development

No branches or pull requests

5 participants