[core] Rename `direction` prop to `dir` #831

mj12albert · 2024-11-19T08:40:13Z

We decided to rename this to dir to match the HTML dir attribute, which would help reduce a potential collision

Relevant components:

Accordion
Menu
Meter
Progress
RadioGroup
Slider
Tabs
ToggleButtonGroup
Toolbar

Search keywords:

The text was updated successfully, but these errors were encountered:

oliviertassinari · 2024-11-19T14:05:24Z

Does this work in the first place? There seem to be two cases:

Greenfield project. I set the direction on the html element, once at the root <html dir="rtl">. I don't want the underlying DOM node to have the dir attribute to avoid bloat in the DOM tree.
Brownfield project. I've added a few new components. I haven't made my whole app RTL-compliant yet. I want to add dir="rtl" a pages at a time.

Now, case 2. likely doesn't make a lot of sense. You wouldn't release RTL partially done.
But in case 1. doesn't this hold?

I don't want the underlying DOM node to have the dir attribute to avoid bloat in the DOM tree.

If I set <Slider dir="rtl"> I would expect this to be on the DOM node, but I would likely not want this attribute, only the RTL behavior. If it's not on the DOM node, how do I add it for case 2?

So overall, the core problems seem to be that the dir prop inherits but the direction prop doesn't inherit, so those need to be two different prop names?

mj12albert · 2024-11-19T14:48:10Z

I don't want the underlying DOM node to have the dir attribute to avoid bloat in the DOM tree

Good point ~ I didn't consider this but with that in mind, what Ariakit does by separating behavior and layout makes sense¹

I think we could expand on what was done in #825 and apply to all components that need to support RTL behavior:

Detect the CSS direction property (inheritable from <html dir="rtl">) and use that to determine the default text direction and apply LTR/RTL behavior accordingly
Provide a prop (e.g. direction, anything that is not dir to avoid confusion) that will let you override this
Base UI components will never set the dir attribute in the DOM

What do you think? @oliviertassinari @vladmoroz

https://ariakit.org/reference/toolbar#rtl ↩

mj12albert · 2024-11-19T14:51:41Z

Here's an example of Ariakit's Toolbar with RTL: https://stackblitz.com/edit/vitejs-vite-jth9ne?file=src%2Fstyle.css

The rtl prop applies RTL keyboard behavior but it still appears LTR because the dir attribute is not 'rtl' in the DOM

oliviertassinari · 2024-11-19T22:07:57Z

Ariakit does by separating behavior and layout

@mj12albert They say in https://ariakit.org/reference/toolbar#rtl "This only affects the composite widget behavior. You still need to set dir="rtl" on HTML/CSS.".

I don't know, Ariakit API seems unintuitive. If I set dir="rtl" on a component, why is the component swallowing the prop? I would expect the attribute to be on the DOM node if the prop is named this way.

mj12albert · 2024-11-20T02:49:00Z

If I set dir="rtl" on a component, why is the component swallowing the prop? I would expect the attribute to be on the DOM node if the prop is named this way.

I mean we do this without renaming our direction prop: the inherited CSS direction property would determine the default behavior. Users could do that by setting the HTML dir attribute anywhere from the html tag to the component itself, we will not interfere with it. This should cater to both greenfield and brownfield cases.
(The direction prop would provide a way to override this if needed)

Ariakit API seems unintuitive

I agree - Ariakit makes you set both dir and their rtl: boolean prop. For us, you'd only need to set the dir attribute

oliviertassinari · 2024-11-20T18:26:12Z

@mj12albert I'm lost, I don't understand your last comment. What's the solution to handle the html dir attribute that inherits vs. the React prop for the rtl direction that doesn't inherit?

A few ideas on the solution space, options:

we call the prop for the components differently, not dir
we have a Direction React context, no props (I guess we need a context anyway to store this for each component, so maybe it makes sense to unify it. RTL is 2% of the needs, so we can degrades the UX/DX for RTL if it improves LTR)
I have doubts about reading the DOM to figure out the dir works in the first place: a. can this lead to layout trashing? b. can't change the DOM structure based on it, breaks SSR c. CSS isn't enough https://stackoverflow.com/questions/5375799/direction-ltr-rtl-whats-the-difference-between-the-css-direction-and-html-di

mj12albert · 2024-11-21T07:54:42Z

we call the prop for the components differently, not dir

Yes ~ after considering your point about not cluttering the DOM with dir attributes everywhere, I agree with this @oliviertassinari

What's the solution to handle the html dir attribute

I'm proposing that components can be aware of the dir attribute anywhere higher in the DOM, and set their keyboard interactions based on that by default:

<html dir="rtl">
  <body>
    <Slider /> {/* left/right arrow keys are reversed */}
  </body>
</html>

This would also work, but dir is not a prop, it's just forwarded to the root element as a normal HTML attribute

<html>
  <body>
    <Slider dir="rtl" /> {/* left/right arrow keys are reversed */}
  </body>
</html>

Here's a demo of how this works with ToggleButtonGroup: https://deploy-preview-763--base-ui.netlify.app/experiments/toggle-group
(and the source here)

a. can this lead to layout trashing? b. can't change the DOM structure based on it, breaks SSR c. CSS isn't enough

a. in Firefox, where computedStyleMap isn't supported, we do have to use getComputedStyles
b./c. this is only about RTL keyboard interactions, especially if we want to avoid putting the dir attribute at the component level, so I don't think it has any SSR implications?

we have a Direction React context

I know Radix does this with a DirectionProvider, though both using that and using a prop on a component will set the HTML dir attribute on the component (sandbox). We can discuss this option as well with the team

oliviertassinari · 2024-11-25T01:04:26Z

in Firefox, where computedStyleMap isn't supported, we do have to use getComputedStyles

@mj12albert So computedStyleMap wouldn't trigger https://gist.github.com/paulirish/5d52fb081b3570c81e3a?

so I don't think it has any SSR implications

For a Slider, the SSR output depends on the direction, no?

base-ui/packages/react/src/Slider/Root/useSliderRoot.ts

Line 345 in 345ce7a

percent = (finger.x - left) / width + offset * (isRtl ? -1 : 1);

I know Radix does this with a DirectionProvider

Oh, I didn't know, option 2.

mj12albert added the core Infrastructure work going on behind the scenes label Nov 19, 2024

mj12albert self-assigned this Nov 19, 2024

mj12albert mentioned this issue Nov 22, 2024

[tabs] Modernize implementation #751

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Rename `direction` prop to `dir` #831

[core] Rename `direction` prop to `dir` #831

mj12albert commented Nov 19, 2024 •

edited

Loading

oliviertassinari commented Nov 19, 2024 •

edited

Loading

mj12albert commented Nov 19, 2024

mj12albert commented Nov 19, 2024

oliviertassinari commented Nov 19, 2024 •

edited

Loading

mj12albert commented Nov 20, 2024 •

edited

Loading

oliviertassinari commented Nov 20, 2024 •

edited

Loading

mj12albert commented Nov 21, 2024 •

edited

Loading

oliviertassinari commented Nov 25, 2024 •

edited

Loading

[core] Rename direction prop to dir #831

[core] Rename direction prop to dir #831

Comments

mj12albert commented Nov 19, 2024 • edited Loading

oliviertassinari commented Nov 19, 2024 • edited Loading

mj12albert commented Nov 19, 2024

Footnotes

mj12albert commented Nov 19, 2024

oliviertassinari commented Nov 19, 2024 • edited Loading

mj12albert commented Nov 20, 2024 • edited Loading

oliviertassinari commented Nov 20, 2024 • edited Loading

mj12albert commented Nov 21, 2024 • edited Loading

oliviertassinari commented Nov 25, 2024 • edited Loading

[core] Rename `direction` prop to `dir` #831

[core] Rename `direction` prop to `dir` #831

mj12albert commented Nov 19, 2024 •

edited

Loading

oliviertassinari commented Nov 19, 2024 •

edited

Loading

oliviertassinari commented Nov 19, 2024 •

edited

Loading

mj12albert commented Nov 20, 2024 •

edited

Loading

oliviertassinari commented Nov 20, 2024 •

edited

Loading

mj12albert commented Nov 21, 2024 •

edited

Loading

oliviertassinari commented Nov 25, 2024 •

edited

Loading