Missing button group style injections when updating children className (React)

[Bielousov]

Describe the bug

The Button Group component injected class names are lost after updating child component's className.

To Reproduce

1. create a button group
2. update any button className property after it is mounted

e.g.:

  const [className, setClassName] = useState('test1');
  useEffect(() => {
    setTimeout(() => setClassName('test2'), 2000);
  }, []);

  return (
    <SlButtonGroup>
      <SlButton className={className}>One</SlButton>
      <SlButton className={className}>Two</SlButton>
    </SlButtonGroup>
  );

Expected: the updated Button className should should have both className and injected Shoelace class names
Actual: only default className classes are set to the button inside the ButtonGroup

Presumably for the same reason, internal button group class names are also often reset after a hot refresh in dev.

Screenshots

Before the className changed	After the className changed
class="test1 sl-button-group__button sl-button-group__button--first"	class="test2"
class="test1 sl-button-group__button sl-button-group__button--last"	class="test2"

[claviska]

Thanks for posting this. We'll probably need to switch to data attributes. I don't think we can avoid React replacing the entire class when using className.

[Bielousov]

Thanks for posting this. We'll probably need to switch to data attributes. I don't think we can avoid React replacing the entire class when using className.

Makes sense, I'm in fact working around this issue by using attributes.
Unless you find a reliable solution with pseudo-selectors, as in :first-child, :last-child, this should allow dynamic appending and deletion of children too. E.g.:

const [withDelete, setWithDelete] = useState(false);
useEffect(() => {
  setTimeout(() => setWithDelete(true), 2000);
}, []);

return (
  <SlButtonGroup>
    <SlButton>Action</SlButton>
    {withDelete && <SlButton>Delete</SlButton>}
  </SlButtonGroup>
);

This one is harder to work around with hidden attribute, for a different button should become --last.
If there was a method to re-initialize internal class names to call on re-render, that would probably do the job too.

[KonnorRogers]

@Bielousov There actually is. I had forgotten about this issue, but im 99% positive I can fix this. Let me see what I can do.

[KonnorRogers]

Reproduction here:

https://codepen.io/paramagicdev/pen/eYoBpqK?editors=0010

[KonnorRogers]

Since its hard to test the React version, heres the bare HTML version:

https://codepen.io/paramagicdev/pen/PogbZNb?editors=1000 <- Broken
https://codepen.io/paramagicdev/pen/XWQNXdo?editors=1000 <- Fixed

And heres the same codepen with the PR here: #1928