-
Notifications
You must be signed in to change notification settings - Fork 212
feat: Add datePlaceholder and timePlaceholder props to date-range-picker component's i18nStrings #4104
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
base: main
Are you sure you want to change the base?
Conversation
src/date-range-picker/interfaces.ts
Outdated
|
|
||
| /** | ||
| * Placeholder text for date inputs in absolute mode. | ||
| * Should match the expected date format (e.g., "YYYY-MM-DD", "JJJJ-MM-TT" for German). |
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.
use "for example" instead of "e.g."
gethinwebster
left a comment
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.
"feat:" commits end up in our release notes, so they should include the component name somewhere
…e-picker from totoro!
914bd4e to
df559a5
Compare
5f5bfa3 to
585a74e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4104 +/- ##
==========================================
+ Coverage 97.18% 97.19% +0.01%
==========================================
Files 884 886 +2
Lines 25883 26000 +117
Branches 9350 9420 +70
==========================================
+ Hits 25154 25271 +117
+ Misses 723 682 -41
- Partials 6 47 +41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "i18nStrings.endMonthLabel": "نهاية الشهر", | ||
| "i18nStrings.endDateLabel": "تاريخ الانتهاء", | ||
| "i18nStrings.endTimeLabel": "وقت الانتهاء", | ||
| "i18nStrings.datePlaceholder": "YYYY-MM-DD", |
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.
Why are we using / for some and - for others, IMO it should be consistent with the i18Strings.dateConstraintText
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.
I think there need to be two different translations: datePlaceholder and slashedDatePlaceholder, similar to how there is dateTimeConstraintText and slashedDateTimeConstraintText
(although in this specific case of ar, it also looks like dateTimeConstraintText is incorrect: it should have YYYY-MM-DD)
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.
I've created a CR in the i18n package to add two new props:
isoDatePlaceholder(YYYY-MM-DD with dashes)slashedDatePlaceholder(YYYY/MM/DD with slashes)
This follows the same pattern as dateConstraintText/isoDateConstraintText/slashedDateConstraintText.
The original datePlaceholder will remain as-is for backward compatibility. Waiting on translations from Totoro team.
|
I think I understand why now, there's a property |
|
Added Totoro translations merged into main via CR-252051089. |
I've added format-specific placeholder strings to address this:
This follows the same pattern already established with Regarding the Totoro translations - I've noticed some inconsistencies across locales. I'll do a thorough audit of all date-related i18n strings and update any mismatches to ensure consistency between the placeholder and constraint text formats. |
NathanZlion
left a comment
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,
Description
Add configurable
datePlaceholderandtimePlaceholderprops to the date-range-picker component's i18nStrings, allowing developers to customize placeholder text for date and time inputs in absolute mode.Related links, issue #, if available: n/a
AWSUI-61514How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.