[EUI] Improve EuiFilterButton badge i18n#5061
Conversation
- Refactor `activeFilters` to `numActiveFiltersDefined` to be more consistent with existing `numFiltersDefined` var - pull out showBadge conditional to make reading it a little clearer - DRY out badgeCount var
- Rather than trying to conditionally chop up the i18n string into parts, it makes more sense (and likely makes for better translations) to have 2 separate labels that render conditionally based on state + Switch from i18n render prop to use hook
… together - this feels more organized and easy to follow distinct groups of logic to me, but happy to revert if others disagree
|
@chandlerprall @thompsongl Couple questions for you on this one!
|
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5061/ |
Yes. Good to note for any consumer that might be using the tokens.
The release script automatically updates |
|
Awesome, thanks Greg! I added a changelog entry in bc6a8a1 but not sure I totally nailed it or if it could use work - feel free to leave feedback! 😅 |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5061/ |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5061/ |
thompsongl
left a comment
thompsongl
left a comment
There was a problem hiding this comment.
LGTM! Converting to useEuiI18n makes this easier to read; thanks!
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_5061/ |
3 participants
Summary
Partially addresses #4786 (Kibana's
i18n_eui_mapping.tsxwill need to be updated on next release upgrade as well to account for the i18n token changes).This PR fixes the static
activeandavailablestrings in our filter badge count aria-label not being properly translated. I originally went with the simpler fix in elastic/kibana#109333, but after feedback from Greg and Alejandro, I decided to remove the original conditional i18n string and instead split it up into 2 separate i18n strings, which I think will get translated more smoothly in different languages (vs. trying to chop it up based on English).This PR also contains some slightly opinionated code cleanup (mostly variable naming/organization) - I recommend following along by commit!
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in Chrome, Safari, Edge, and Firefox- [ ] Props have proper autodocs and playground toggles- [ ] Added documentation- [ ] Checked Code Sandbox works for the any docs examples- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately