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

Add a way to provide custom styles based on design system luminance in web components #3973

Closed
eljefe223 opened this issue Sep 25, 2020 · 8 comments
Labels
area:fast-foundation Pertains to fast-foundation feature A new feature status:needs-investigation Needs additional investigation

Comments

@eljefe223
Copy link
Contributor

What feature are you requesting?
In our react package @microsoft/fast-components-msft we had a isDarkMode() method that would check the current designSystem luminance and return either true or false. This allowed us to return 2 separate color values, classes etc. based on whether the parents designSystem was in dark mode or not.

What would be the benefit of adding this feature?
One use case would be some kind of finance UI, We may want to make some text red and some text green. It may be difficult to pick one red value and one green value that would be accessible across every background.

What solution would be ideal for you?
Should we add a similar method isDarkMode to our web components package? OR I would expect some kind of wrapper that makes it easy to update the accent-color that automatically updates the accentPalette.

What alternatives have you considered?
We could wrap those special elements in their own design system and update the accent color... But that seems overly cumbersome and heavy handed.

@bheston @nicholasrice @chrisdholt

@eljefe223 eljefe223 added the status:triage New Issue - needs triage label Sep 25, 2020
@chrisdholt chrisdholt added area:fast-foundation Pertains to fast-foundation feature A new feature status:planned Work is planned and removed status:triage New Issue - needs triage labels Sep 25, 2020
@chrisdholt
Copy link
Member

@bheston, thoughts on this? Any new guidance or is the previous function equally applicable and desired here?

@bheston
Copy link
Collaborator

bheston commented Sep 25, 2020

We need to separate out two specific concerns:

  1. Adjusting the design in a specific custom way depending on the luminance of the context
  2. Making it easier to change color palettes and add support for multiple palettes

Based on the 'benefits' question above, this issue relates to item 2. The isDarkMode switch was never intended for specifying light and dark mode colors used for text in this scenario. It has probably been used in that way, but as an unfortunate fallback for complexity and apparent overhead in overriding the design system and recreating a palette. The intent was more as a base for recipes like neutral focus that switch direction to find the right color that increases contrast to the context. That's because hard-coding red or green values doesn't ensure they're contrast compliant or correct for potentially different backgrounds.

OR I would expect some kind of wrapper that makes it easy to update the accent-color that automatically updates the accentPalette.

I was just about to write up a feature request for this. Now that we have change notifiers it seems that the design system provider could handle accentBaseColorChanged and always recreate the palette. There's no case I believe we ever want those to be unrelated.

That would make it easier to do something like <design-system-provider accentBaseColor="red"> or <design-system-provider accentBaseColor="green">, but you'd still have to switch those out or apply them very localized for a single piece of text. A stronger design would be to be able to apply multiple separate palettes like positive and negative and then apply a form of the accent recipes that reference the desired palette. This feels like it might fit into #3833. We have discussed this for common colors like error and warning before.

Regarding the concern in item 1, I emphasized the 'specific' nature because I think this applies to something like showing a sun or a moon icon depending on the mode. In app-level consumer code I would expect a function like this to be used very minimally.

This question has come up recently regarding opacity overlay as well, looking to use one opacity in light mode and another in dark, but that needs to be addressed at the design level to understand the behavior or it breaks down when looking at personalization like a custom neutral palette and adjusted baseLayerLuminance toward the middle. I do not recommend using isDarkMode in this case either.

@chrisdholt chrisdholt added status:needs-investigation Needs additional investigation and removed status:planned Work is planned labels Sep 25, 2020
@nicholasrice
Copy link
Contributor

nicholasrice commented Sep 28, 2020

It's worth noting that @microsoft/fast-components does export isDarkMode, but I think @bheston makes some good points about when and how it should be used. Relating the base and accent color directly will be part of #3833 as is mentioned above.

Based on the above, is there any action we need to take on this now or can this be closed?

@chrisdholt
Copy link
Member

The wrinkle here with @microsoft/fast-components exporting isDarkMode is that it's technically deprecated and has been moved to Fluent UI. I think that's where the request here came from.

@nicholasrice
Copy link
Contributor

The function definition does not contain a deprecation doc tag https://github.com/microsoft/fast/blob/master/packages/web-components/fast-components/src/color/palette.ts#L160. Where do you see that it is deprecated? The function is exported from @microsoft/fast-components: it hasn't been moved to Fluent UI.

@chrisdholt
Copy link
Member

chrisdholt commented Sep 29, 2020

@nicholasrice - I misread...I thought your initial post referenced @microsoft/fast-components-msft. In this case, @eljefe223 - it seems like this would be something missing in the FluentUI repo (in the case someone didn't want to take a dep on the fast components as well). In that case, should we add this, or would our preference be to wait for #3833?

@nicholasrice
Copy link
Contributor

Gotcha, no worries. Yea FluentUI uses @microsoft/fast-components-styles-msft, but that package exports isDarkMode as well

@chrisdholt
Copy link
Member

Closing this given #5853

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:fast-foundation Pertains to fast-foundation feature A new feature status:needs-investigation Needs additional investigation
Projects
None yet
Development

No branches or pull requests

4 participants