joe-bell/raam

Explore 'cloning' children

joe-bell opened this issue · 9 comments

joe-bell commented

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.

robphoenix commented

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.

joe-bell commented

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?

  • 👍2
robphoenix commented

It didn’t at first, but yes, that does make sense. Trying to build these reusable layout components is massively helping my CSS understanding!

jparklev commented

I've been thinking of using this libaray, and this would be great imo! In a project where i'm using theme-ui, i've been doing

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?

  • ❤️1
joe-bell commented

Thanks @jparklev, this is super helpful! I will give this approach a try soon and keep you posted

  • ❤️1
joe-bell commented

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 Item element of the layout?

So for example, with Stack:

<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

jparklev commented

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 Stack has StackItems and sometimes not). However, I certainly agree with this being "significantly less messy," even more so the longer I use the cloning approach in my own projects

  • 👍1
joe-bell commented

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!

robphoenix commented

it becomes difficult to check the "type" of the children (and knowing when to apply styles)

Yeah I found this recently, you can't guarantee your children are going to be StackItems. Which can be ok, reach-ui does a lot of composition where a certain weight of responsibility is on the user to use the right components. However for a library such as this which is standalone layout components, rather than contained as a part of a larger system, I guess you have to expect users to put whatever they want in the Stack and for the Stack to deal with it all appropriately.

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 🤷

  • 👍1