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

Localize date-range-picker #18945

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

karwosts
Copy link
Contributor

@karwosts karwosts commented Dec 7, 2023

Proposed change

Localizing the weekdays in date range picker. I think we still need to do the months as well, but wanted to get a review of this before moving to that.

image

Looking at what's currently on lokalize, this seems like it would work good for every language except Malayalam, which currently has pretty long strings for abbreviated weeekday, and get some overflow issues.

I'm not sure if there's anything I can do there except just push it out and wait for someone to come update it to be shorter?

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

src/translations/en.json Outdated Show resolved Hide resolved
@karwosts
Copy link
Contributor Author

karwosts commented Dec 7, 2023

@arunshekher - Could use your assistance/opinion for a Malayalam translation issue here, if you wouldn't mind to take a look (see image in top post)

@bramkragten
Copy link
Member

We can just use Intl.DateTimeFormat right?

Intl.DateTimeFormat(locale, { weekday: "short" })

@arunshekher
Copy link
Member

@karwosts Perhaps the agglutinative nature of the language and complex script characters makes the strings longer, but they are already in the shortened form and denotes just the names of the celestial bodies that represents the day without the 'day' part. If you want to shorten it further I think we can make it something like below. But I have never seen such a convention used in print or web.

  • ഞാ
  • തി
  • ചൊ
  • ബു
  • വ്യാ
  • വെ

@karwosts
Copy link
Contributor Author

karwosts commented Dec 8, 2023

I took another look at the calendar CSS and I think I figured out the right way to make it flex correctly, so I think it will be ok for all languages, it will just be a bit wider for some. Thanks for taking the time to check.

@karwosts
Copy link
Contributor Author

karwosts commented Dec 8, 2023

Updated:

image

short: boolean
): string => {
const date = new Date(Date.UTC(1970, 0 + month, 1));
return new Intl.DateTimeFormat(language, {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of creating 12 instances of DateTimeFormat, can we reuse it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

timeZone: "UTC",
}).format(date);
};

export const localizeWeekdays = memoizeOne(
Copy link
Member

@bramkragten bramkragten Dec 18, 2023

Choose a reason for hiding this comment

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

Last question, how often do we call this function, is it needed to be memoized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't pretend to understand the full page load sequence, but just by breakpoints and log messages looks like the Vue date-range-picker createElement function is called 4 times every time we navigate to the energy page (+1 more time when the picker is expanded), and 5 times every time we navigate to the history page (+1 more time when the picker is expanded).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also seems to be called 6 more times every time we change an entity in the history panel 😕

Copy link
Member

Choose a reason for hiding this comment

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

Seems less than ideal, but another issue 👍

@bramkragten bramkragten merged commit 2306234 into home-assistant:dev Dec 19, 2023
13 checks passed
@karwosts karwosts deleted the localize-date-range-picker-1 branch December 19, 2023 13:11
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing text in translation - Calendar days of week
3 participants