Fluentui: React Hooks for styles and theming

Created on 20 Aug 2020  路  15Comments  路  Source: microsoft/fluentui

Describe the feature that you would like added

Please add React Hooks for styling like in React-JSS or Material-UI libraries, that will allow to access the theme object directly and release the dependencies of 3rd party libraries.

What component or utility would this be added to

Not relevant...

Have you discussed this feature with our team, and if so, who

No

Additional context/screenshots

Something like:

const useStyles = fluentUseStyles((theme: ITheme) => ({
    root: {
        color: theme.palette.themeLighterAlt,
        fontSize: theme.fonts.superLarge,
    }
});

const MyButton: React.FC = () => {
    const classes = useStyles();

    return <DefaultButton classNames={classes.root} />
}
JS Styling Fluent UI react Fixed

All 15 comments

@hraz-msft Thanks for the feature request.

@dzearing FYI who's been driving styling all up for the component library.

At this time we are examining our styling API. I don't have any specific timelines to share at this time, but we'll take this into consideration as we make progress.

Feel free to reach out if you want to play around the API once we have something consumable for feedback.

Marking this as "Won't fix" at this time because are we restructuring the API.

We are going to provide an interim solution to help here. I think it should help unblock your use cases.

You can preview the implementation of useStyles in the codesandbox here:

https://codesandbox.io/s/usestyles-to-build-styling-using-the-fluent-ui-theme-wt0ue?file=/src/App.tsx

However, as @paulgildea mentioned, we're implementing a more performant approach. Rendering styling like this has some performance consequences that aren't terrible but can certainly be abused when done in bulk. We want to do better.

Some of the pitfalls we've seen:

  • Every render must compute the styling
  • You can be clever memoizing the result, but the memo keys must be fast to calculate and not constantly mutate.
  • Memoizing should be done on a more global level rather than instance level, otherwise you're not saving time.

Which means we need to have something that scales a bit better than per instance evaluation of styling.

Love it! Thanks @dzearing glad to hear we could provide some value for v7 here!

Thank you! Looks awesome! I can't wait to see the official package!

If you use the useStyles inside of the component rendering logic, you can use the useMemo hook to memorize the "expensive" calculation of the style, and use the theme object as a dependency. This is a quick way to improve the performance a little bit 馃槃

I will try to improve the implementation tomorrow in CodePen and tag you.

And of course, can't wait to see your final robust solution... 馃殌 (Hope it will be similar)

Totally thought about useMemo. The problem is that it will memo on a per component instance, so each of your buttons will individually run the function, when you want to have more of a global cache which only calculates classes 1 time for a unique theme object.

Hi @dzearing, I've built an example in CodePen that has same interface as my suggestion and it meets all the requirements that you have raised.

Every render must compute the styling

My code computes the styling only when it must. The createUseStyles accepts a style object or a function that depends on the theme.
If the parameter is an object, it computes the styles only once (like in NotDependsOnTheme component) - the components will use the same object for their entire life.
If the parameter is a function (depends on theme like in DependsOnTheme component), it will compute styles only if the theme has changed - the component will use the same object until the theme changes.

(Sample below)

You can be clever memoizing the result, but the memo keys must be fast to calculate and not constantly mutate.

The memorizing is quite simple. It always returns the same object. With the registerOnThemeChangeCallback I listen to theme changes. If the theme has changed, I re-render all the components that has a dependency on the theme. In this case there is almost no overload (logic / comparison) on the render function.

Memoizing should be done on a more global level rather than instance level, otherwise you're not saving time.

In my example, all the components that use the same style will get the same style instance. The computation of the styling happens only once per all the instances of the same component.

It can be seen easily in my example. I added a unique number to the style object that can be seen with:

selectors: {
    "&:after": {
        paddingLeft: 8,
        content: `\"${Math.random()}\"`
    }
}

This random number will be shared across all the instances of the same component. All the NotDependsOnTheme instances have the same number and all the DependsOnTheme instances have the same number.

image

And the number will not change if the components re-render (I count the clicks to change their state which will cause them to re-render).

The number of DependsOnTheme will be changed if Change Theme button is clicked. This button will change the theme primary color to a new random color.

When the theme will be changed, all the DependsOnTheme change immediately and apply the the new color. They will get a new random number (because the style is re-computed), but still all of them will share the same unique number:
image

Note that the number of all the NotDependsOnTheme did not changed while the theme has changed.

I believe that this implementation is a good starting point, and I hope that your final solution will be similar to that... 馃槂

@hraz-msft Thanks, I'll take a look! I put the PR out to get this off the ground, but I will check this out and see if we need to tweak before merging.

I've forked yours here: https://codesandbox.io/s/usestyles-to-build-styling-using-the-fluent-ui-theme-forked-p6o7m?file=/src/useStyles.ts

Overall I like it; this is very similar to a tweak I was discussing with someone yesterday. Create a hook factory which can store the cached results outside the hook. Plus, by requiring that the factory, which executes in file scope and not render scope, is defined at the factory, this creates a forcing factor that the classes can't be dependent component state, which is the root of the performance concern.

We still need to re-evaluate the styles if:

  • The window changes (Child window rendering scenarios)
  • The direction changes (RTL scenarios)
  • The theme changes (Scoped theming scenarios)

So, I've changed it to create a map graph of these things which point to the resolved set. This is cached inside the factory but outside the resulting hook. I've also reverted away from loadTheme/getTheme and used Customizer (to be replaced with ThemeProvider in v8) which relies on global state rather than contextual values.

This is very similar to our getClassNames functions, except no style props dependencies. I think this has a lot of goodness.

There are still a few more needs here:

  • A reset function to reset the cache for testing
  • Handle the no-window SSR scenarios well

This is very nice! But your fork does not support the loadTheme function. Since the factory doesn't know that the theme has changed and won't re-render the component automatically.

Is it OK? Will the loadTheme be deprecated? All the docs say to use loadTheme in order to set the app theme like in here: https://developer.microsoft.com/en-us/fluentui#/controls/web/themes

The entire color palette of the controls is themeable. We provide a set of sensible defaults, but you can override all colors individually. To do this, you must call loadTheme() with a custom theme object at app startup.

Thanks for calling out; I will double check here that we don't break this.

Yes, our plan is to phase out global theme setting in favor of context. We started down this path when context was finalized in React using the Customizer, but still supported both. Global objects are a very bad problem for partners who may be sharing code on the same page from multiple sources which want to apply theme overrides.

I have updated the PR and have named this makeStyles, as I've seen this naming convention for hook factories in other open source projects, and have been trying to standardize towards it (shorter than createUseStyles, though I've teetered on that as well in other cases.)

Hi @dzearing, I saw that the PR was merged! 馃槃

When it will be released as npm package?

Overnight at 3am.

:tada:This issue was addressed in #14641, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

@hraz-msft

Example usage here:

https://codesandbox.io/s/fluent-ui-themeprovider-and-makestyles-example-d84my

This was released in the @fluentui/react-theme-provider package. Should work with existing theme via loadTheme or with Customizer provided themes. Please let us know if you run into any issues (feel free to reach out on Teams.)

Thanks @dzearing, I will integrate it to my project tomorrow morning! Thank you for the update and the rapid development!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mattcoxonline picture mattcoxonline  路  3Comments

carruthe picture carruthe  路  3Comments

justinwilaby picture justinwilaby  路  3Comments

prashkan picture prashkan  路  3Comments

nekoya picture nekoya  路  3Comments