Skip to content

[internal] Add DirectionProvider#931

Merged
mj12albert merged 11 commits into
mui:masterfrom
mj12albert:feat/direction-provider
Dec 6, 2024
Merged

[internal] Add DirectionProvider#931
mj12albert merged 11 commits into
mui:masterfrom
mj12albert:feat/direction-provider

Conversation

@mj12albert

@mj12albert mj12albert commented Dec 2, 2024

Copy link
Copy Markdown
Member

@mj12albert mj12albert added the core label Dec 2, 2024
@mui-bot

mui-bot commented Dec 2, 2024

Copy link
Copy Markdown

@mj12albert
mj12albert force-pushed the feat/direction-provider branch 11 times, most recently from 01a19b9 to 451ba9c Compare December 3, 2024 09:20
@mj12albert
mj12albert marked this pull request as ready for review December 3, 2024 09:25
@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 3, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch from 451ba9c to 3315837 Compare December 3, 2024 14:18
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 3, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch 4 times, most recently from 31806df to 7115bd1 Compare December 4, 2024 09:29
@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 4, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch from 7115bd1 to e9e5589 Compare December 4, 2024 09:40
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 4, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch 3 times, most recently from 1a0016f to f05d2fa Compare December 4, 2024 10:42
@colmtuite
colmtuite requested a review from vladmoroz December 5, 2024 01:36
@mj12albert
mj12albert force-pushed the feat/direction-provider branch from a81268b to 45d6b6c Compare December 5, 2024 03:55
@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 6, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch from 45d6b6c to ed92993 Compare December 6, 2024 01:30
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 6, 2024
Comment on lines +37 to +38
const directionContext = useDirectionContext();
const direction = directionContext?.direction ?? 'ltr';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it only has one value, we could simplify this by defaulting ltr inside useDirectionContext() and returning the value directly, const direction = useDirectionContext()

@atomiks atomiks left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ScrollArea also checks for RTL. Should we default to checking for computed styles if it's cheap, like inside an event, and falling back to the provider otherwise?

Floating elements force read the style and would ignore the provider, unless we add it here: https://floating-ui.com/docs/platform#isrtl

@mj12albert

Copy link
Copy Markdown
Member Author

Should we default to checking for computed styles if it's cheap, like inside an event, and falling back to the provider otherwise?

I thought about this, but we have to document DirectionProvider as a required step when changing the behavior is involved anyway to maintain that "there is one single way to set up RTL", even though technically it's not necessary (e.g. for ToggleGroup/RadioGroup)

And if a component doesn't actually use the provider internally it would obscure issues when someone used the provider but forgot to set dir

@github-actions github-actions Bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 6, 2024
@mj12albert
mj12albert force-pushed the feat/direction-provider branch from ed92993 to e4571cc Compare December 6, 2024 05:36
@github-actions github-actions Bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 6, 2024
@mj12albert

mj12albert commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

Floating elements force read the style and would ignore the provider,

@atomiks I think this should be fine for our components since only Menu is the only one that requires the provider (it has keyboard nav).

For the rest (popover, preview card etc), they should first use logical sides instead of left/right

I suppose since align: 'start'|'end' is already direction-aware without the provider, maybe side should be as well? 0a51ad1#diff-3edb30ade872773deda81b8799e55170e5a0a58fab8e2bee97a2210169e99066R95-R109

@mj12albert
mj12albert requested a review from atomiks December 6, 2024 06:11
@mj12albert
mj12albert merged commit 0559f51 into mui:master Dec 6, 2024
@mj12albert
mj12albert deleted the feat/direction-provider branch December 6, 2024 07:02
@oliviertassinari oliviertassinari added internal Behind-the-scenes enhancement. Formerly called “core”. and removed core labels Aug 2, 2025
@oliviertassinari oliviertassinari changed the title [core] Add DirectionProvider [internal] Add DirectionProvider Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Behind-the-scenes enhancement. Formerly called “core”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[internal] Use a DirectionProvider to configure text direction (RTL/LTR) [tabs] Missing reference for RTL keyboard navigation direction

4 participants