as_child for Button

[mahdi739]

This is not a ready-to-merge PR. I just had an idea and wanted to share it.

I implemented a very naive as_child mechanism in the Button component, which passes the children directly without the button element.

This probably comes with a lot of edge cases that I haven't tested.

In comparison to Radix UI, in all conflicts, the parent wins, except for classes, which will merge because of the new PatchClass type I declared (it probably has room for further optimization and improvement).

[adoyle0]

Hmm, what is the use case for this? I'm guessing on <...Trigger> components? So you'd have something like

<DropdownTrigger>
    <Button as_child=true />
</DropdownTrigger>

right? Are you using this anywhere? If so can you provide an example of how you're using it? If this is what I think it is then I'd love to have it because right now all trigger components are just buttons, which is fine most of the time until it's not.

Or, as I just realized before submitting this, I think <DropdownTrigger> would get as_child=true and then the button would become the trigger, right? I'd like to see some examples and then we can poke at it.

[mahdi739]

Yes, but as_child should be applied to the parent element. It then renders only the child element and attaches the parent’s attributes to it. So, the appearance and functionality of the parent are merged with the child, without rendering the parent element itself.

My own usecase was this:

<Button as_child=true>
  <label for="file" class="cursor-default">
    <input type="file" id="file" class="hidden" />
    {icon!(LuUpload)}
    "Upload File"
  </label>
</Button>

Which is rendered like this:

<label class="cursor-default singlestage-btn-primary" for="file">
<input type="file" id="file" class="hidden">
<svg class="" style="" xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" stroke="currentColor" fill="none"><path d="M12 3v12"></path>
<path d="m17 8-5-5-5 5"></path>
<path d="M21 15v4a2 2 0 0 1-2 2H5a2 2 0 0 1-2-2v-4"></path><!---->
</svg>
Upload File
</label>

I found a lot of examples in the shadcn blocks page:
https://ui.shadcn.com/blocks/sidebar

[mahdi739]

I added as_child to some triggers:
Here are a few challenges I faced:

1. Using as_child in components with more than one child element feels awkward to me, especially in cases like AccordionTrigger, which relies on the <summary> element. (It breaks if replaced, due to how <details> works in HTML.)
   Should we shift this responsibility to the user? Or should we rewrite the component entirely?

2. Another challenge is that classes are hard-coded in the components. So even when a user passes a custom child via as_child, the original class (e.g., singlestage-btn-primary) still gets applied to it.
   One solution is to make the entire library unstyled and provide defaults in the website. (My fave)
   Another option is to use a closure that passes the class as an argument. For example (untested):

   <Button render=move |class| {
       view! { <CustomChild class=class /> }
   } />

   Or maybe:

   <Button as_child=true let:class>
       <CustomChild class=class />
   </Button>

3. Another issue is specific to certain trigger components. Currently, some use a <button> with popovertarget, but this attribute only works on <button> or some input elements. So while having as_child make sense it doesn't work for every element. It would be a kind of abstraction leak.

[mahdi739]

I tested this syntax:

<Button as_child=true let:class>
    <CustomChild class=class />
</Button>

It works and looks nice. we can even expose more than just the class, like component states or other props. BUT, the let: syntax is not optional, so users are forced to write it even when they don’t need it.
A workaround is to create a dedicated component, like ButtonSlot. And we’d end up with a twin for every component with a prefix or suffix.

[adoyle0]

1. Using `as_child` in components with more than one child element feels awkward to me, especially in cases like `AccordionTrigger`, which relies on the `<summary>` element. (It breaks if replaced, due to how `<details>` works in HTML.)
   Should we shift this responsibility to the user? Or should we rewrite the component entirely?

Maybe we can only affect the inner element (the <h3> in this case), and just make sure it's always wrapped in a summary. I don't really care what happens to the components, as long as the API remains the same to the user/is backwards compatible. If there's a major conflict I think I would rather make the changes to the as_child feature to accommodate the Accordion, rather than modify the behavior of the Accordion because right now it mostly matches shadcn and should be intuitive to most users. Also I think the more common use-case is without as_child.

The CSS will have to change for sure (like > h3 to > .singlestage-accordion-trigger). This is a pattern that I'm realizing was a bad choice and exists in a few places around the library. We should be able to then just apply class="singlestage-accordion-trigger" to the replacement element to get more or less the same styling as the default. There may be conflicts using the full styling so maybe we'll need something like a singlestage-accordion-child-trigger with just minimal required styling (if any) and leave the rest to the user. I think if they're using as_child then they already have opinions on styling anyway so they shouldn't mind doing it themself and may actually prefer to.

2. Another challenge is that classes are hard-coded in the components. So even when a user passes a custom child via `as_child`, the original class (e.g., `singlestage-btn-primary`) still gets applied to it.
   One solution is to make the entire library unstyled and provide defaults in the website. (My fave)
   Another option is to use a closure that passes the class as an argument. For example (untested):
   ```
   <Button render=move |class| {
       view! { <CustomChild class=class /> }
   } />
   ```
   Or maybe:
   ```
   <Button as_child=true let:class>
       <CustomChild class=class />
   </Button>
   ```

I'm not sure what you mean specifically about "provide defaults in the website" but we should be able to just conditionally include the class depending on if as_child is being used or not. Or even conditionally choose between something like singlestage-btn and singlestage-btn-as-child.

3. Another issue is specific to certain trigger components. Currently, some use a `<button>` with `popovertarget`, but this attribute only works on `<button>` or some input elements. So while having as_child make sense it doesn't work for every element. It would be a kind of abstraction leak.

button is a bit of a hard requirement. I'll have to play around a bit to see if I can get an unstyled button to play nicely with arbitrary children. Maybe <input type="button" /> will offer more freedom?

At the end of the day we have quite a bit of freedom to modify implementation code as long as the API and behavior remain the same, or similar. We can break things if we really have to. This is a feature I would like to have, whether it matches Radix/shadcn or not. Like I said above, if we have to bend things to fit I would rather bend the new feature than break the existing API. Within reason.

let:: does conditionally applying styling classes or more granular styling classes like I mentioned above change this requirement? Allowing this feature, or using it internally is one thing, but I don't like the idea of requiring it because I think it's ugly. It's probably outside the scope of this PR to explore, but something I've been meaning to try out is using custom HtmlElements instead of components. This should allow cutting down on unused attrs as props, and may also give us the freedom to avoid this let: requirement.

[adoyle0]

https://www.w3.org/TR/2012/WD-html5-20120329/the-button-element.html#the-button-element buttons actually support lots of children: "Phrasing content, but there must be no interactive content descendant."

[mahdi739]

Maybe we can only affect the inner element (the <h3> in this case), and just make sure it's always wrapped in a summary.

Yes, that probably works most of the time, but I think when users use as_child, they assume there is no longer an inner element. As a result, they write CSS selectors targeting the original structure, which would break.

The CSS will have to change for sure (like > h3 to > .singlestage-accordion-trigger). This is a pattern that I'm realizing was a bad choice and exists in a few places around the library. We should be able to then just apply class="singlestage-accordion-trigger" to the replacement element to get more or less the same styling as the default.

Totally agree.

I'm not sure what you mean specifically about "provide defaults in the website"

I mean something like BaseUI. Unstyled by default, but the website explicitly provides ready-to-use styles (via both Tailwind and CSS modules) that users can copy. This way, there’s no complexity in tweaking styles, everything lives in user land.

choose between something like singlestage-btn and singlestage-btn-as-child.

Exactly. Right now, these classes are implementation details, but you can expose them as official default styles. Then users can explicitly apply them, tweak them, or write their own.

If we have to bend things to fit I would rather bend the new feature than break the existing API. Within reason.

I understand, but since this library is still in its early stages, you can make bigger changes now. Breaking changes now are far better than being stuck with a problematic design decision a year from now.

I don't like the idea of requiring it because I think it's ugly.

I agree. What about the twin component idea? (<ButtonSlot/>)

[adoyle0]

let is better than Slot if neither is not an option

[adoyle0]

Also if a user wants to do their own styles they can just choose to not use ThemeProvider. I could add none variants as options to things that use style variants. Without ThemeProvider all the classes can be reused like .singlestage-input, etc.