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

[EUI] Fix EuiFilterButton badge i18n #109333

Closed
wants to merge 1 commit into from
Closed

Conversation

cee-chen
Copy link
Contributor

Summary

closes elastic/eui#4786

I picked up a super simple EUI issue to familiarize myself more with EUI's i18n process and relationship with Kibana's i18n :)

I think this should solve the problem described in the linked issue above, but let me know if not!

Checklist

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v7.16.0 labels Aug 19, 2021
@cee-chen cee-chen requested a review from thompsongl August 19, 2021 19:49
@cee-chen cee-chen requested review from a team as code owners August 19, 2021 19:49
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 423.6KB 423.8KB +215.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@afharo
Copy link
Member

afharo commented Aug 20, 2021

The fix in Kibana LGTM. However, I wonder if we should fix it in the EUI library so any other consumers can take advantage of it?

@thompsongl
Copy link
Contributor

I wonder if we should fix it in the EUI library

Yeah @constancecchen we'll want first change it in EUI and then update the i18n_eui_mapping file here after it's released during the upgrade.

@cee-chen
Copy link
Contributor Author

Ahh great calls! I actually did spike out a more opinionated change in EUI, basically splitting up into 2 different strings rather than trying to make a single string conditional. I think it will get translated better that way and possibly more smoothly in different languages, rather than trying to chop it up based on English. What do y'all think? I'll open up the PR for EUI, would love thoughts there :)

@thompsongl
Copy link
Contributor

That diff looks good to me, @constancecchen!
The path remains the same: make the change in EUI and handle updating i18n_eui_mapping during the upgrade PR

@cee-chen
Copy link
Contributor Author

Closing in favor of elastic/eui#5061

@cee-chen cee-chen closed this Aug 20, 2021
@cee-chen cee-chen deleted the eui-i18n branch August 20, 2021 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[i18n] core.euiFilterButton.filterBadge is setting hardcoded values
4 participants