-
Notifications
You must be signed in to change notification settings - Fork 103
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
feat: add localized formatting #2160
Conversation
🦋 Changeset detectedLatest commit: 629b7d8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Please update from master. Lets try to get this in a patch but lets fix conflicts first.
I'll update from master, but I think this should be part of at least a minor version since placeholder text will need to be updated in applications that are using the component. |
Can you show what teams need to do and what will break for teams? |
This is actually nontrivial for teams, since the placeholder needs to switch from being unconditionally |
179135a
to
7454f0d
Compare
So for now: lets not make this automatic. If they want to get this new functionality, have them pass an argument like |
7454f0d
to
8d25acd
Compare
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.
Approving for now if that placeholder issue is fixed.
8d25acd
to
886fc9d
Compare
Need to update snapshots in tests. @LuLaValva |
ae2cb4d
to
a48da89
Compare
LGTM overall codewise In |
Thanks for the review @agliga, while making an example for the error state I realized that we do not expose the necessary attributes for the textbox, so instead of continuing indirection I opted to add an |
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.
LGTM just make sure CI passes
Addresses: #2174
Description
Use
date-fns
to localize date format in date textbox instead of using ISO.Note
date-fns
maintains their own list of locales that likely doesn't exactly match the browser. This is likely something we will need to maintain ourselves, or provide/find a mappingScreenshots