-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
chore: follow-up for rtl date/time format #7685
base: main
Are you sure you want to change the base?
Conversation
@@ -155,6 +155,10 @@ If the date field does not have a visible label, an `aria-label` or `aria-labell | |||
|
|||
Note that most of this anatomy is shared with [TimeField](TimeField.html), so you can reuse many components between them if you have both. | |||
|
|||
### Internationalization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rob suggested that it would be a good idea to leave some kind of information regarding the CSS changes needed to ensure the date/time format properly in RTL locales. I decided to add it like this but let me know if you have other opinions where it should go or contents that it should include.
@@ -31,7 +31,7 @@ export function useFormatHelpText(props: Pick<SpectrumDatePickerBase<any>, 'desc | |||
return ( | |||
formatter.formatToParts(new Date()).map((s, i) => { | |||
if (s.type === 'literal') { | |||
return <span key={i}>{s.value}</span>; | |||
return <span key={i}>{` ${s.value} `}</span>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noticed in chromatic when i ran it on main. just need to add some extra spacing since that's what we had before
Closes
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: