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
Utility class mapping #33
Comments
I feel like we could also use a shorthand for those component definitions that looks something like this, which would give us the function createPrimerComponent(Element, mapProps) {
const Component = props => <Element {...mapProps(props)} />
Component.withComponent = other => createPrimerComponent(other, mapProps)
return Component
}
const Box = createPrimerComponent('div', boxMap)
const Text = createPrimerComponent('span', textMap)
const Heading = Text.withComponent('h1')
Heading.h1 = Heading
Heading.h2 = Heading.withComponent('h2')
// etc. |
We chatted about this IRL this morning and decided that as a first step we'll just include all the Primer utilities in the utility map, and then later on improve the mapping to split utilities into different groups and possibly restrict certain utilities from being used on certain components ❇️ |
One thing I suggested was using decorators to return modified versions of components, e.g. import {whitelist} from './utils/decorators'
@whitelist('id', 'hidden', 'aria-*')
export default class Component {
render() {
// the whitelist() decorator would modify the props by moving id, hidden,
// and aria-* props into a "whitelisted" object prop
const {x, whitelisted, children} = this.props
return (
<div className={`util-${x}`} {...whitelisted}>
{children}
</div>
)
}
} For functional components it would look something like this: import {whitelist} from './utils/decorators'
const Component = ({x, whitelisted, children}) => (
<div className={`util-${x}`} {...whitelisted}>
{children}
</div>
)
export default whitelist('id', 'hidden', 'aria-*')(Component) One benefit of this approach is that the decorator function could return a component class with |
It seems like we're not ready for this level of unification across all components, so I'm going to close this as OBE for now. |
I've been using the Primer mapping from
system-classnames
in #32, but after looking into Text and Heading components I think we should consider more granular mappings for each, e.g.Then Box, Text, and Heading would each use them like:
Does this seem like the right way to go about it?
The text was updated successfully, but these errors were encountered: