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

feat: add locale prop to Settings #2164

Merged
merged 6 commits into from
Sep 14, 2023

Conversation

markov00
Copy link
Member

Summary

This PR adds the locale prop to the Settings component.
This Unicode Locale Identifier is used across the entire charting library whenever a localized comparison or calculation is needed. For example in the multi-layer time axis or in some categorical/numerical comparison.

Details

I've also removed the manual translation used by the timeslip component because they where wrong and unused.

Issues

fix #2158
fix #2157

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • New public API exports have been added to packages/charts/src/index.ts
  • Unit tests have been added or updated to match the most common scenarios
  • The proper documentation and/or storybook story has been added or updated

@markov00 markov00 added enhancement New feature or request :i18n i18n related issue :all Applies to all chart types labels Sep 12, 2023
Copy link
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

There's a lot of passing around of locale I wonder if we could leverage a global context to access this easier like with useLocale hook. No for this PR but something to consider in the future.

@markov00
Copy link
Member Author

LGTM.

There's a lot of passing around of locale I wonder if we could leverage a global context to access this easier like with useLocale hook. No for this PR but something to consider in the future.

True, but I prefer to not be bonded to React hooks too much, it is also a bit difficult to use hooks here because most usage of the locale are within selectors/utils function rather then react environment.

@markov00 markov00 enabled auto-merge (squash) September 14, 2023 09:37
@markov00 markov00 merged commit 0bb3ab1 into elastic:main Sep 14, 2023
13 checks passed
@markov00 markov00 deleted the 2023_09_12-add_locale branch September 14, 2023 13:47
nickofthyme pushed a commit that referenced this pull request Sep 20, 2023
# [60.0.0](v59.1.0...v60.0.0) (2023-09-20)

### Bug Fixes

* **deps:** update dependency @elastic/eui to ^88.2.0 ([#2161](#2161)) ([6609a19](6609a19))
* **deps:** update dependency @elastic/eui to ^88.3.0 ([#2163](#2163)) ([624f43a](624f43a))
* **deps:** update dependency @elastic/eui to v85 ([#2113](#2113)) ([1b3fa7c](1b3fa7c))
* **deps:** update dependency @elastic/eui to v87 ([#2145](#2145)) ([312c32c](312c32c))
* **deps:** update dependency @elastic/eui to v88 ([#2154](#2154)) ([4070da0](4070da0))
* **tooltip:** rendering in react v18 ([#2169](#2169)) ([f30df54](f30df54))
* update font family ([#2165](#2165)) ([be07b0c](be07b0c))
* **waffle:** remove alpha artifacts ([#2139](#2139)) ([8eb4ede](8eb4ede))
* Wait a tick before reporting render status ([#2131](#2131)) ([fd2bca4](fd2bca4))
* **xy:** disable legend extra on ordinal ([#2114](#2114)) ([3ddfb18](3ddfb18))

### Features

* add locale prop to Settings ([#2164](#2164)) ([0bb3ab1](0bb3ab1))

### BREAKING CHANGES

* **xy:** when using the `ScaleType.Ordinal` for the X scale the legend extra value, representing the last and current hovered value, will not be shown.
@markov00 markov00 mentioned this pull request Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:all Applies to all chart types enhancement New feature or request :i18n i18n related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove specified locale translations for multi-layer time axis [xy] Connect multi-time axis to Kibana Locale
2 participants