-
Notifications
You must be signed in to change notification settings - Fork 129
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
[RFC] Add abstraction layer to theme #871
Comments
Okay, I have a few concerns with this addition:
That said, I'm not sure I see the benefits being greater than the risks/costs of adding this abstraction layer (I don't know about other contexts, but I don't see conversations about implementing dark mode in the near future). |
True. The abstraction layer would be a reflection of those design guidelines and thus would simplify communication between designers and developers.
The design spec should include the semantic colors that should be used. That's not always the case, so there's some truth to this 👍
That's my concern as well. Fortunately, it could be done gradually.
While there is no concrete timeline for it yet, this is very much on the roadmap and preliminary research and work is already in progress. |
Sorry, closed it accidentaly 😂 Hmm, if dark mode is on the roadmap and we're already working on it this might bring more benefits than I initially thought. |
My idea for a gradual migration looks as follows:
|
I think this is ultimately what have to do to have a proper "theme", like Every component in a design system needs to have a schema of themable CSS properties for every one of its variants. The styled system docs describe how a theme API using variants may look and why (pretty much what @connor-baer shared). With TypeScript, I think it would make sense for the components to export the types for their variant themes so they can be used in the application theme. Of course, what a change like this requires is that design considers the themability of every component beforehand and provides these specifications to developers. Regarding the API, at the moment -- unless this has changed -- I think every variant has it's own style function. I think it would make sense to keep that to cleanly separate the different variants. Using your example, I would make the following proposal. // This proposed variant style
const variantStyles = ({ theme, variant }) => css`
color: ${theme.button[variant].color};
background: ${theme.button[variant].backgroundColor};
// ...
`;
const Button = styled.button(variantStyles, /* ... */);
// becomes:
const primaryStyles = ({ theme, variant }) => variant === 'primary' && css`
color: ${theme.button[variant].color};
background: ${theme.button[variant].backgroundColor};
// ...
`;
const Button = styled.button(primaryStyles, /* ... */); I think this would make it easier to track the various variants, as their style schemas are likely to not overlap perfectly (i.e., you would have one function for the shared themable CSS properties and one for each variant with the variant specific styles). |
Ah, also I think this needs to happen outside of design-tokens. The themes are component-specific and therefore belong with the component library. The tokens are used as input to the component theme. You get a theme hierarchy where the tokens are the outer theme and the component themes are the inner theme. You can then nest these with multiple theme providers to prevent unnecessary re-renders, I think. The emotion docs have an example of this. import * as React from 'react'
import styled from '@emotion/styled'
import { ThemeProvider, withTheme } from '@emotion/react'
// object-style theme
const theme = {
backgroundColor: 'green',
color: 'red'
}
// function-style theme; note that if multiple <ThemeProvider> are used,
// the parent theme will be passed as a function argument
const adjustedTheme = ancestorTheme => ({ ...ancestorTheme, color: 'blue' })
class Container extends React.Component {
render() {
return (
<ThemeProvider theme={theme}>
<ThemeProvider theme={adjustedTheme}>
<Text>Boom shaka laka!</Text>
</ThemeProvider>
</ThemeProvider>
)
}
} In our use case, I think things get a bit more complicated because the values in the inner component themes depend on the design token theme. For example: const adjustedTheme = ancestorTheme => ({ ...ancestorTheme, color: 'blue' }); would become const adjustedTheme = tokens => ({ ...tokens, color: tokens.colors.b500 }); On the implementation side, this would then lead you to every component exporting a export const createButtonTheme = tokens => ({
primary: {
backgroundColor: tokens.colors.b500,
},
secondary: {
backgroundColor: tokens.colors.n300,
},
}) These component theme functions can be combined in a global theme function in Circuit UI import { createTheme as createButtonTheme } from 'components/Button';
import { createTheme as createTextTheme } from 'components/Text';
export const createCircuitTheme = tokens => ({
...tokens,
button: createButtonTheme(tokens),
text: createTextTheme(tokens),
}) When applying this to the Emotion example you would get the following in an application. import styled from '@emotion/styled'
import { ThemeProvider } from '@emotion/react'
import { light } from '@sumup/design-tokens'
import { createTheme } from '@sumup/circuit-ui'
export const App = ({ children }) => (
<ThemeProvider theme={light}>
<ThemeProvider theme={createTheme}>
{children}
</ThemeProvider>
</ThemeProvider>
)
}
} Yes this is a lot of JS at runtime. Potentially developing a way to create complete application themes in userland at build time would be preferable. In an alternative API, the component themes could use strings for the values of CSS properties. For example, the const buttonTheme = {
primary: {
backgroundColor: 'b500',
},
secondary: {
backgroundColor: 'n300',
},
} You can then adjust usage in the component styles accordingly: const primaryStyles = ({ theme, variant }) => variant === 'primary' && css`
background-color: ${theme.colors[theme.button.primary.backgroundColor]};
` This would still be type safe, but I think it's butt ugly. |
I think this is still missing something. You need a way to tell the import styled from '@emotion/styled'
import { ThemeProvider } from '@emotion/react'
import { tokens } from '@sumup/design-tokens'
import { createTheme } from '@sumup/circuit-ui'
export const App = ({ children }) => (
<ThemeProvider theme={tokens}>
<ThemeProvider theme={createTheme('light')}>
{children}
</ThemeProvider>
</ThemeProvider>
)
}
}
|
Hey 👋 This definitely sounds like a nice improvement to me. 🙂 The specific values allow for greater customization and themeing of the components. Want to keep blue as the primary color, but turn all primary buttons orange? That's possible now! Decoupling of the Theme colours and the components/elements colours sounds great. 💯 👍 @connor-baer maybe we can have some nice scripts that could help with the breaking changes? 🙂 |
We'll investigate how we can reduce the work for the migration, but I'm afraid that fully automating it is not possible. I expect that each generic theme color will be mapped to multiple semantic colors and it will require a human with context to make the replacements. |
@mcntsh made a great point offline: Using CSS variables instead of static strings could be a major boost for performance as it would shift the responsibility for updating the component styles when switching between themes to the browser. In theory, we could move away from a JSON theme entirely (except for media query values) and use CSS variables directly. We would lose out on type checking and autocomplete though. Since some legacy browsers don't support CSS variables, this is blocked by #790. |
An update here: we're moving forward with the semantic abstraction for color tokens, albeit with a slightly different shape proposal. Semantic theme shapeWe're planning on adding new, semantic color values to
There's more context and discussion in this (internal) RFC. The main difference with what has been discussed in this thread so far is that semantic tokens are not component-aware, they're "global" semantic colors. This makes the theme's implementation simpler, which also means that it makes creating a custom theme easier. Theming a single component is still possible by wrapping it in Emotion's POCA POC branch was started at #1447. It contains a subset of the necessary changes inside Circuit UI, as well as a live example in Storybook. Migration strategy
|
Completed in #1880. |
Summarize the feature
I propose to add an abstraction layer on top of the current design tokens. Instead of referencing e.g. the base colors (
p500
) directly, components would use semantically named values (buttonColor
).Basic example
It would be used like so:
Motivation
This approach has multiple benefits:
Migration
This is a major breaking change and thus will need to be rolled out in stages:
The text was updated successfully, but these errors were encountered: