Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pickers] MultiSectionDigitalClock incorrect RTL behavior #11525

Closed
Eliav2 opened this issue Dec 27, 2023 · 10 comments · Fixed by #13447
Closed

[pickers] MultiSectionDigitalClock incorrect RTL behavior #11525

Eliav2 opened this issue Dec 27, 2023 · 10 comments · Fixed by #13447
Assignees
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! component: TimePicker The React component. good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@Eliav2
Copy link

Eliav2 commented Dec 27, 2023

are we sure this is fixed?

image

we can see that the minutes seconds are swapped.

Originally posted by @Eliav2 in #5561 (comment)

heres a complete sandbox showing the issue.

we can see that the hours and minutes are on the opposite side of what is expected. this sandbox implements MUI RTL theme as the docs describe.

Search keywords:

@github-actions github-actions bot added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 27, 2023
@LukasTy
Copy link
Member

LukasTy commented Dec 28, 2023

Hello @Eliav2. Thank you for creating this issue! 🙏
Could you please confirm if we understand the expected behavior correctly?
Screenshot 2023-12-28 at 10 05 36
Would you expect the hours and minutes sections to be swapped in this case to reflect how the Digital Clock renders time?

@LukasTy LukasTy added component: pickers This is the name of the generic UI component, not the React module! component: TimePicker The React component. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 28, 2023
@LukasTy LukasTy changed the title x time-pickers RTL support [pickers] MultiSectionDigitalClock RTL behavior Dec 28, 2023
@LukasTy LukasTy changed the title [pickers] MultiSectionDigitalClock RTL behavior [pickers] MultiSectionDigitalClock incorrect RTL behavior Dec 28, 2023
@Eliav2
Copy link
Author

Eliav2 commented Dec 28, 2023

Yes, exactly. I would even suggest considering adding an option to pass props to any inner element to allow full customization including overriding styles
(if this is already possible sorry about it)

@LukasTy
Copy link
Member

LukasTy commented Dec 28, 2023

Thank you for clarifying. 👍

I would even suggest considering adding an option to pass props to any inner element to allow full customization including overriding styles

There is the digitalClockSectionItem slot that you can target using slotProps. Does it allow you to achieve what you need? 🤔

@Eliav2
Copy link
Author

Eliav2 commented Dec 28, 2023

Our ux decided to proceed with different ui but this component is awesome and a true RTL support would benefit to others for sure(i have not trying override styles with slots)

@flaviendelangle flaviendelangle added the bug 🐛 Something doesn't work label Jan 9, 2024
@joserodolfofreitas
Copy link
Member

@Eliav2, I'm curious about your UX team's decision. What are you using now instead of the MUI X time picker?

@Eliav2
Copy link
Author

Eliav2 commented Jan 11, 2024

Because we focus on relative time(eg hours,minutes ,seconds from now) we just used 3 mui Select components to represent hours,minutes, seconds. @joserodolfofreitas

@LukasTy LukasTy added the good first issue Great for first contributions. Enable to learn the contribution process. label May 3, 2024
@arthurbalduini arthurbalduini self-assigned this May 29, 2024
@LukasTy
Copy link
Member

LukasTy commented Jun 17, 2024

@Eliav2 would you be able to clarify the expected tabulation order for the MultiSectionDigitalClock in RTL mode?
Does the focus order of following the views order make sense?
In the default case, it would be: hours, minutes, and meridiem.

cc @MBilalShafi if you can confirm this. 🙏

@Eliav2
Copy link
Author

Eliav2 commented Jun 17, 2024

Hours should be on the left. Minutes right to them. Seconds right to them. Time are internetional.

The resolution is as simple as adding sx={direction:ltr} on the right div. Because direction ordering should never change on nunbers. They are always the same.

@arthurbalduini
Copy link
Member

arthurbalduini commented Jun 18, 2024

Hey @Eliav2, thanks for the suggestion!
The way I see it, to implement your idea we'd need to create a new div to wrap the number sections separately from the meridiem. Although it might be a simple move, this would affect some critical parts of our components and might cause breaking changes in the DOM structure that could affect someone doing some CSS magic.
We can surely consider this for the next major version, but, for now, the best solution I see is to refactor how we order the sections considering the different directions.
Suggestions, comments and reviews are very welcome ! 😄

Copy link

⚠️ 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.

@Eliav2: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: pickers This is the name of the generic UI component, not the React module! component: TimePicker The React component. good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants