microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
922 stars 285 forks source link

[Feature] theming infrastructure - export the applyColorScheme function for use by application developers #3085

Open HarminderSethi opened 4 months ago

HarminderSethi commented 4 months ago

Feature Request

Currently there is no way to change the color value of classess related to --accent-fill- under people picker component. similar to other classess available for customization please allow to customize the value for --accent-fill- as well

Describe the solution you'd like

Allow to set the style for --accent-fill-* classess for People Picker

gavinbarron commented 4 months ago

Can you please describe the scenario where you're looking to set the values of these css properties?

The fills and numerous other css properties are supplied by the fluentui web components and are algorithmically generated from some seed colors, setting the underlying css properties, while possible is not a recommended approach.

HarminderSethi commented 4 months ago

@gavinbarron when PeoplePicker is active, border bottom color on ::after property is set using --accent-fill-rest which seems to be hardcoded and does not seem to be coming from fluent ui web components theme. PFA screenshot. In my case I need to set it black to match with the project theme.

image

gavinbarron commented 4 months ago

Thanks for the feed back @HarminderSethi.

The color is coming from the fluentui web components theme that is used by the components. The content in the flyout is at a higher level of elevation via a fluent-card and fluent ui web components is injecting the newly calculated set of colors based on the theme from the parent element. While we could up a specific css variable to try to create an override for you I feel that this might simply be a huge game of whack-a-mole due to the multiple places that these css properties can be used, and the way that the color values are injected at multiple places in the (shadow)DOM tree.

I think that ability to pass in your own custom theme settings from the root of your application would probably be more useful and flexible.

We currently expose an applyTheme function that takes an element and a string. How about if we were to expose a more powerful version of this:

interface ColorScheme {
  /**
   * Hex color string for accent base color
   *
   * @type {string}
   */
  accentBaseColor: string;

  /**
   * Hex color string for neutral base color
   *
   * @type {string}
   */
  neutralBaseColor: string;

  /**
   * Base layer luminance for the theme
   * in the range of 0 to 1
   *
   * @type {number}
   */
  baseLayerLuminance: number;

  /**
   * Optional function to override design tokens
   */
  designTokenOverrides?: (element: HTMLElement) => void;
}

applyColorScheme = (settings: ColorScheme, element: HTMLElement = document.body) => void

You'd may need to provide some implementation for designTokenOverrides, but you'd have as much power to set the css props as we can give you.

Sample ColorScheme:

import {
  accentFillActive,
  accentFillFocus,
  accentFillHover,
  accentFillRest,
  StandardLuminance,
  SwatchRGB
} from '@fluentui/web-components';
// @microsoft/fast-colors is a transitive dependency of @fluentui/web-components, no need to explicitly add it to package.json
import { parseColorHexRGB } from '@microsoft/fast-colors';

const darkTheme: ColorScheme ={
  accentBaseColor: '#479ef5',
  neutralBaseColor: '#adadad',
  baseLayerLuminance: StandardLuminance.DarkMode,
  designTokenOverrides: element => {
    accentFillRest.setValueFor(element, SwatchRGB.from(parseColorHexRGB('#115ea3')));
    accentFillHover.setValueFor(element, SwatchRGB.from(parseColorHexRGB('#0f6cbd')));
    accentFillActive.setValueFor(element, SwatchRGB.from(parseColorHexRGB('#0c3b5e')));
    accentFillFocus.setValueFor(element, SwatchRGB.from(parseColorHexRGB('#0f548c')));
  }
};

applyColorScheme(darkTheme);

I think that this solution will give you all the tools you need to solve this problem in a more maintainable way. Do you think that this solution could work for your scenario?

HarminderSethi commented 4 months ago

@gavinbarron Yes, it should work