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

feat(labs): Theming (react) #272

Merged
merged 53 commits into from
Dec 6, 2019
Merged

feat(labs): Theming (react) #272

merged 53 commits into from
Dec 6, 2019

Conversation

anicholls
Copy link
Contributor

@anicholls anicholls commented Oct 22, 2019

Summary

Spent last week working through how theming will work in Canvas Kit. This is a functional POC, but still needs some work. It also only adds theming to one existing component as a test.

Note 1: This does not have any breaking changes 🎉
Note 2: Since I've put the theming work in labs (to allow for updates to the API/theme object), we now need to depend on @workday/canvas-kit-labs-react-core if we want to start adding theming support to our components.
Note 3: This does not address CSS theming. The implementation of that is still up in the air.

Implementation

This adds a new CanvasProvider (which includes an InputProvider) that provides a theme using Emotion's ThemeProvider. You can pass a custom theme using the createCanvasTheme function:

const theme = createCanvasTheme({
  palette: {
    primary: {
      main: 'orange'
    },
  }
});
<CanvasProvider theme={theme}>
  // Components
</CanvasProvider>

createCanvasTheme will expand a partial theme into a full one based on the default theme. If you only pass a main shade, all other shades will be generated automatically based on that.

CanvasTheme also contains breakpoints (zero, s, m, l, xl) that can be accessed with the context. It also includes functions that return media queries based on the breakpoints (e.g. theme.breakpoints.up('m') will return '@media (min-width:960px)' by default).

Finally, since this is all based on React Contexts, we have an issue if anyone is not able to use contexts. This can be true for teams with multiple small React trees. To handle this, CanvasProvider can accept the setThemeGlobal prop. This sets the theme on window.wdCanvas.theme. Whenever we access the theme, we use the useTheme hook, which first looks for a context theme, then for a window theme, before falling back to the default. I added an override/wrapper to Emotion's styled function which calls useTheme behind the scenes.

Checklist

  • branch has been rebased on the latest master commit
  • tests are changed or added
  • yarn test passes
  • all (dev)dependencies that the module needs is added to its package.json
  • code has been documented and, if applicable, usage described in README.md
  • module has been added to canvas-kit-react and/or canvas-kit-css universal modules, if
    applicable
  • design approved final implementation
  • a11y approved final implementation
  • code adheres to the API & Pattern guidelines


return (
<ThemeProvider theme={theme}>
<InputProvider>{children}</InputProvider>
Copy link
Contributor Author

@anicholls anicholls Oct 22, 2019

Choose a reason for hiding this comment

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

CanvasProvider would provide both the theme and the input type to avoid multiple HOCs.

It would be cool if this automatically loaded the fonts module and injected the global style if a flag for that was passed as well.

backgroundColor:
error === ErrorType.Error ? theme.palette.error.main : theme.palette.alert.main,
color: error === ErrorType.Error ? theme.palette.error.contrast : colors.blackPepper400,
'&:hover': {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I realized we don't have active states for our Banner component. The color/contrast gets pretty terrible based on our current system, so we should consider this.

@anicholls anicholls added this to the v4.0.0 milestone Oct 22, 2019
@anicholls anicholls force-pushed the theming branch 2 times, most recently from 3d4a544 to a57b54c Compare October 24, 2019 17:54
@anicholls anicholls force-pushed the theming branch 2 times, most recently from 49dba6f to d7b081f Compare October 24, 2019 22:29
@anicholls anicholls changed the title WIP - feat: Theming POC WIP - feat(labs): Theming Oct 25, 2019
@anicholls anicholls force-pushed the theming branch 5 times, most recently from d6f8db3 to e4eb9db Compare October 29, 2019 20:58
@anicholls anicholls changed the title WIP - feat(labs): Theming feat(labs): Theming Nov 1, 2019
@anicholls
Copy link
Contributor Author

Update on the conflict with @storybook/addon-docs. I created our own theme context/provider and that still didn't resolve the issue. After a lot of fun git bisecting, I ended up needing to enforce emotion-theming@10.0.10 to keep the error away. I tried updating to the latest version of @storybook/addon-docs and that did not fix it.

@lychyi lychyi added the p:1 label Dec 5, 2019
@anicholls anicholls merged commit 929de08 into Workday:master Dec 6, 2019
@anicholls anicholls deleted the theming branch December 6, 2019 23:06
@anicholls anicholls removed this from the v4.0.0 milestone Mar 31, 2020
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.

4 participants