mui / base-ui

Base UI is an open-source library of accessible, unstyled UI components for React.
https://mui.com/base-ui/
MIT License
281 stars 46 forks source link

[API] Attribute duplication on slots vs. not, e.g. Slider data-orientation="horizontal" #625

Closed oliviertassinari closed 1 month ago

oliviertassinari commented 1 month ago

Summary

Open https://master--base-ui.netlify.app/components/react-slider/ see data-orientation="horizontal" 4 times in the DOM notes

SCR-20240916-uhfe

I'm confused, why is this not only once on the root DOM node and people use ~parent~ nested CSS selectors to write the CSS?

Search keywords: -

colmtuite commented 1 month ago

Why would you want to have to use :has() or descendant selectors?

It's generally a bad idea to have your styles rely on DOM structure. It will of course be fine in this specific example, but it's a bad practice. :has() and descendant selectors are less ergonomic and add noise. Especially with CSS-in-JS, syntax can get a little gnarly. It's standard practice in the space to provide style hooks per component.

github-actions[bot] commented 1 month ago

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

oliviertassinari commented 1 month ago

Why would you want to have to use :has() or descendant selectors?

@colmtuite I would imagine that we could have one data-orientation="horizontal" attribute on the root. In this configuration, developers would write CSS that looks like:

/* root */
.Slider {
  color: red;

  &[data-orientation="vertical"] {
    color: blue;
  }
}

/* thumb */
.Slider-thumb {
  background-color: red;

  [data-orientation="vertical"] & {
    background-color: blue;
  }
}

I don't see how :has() applies here.

With CSS-in-JS:

const SliderRoot = styled(SliderBase.Root)(`
  color: red;

  &[data-orientation="vertical"] {
    color: blue;
  }
`);

const SliderThumb = styled(SliderBase.Thumb)(`
  background-color: red;

  [data-orientation="vertical"] & {
    background-color: blue;
  }
`);

It feels like this duplication makes it harder to read the DOM in the dev tools to figure out what can be used for customization. On the opposite side, I don't really see the added value of the duplication.

colmtuite commented 1 month ago

I mentioned :has() because you mentioned parent CSS selectors, and :has() is the closest thing we have to a parent selector. I mentioned descendant selectors in my reply too.

we could have one data-orientation="horizontal" attribute on the root.

But I'm wondering why we would limit devs to this, and force them to use awkward selectors and rely on DOM structure?

.SliderThumb[data-orientation="vertical"] {} is simpler and safer.

oliviertassinari commented 1 month ago

@colmtuite My bad, I fixed that issue description, from the perspective of the node that gets customized, :has() seems like a child selector and .a. .b seems like a parent selector, but it's not the official terminology, that was created purely based on the look of the CSS selector 🙃.

But I'm wondering why we would limit devs to this, and force them to use awkward selectors and rely on DOM structure? .SliderThumb[data-orientation="vertical"] {} is simpler and safer.

Today, the duplication on this component seems OK-ish, no major downsides. I think that true problem could come in the future if we add a LOT of data attributes to the DOM, and that duplication would start to feel really heavy on the HTML output and browser devtools.

I have changed this issue description to be broader in scope. It will be interesting to see how developers approach this.

colmtuite commented 1 month ago

I don't anticipate a lot of attrs in the future. In fact I don't see a need for any additional attrs on sub components.

The idea is to be hands-off when it comes to styling. We don't want to say "you can't style this directly, you need to use descendant selectors". We want people to be able to style elements however they want. We want to enable devs, not restrict them.

There is also the TW experience to consider.