Skip to content

Include Plotly.js localization #7323

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

Merged

Conversation

yoshiokatsuneo
Copy link
Contributor

What type of PR is this?

  • Feature

Description

For now, the locale for the chat is English. It is not very intuitive for non-english people to show chart in English (ex: "January").

Make localized charts available by including and registering plotly.js localization definitions, and set locale based on browser language.

How is this tested?

  • Manually

I confirmed that the "Month" and pop-up information is localized.

image image

@yoshiokatsuneo yoshiokatsuneo force-pushed the feat/include_plotly_localization branch from 2e1eae5 to d518a6d Compare February 12, 2025 08:58
Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

Is there a method of including all locales? It's hard to tell if we missed one

@yoshiokatsuneo
Copy link
Contributor Author

@eradman

Is there a method of including all locales? It's hard to tell if we missed one

I don't think so.
Unfortunately, I think plotlyjs does not provide the way to register all locale at once, and webpack does not provide simple way to import all the locale files.

@eradman
Copy link
Collaborator

eradman commented Feb 19, 2025

If I'm reading this right, this will add about 88KB to the size of the web app:

before
web-ui-load

after
web-ui-load-pr-7323

Out of 6.5MB this is an increase of 1.3%, which is probably fine.

@eradman
Copy link
Collaborator

eradman commented May 15, 2025

I think we determined that this change will have a minimal impact on the size of the distribution. @yoshiokatsuneo please resolve conflicts and I think we can merge.

@yoshiokatsuneo yoshiokatsuneo force-pushed the feat/include_plotly_localization branch from d518a6d to cbd8820 Compare May 16, 2025 16:25
@yoshiokatsuneo
Copy link
Contributor Author

@eradman

Thanks !
I just rebased and resolved the conflicts !

Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

Super! Merging

@eradman eradman merged commit e46d44f into getredash:master May 16, 2025
11 checks passed
@yoshiokatsuneo
Copy link
Contributor Author

@eradman Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants