-
Notifications
You must be signed in to change notification settings - Fork 671
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
Support referencing theme values when defining other theme properties #961
Comments
Instead of implementing a
This is inspired by how Modulz approaches surfacing theme values in their visual editor: We'd sacrifice the ability to define neat scales like On the other hand, I've come to like how Modulz places emphasis on the actual value, and handles the theme scale aspect under the hood. |
Alternative idea. Possible to typecheck and autocomplete for nice DX. It's close to current derived properties in style objects, so it doesn't introduce any special new syntax. const theme = {
colors: {
primary: '#0022ff",
success: t => t.colors.primary,
secondary: t => lighten(0.2, invert(t.colors.primary)),
}
} |
Yep, I agree. My implementation of import {themeGet} from 'styled-system'; // I don't think there's an impl in theme-ui
const $ref = path => theme => get(theme, path);
colors = {
primary: '#0022ff',
alias1: theme => theme.colors.primary,
alias2: theme => get(theme, 'colors.primary'),
alias3: themeGet('colors.primary'),
alias4: $ref('colors.primary'),
} all the aliases would all be functionally (ha) equivalent, based on the idea of allowing functions in themes to be called against the theme value in context. The reason I'm partial to string-based paths is that it constrains more than a generic function, so it seems like we'd be better able to statically analyze the path at build time for optimizations. But a function resolver would work just as well at runtime. |
I like re-using the existing functional syntax, but wrapping that into something like |
I may be a bit allergic to string-based paths, because they can't be typechecked, so no autocomplete possible. Deriving more scale values from a minimal set of scale values with full power of babel-macro-optimized build-time JavaScript, seems very attractive to me. Maybe too much 😅 This might be relevant to leave here as a note: |
Just wondering if there was any real disadvantage to just doing something like this: const baseColors = {
black: '#333',
green: {
500: '#00A846',
},
};
const theme = {
colors: {
...baseColors,
text: baseColors.black,
brand: baseColors.green.500,
},
}; |
@dburles I think that's the current recommended way to handle it, but it wouldn't work for JSON-serializable formats, and I think it's been a feature request for years (in Styled System, etc.), so I'm open to exploring a better API for this sort of thing |
Not sure I understand completely the intent behind JSON serialising themes, definitely interested if there’s somewhere I can find more context behind it. |
Here's another use-case for this: I want darker shadows in my Because I can't reference defined color values inside |
I have the same use case as @NickChristensen. I think it'd be nice for the docs to make clear that the theme specification object does not accept functional values, and I’d be happy to send a PR to this effect. Edit in case anyone needs a workaround for this use case. I'm using // In src/gatsby-plugin-theme-ui/index.js
export const defaultShadow = theme => `10px 10px 0 0 ${theme.colors.primary}` And in your component // Import the shadow
import { defaultShadow } from '../gatsby-plugin-theme-ui'
// Later, when you need to use it:
const MyComponent = () => (
<div sx={{
width: 100,
height: 100,
boxShadow: defaultShadow
}}>A Box</div>
) 😄 |
Hi guys, Great conversation, just came across and put together a babel plugin, where we can enforce theme files to be called ie Can you please check if it would be an ok direction for string-based paths (the existing function shortcuts to access theme props already exist). The project is a very simple conversation starter, just to check the overall expectations. Please also add more test cases for required transformations. The examples are only for colors but it should work for any keys inside the theme. From this theme definition: It would transform to the following output: Of note:
|
Is your feature request related to a problem? Please describe.
When defining theme properties, I sometimes have to copy the same value to two different properties. One common example is when I'm defining raw token names and use-case token names for the same color value:
Describe the solution you'd like
I'd like the ability to reference theme values when defining other theme properties. E.g. (syntax also is just for example, subject to change based on discussion):
I have some use cases / restrictions for a theme reference beyond just "matching values":
When a referent value changes, any reference also changes.
E.g. if I make
colors.black
#000000
, I would expectcolors.text
to also point at#000000
I would expect theme-ui to catch any circular references and warn/exit. Either at build time or runtime is fine, but build time is preferred.
E.g.
text: $ref('colors.black'), black: $ref('colors.text')
should not enter an infinite loop.It could be nice if theme-ui could optimize away any overhead of a $ref at build time, if it detects that's possible.
E.g. use
--theme-ui-colors-black
as the CSS variable for bothcolors.text
andcolors.black
.However, this may not always be what we want, especially if downstream consumers expect they can override
--theme-ui-colors-text
outside ofsx
(maybe the team uses CSS modules.)Describe alternatives you've considered
We could treat theme.js as as compile target and use something like Dhall to write non-duplicated theme rules. That's a lot of overhead to bring into a project if you're not already using Dhall.
I actually have a userland implementation of
$ref
that is effectivelythemeGet
with a__$ref__ = true
property, and have a custom function that keeps applying{css} from @theme-ui/css
until no returned value is a$ref
. This works, but it's hacky. It'd be nice to have this supported by theme-ui.This issue is meant to track "Idea: Provide some solution for self-referencing scales in other scale definitions" in #832
The text was updated successfully, but these errors were encountered: