Skip to content

fix: correct month names localization in Datepicker#547

Open
LeonidShv wants to merge 1 commit into
infermedica:developfrom
LeonidShv:fix/datepicker-localization-months
Open

fix: correct month names localization in Datepicker#547
LeonidShv wants to merge 1 commit into
infermedica:developfrom
LeonidShv:fix/datepicker-localization-months

Conversation

@LeonidShv
Copy link
Copy Markdown

@LeonidShv LeonidShv commented Apr 18, 2025

Description

I've fixed a part of the published issue: "Data picker behaviour with non-English".

When you change the language, the months in the date picker didn’t update and showed incorrect translations.

Commits:
fix: correct month names localization in Datepicker and replace monthNames ref with computed
→ I've made monthNames a computed property that updates automatically when props.lang changes

The issue with the capitalization of the first letter in month names is not fixed in this PR.
I plan to address it in a separate fix after this PR is successfully merged.

Related Issue

"Issue 526: Data picker behaviour with non-English".

steps to reproduce:
Go to https://component.infermedica.com/?path=/story/organisms-datepicker--full-configuration
Click on props.lang
Select any language other than en or en-us
See that nothing changes in Month tab

Closes #

Motivation and Context

"Issue 526: Data picker behaviour with non-English".
When changing the language, the months in the date picker didn’t update and showed incorrect translations.

How Has This Been Tested?

You can test it by using the DatePicker component and changing the lang prop.
Example:

<UiDatepicker :lang="selectedLang" />

Screenshots (if appropriate):

Screen.Recording.2025-04-18.at.20.40.41.mov

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@LeonidShv LeonidShv requested a review from a team as a code owner April 18, 2025 18:16
@AlanLes
Copy link
Copy Markdown
Collaborator

AlanLes commented Apr 22, 2025

Hello @LeonidShv! 🙇
Both your code and the solution look good, and I'll approve that, for sure 😄

However, I see an inconsistency in the main Description.

You've written that:

fix: correct month names localization in Datepicker
→ Added a watcher for the lang prop to update month names when the language changes.

I don't see any new watcher added. Is that up to date?

@LeonidShv
Copy link
Copy Markdown
Author

fix: correct month names localization in Datepicker
→ Added a watcher for the lang prop to update month names when the language changes.

Hi @AlanLes ,
Thanks for the quick reply!

Maybe I didn’t explain it very well.

First, I added the watcher in the first commit. Then, in the second commit, I removed this watcher as part of a small refactoring.
If needed, we can squash these commits during the PR merge. Also, if you prefer, I can squash them myself in this PR.

I’d be happy if you could share some links to your internal resources — maybe a task board, code style guide, or commit naming convention.

@AlanLes
Copy link
Copy Markdown
Collaborator

AlanLes commented Apr 23, 2025

@LeonidShv, yes, please squash it, and let's make this PR full-blown 😄 👍

Regarding our internal resources, I'm afraid all our resources are, as you wrote, internal. Most of the docs are on our Confluence, which is restricted to Infermedica's employees. However, I believe the docs should be sufficient for you. The most crucial information is there. 🙂

Thanks for your contribution! 🙇

@LeonidShv LeonidShv force-pushed the fix/datepicker-localization-months branch from d29521c to f8b7d24 Compare April 23, 2025 07:35
@LeonidShv
Copy link
Copy Markdown
Author

@AlanLes
I've squashed 2 commits. I hope everything is okay, and I'll wait for the approval.
I'm always open to questions or ideas.

Regarding to the documents and code style, I will try to compare my PRs with existed closed on.

Thanks for your time : )

@AlanLes
Copy link
Copy Markdown
Collaborator

AlanLes commented Apr 23, 2025

@LeonidShv, I still don't see any watcher added here.

image

@LeonidShv
Copy link
Copy Markdown
Author

LeonidShv commented Apr 23, 2025

@AlanLes
Thank you for the updates.
That's my mistake — I forgot to update the PR description after squashing the commits. I've updated the PR description accordingly.

After squashing the commits, the watcher is no longer present

Copy link
Copy Markdown
Collaborator

@AlanLes AlanLes left a comment

Choose a reason for hiding this comment

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

Awesome, the code looks good 😄 👍
@DanielHajduk98 @sonya0504 @pspaczek @mateuszkulpa, would you guys like to see and approve, or add any comments to it?

@LeonidShv
Copy link
Copy Markdown
Author

@AlanLes Thank you for the feedback! I hope my next PR will be clearer : )
I don't see the merge button. Could you please help merge this PR? Probably, I don't have permissions for that as an external collaborator

@AlanLes
Copy link
Copy Markdown
Collaborator

AlanLes commented Apr 29, 2025

@LeonidShv sure, I've approved it. Let's wait for another approval from someone from the team. @sonya0504 @DanielHajduk98 @kubajmarek @pspaczek @mateuszkulpa @jbucior? :)

@kubajmarek
Copy link
Copy Markdown
Contributor

I don't think my or Przemek's approval would be useful here, as it doesn't have much power after org removal. 😉

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.

3 participants