-
Notifications
You must be signed in to change notification settings - Fork 2
Explore 'cloning' children #19
Comments
Yes, I found this as well when using the Braid approach of wrapping the children. I experimented with cloning children - system-ui/theme-ui#809 (comment) (there were some issues when using emotion), but reverted it in the end, and I'm really annoyed that I can't remember exactly why, sorry, could well have been some silly restriction in the work component lib. Currently I've ended up copying the react-ui approach, using the lobotomized owl selector. |
My go-to approach was always the lobotomized owl, but the problem is that every component in theme-ui is class-based which means we need to double the specificty to override margins With this I'm keen to extend the cloned element's styles rather than override, if that makes sense? |
It didn’t at first, but yes, that does make sense. Trying to build these reusable layout components is massively helping my CSS understanding! |
I've been thinking of using this libaray, and this would be great imo! In a project where i'm using import { jsx } from 'theme-ui';
import { css } from '@theme-ui/css';
React.Children.map(children, child => {
const style = css({
mb: gap
});
return 'css' in child.props
? React.cloneElement(child, {
css: theme => [child.props.css(theme), style(theme)]
})
: jsx(child.type, { .key: child.key, ...child.props, css: style });
}) It doesn't feel like the cleanest solution, but, based on one of the emotion maintainer's comments here, maybe it's fine. Do you think something like this could work in raam's case as well? |
Thanks @jparklev, this is super helpful! I will give this approach a try soon and keep you posted |
Hey @robphoenix and @jparklev! I've been thinking about this more over the weekend and wanted to run an idea by you both. Rather than "cloning" the element, what if instead there was the option to use an So for example, with <Stack>
<StackItem as="p">
Hello I'm a paragraph
</StackItem>
</Stack> I'm still playing around with ideas, but I feel like something like this could be significantly less messy to manage and could be kept alongisde the current API without introducing breaking changes |
Hmm yea, i think – in combination with the current API – this could be good. My gut feeling is it would be preferable not to add a new element (and avoid somebody new looking at your frontend code and having to ask why sometimes |
I've looked into this some more and I think I'm going to drop this now… I tried an implementation where I used a context/provider but it becomes difficult to check the "type" of the children (and knowing when to apply styles), especially in Emotion I like the idea of having this as a feature, but I'm kinda thinking for odd use-cases it's best to create a use-case style. If either of you have the time to look into this again in the future, please feel free to re-open! |
Yeah I found this recently, you can't guarantee your children are going to be FWIW I've ended up straight up copying Braid's Stack and so far it's given me no issues at all, whereas all other options I've tried have had edge case issues. The only thing I dislike about it is all the extra divs, but such is life 🤷 |
Having used
raam
in a few side projects, I feel like I'm missing the markup flexibility.I'd like to look at a
cloneElement
variant where styles are appended to children rather than wrapped.The text was updated successfully, but these errors were encountered: