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

GrafanaData: Fix week start for non-English browsers #50582

Merged
merged 3 commits into from
Jun 14, 2022

Conversation

AgnesToulet
Copy link
Contributor

What this PR does / why we need it:
This PR fixes the week start behavior on browsers using another language than English. The issue was that weekdays() return the week days in the set locale and we compare it with values from the backend that are always in English (at least for now); so we couldn't find the week day index and setting the week start in the preferences or the dashboard settings didn't work.

Special notes for your reviewer:
I didn't find a better way to ensure that the getWeekdayIndex function doesn't change the locale but I'm not very familiar with moment. I couldn't find anything else in the doc though.

@AgnesToulet AgnesToulet changed the title grafana-data: Fix week start for non-English browsers Grafana-data: Fix week start for non-English browsers Jun 10, 2022
Copy link
Contributor

@leventebalogh leventebalogh left a comment

Choose a reason for hiding this comment

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

LGTM 👍

(I am not sure that getWeekdayIndex() is still used anywhere else?)

@AgnesToulet
Copy link
Contributor Author

(I am not sure that getWeekdayIndex() is still used anywhere else?)

It's not used in Grafana codebase but as it's part of grafana-data package, I was wondering if removing a function from this package would be considered a breaking change? (I'm not used to work on this package so I may be wrong there).

@leventebalogh
Copy link
Contributor

You are absolutely right @AgnesToulet, sorry, I somehow missed that. 👍

@AgnesToulet AgnesToulet changed the title Grafana-data: Fix week start for non-English browsers GrafanaData: Fix week start for non-English browsers Jun 14, 2022
@AgnesToulet AgnesToulet merged commit 99c8ce5 into grafana:main Jun 14, 2022
@AgnesToulet AgnesToulet deleted the grafana-data/fix-start-week branch June 14, 2022 13:12
grafanabot pushed a commit that referenced this pull request Jun 14, 2022
* grafana-data: Fix start week for non-English browsers

* apply review suggestion

(cherry picked from commit 99c8ce5)
AgnesToulet added a commit that referenced this pull request Jun 14, 2022
* grafana-data: Fix start week for non-English browsers

* apply review suggestion

(cherry picked from commit 99c8ce5)

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants