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

Add toPercent utility for compatibility #4057

Merged
merged 2 commits into from
Oct 17, 2022

Conversation

thornbill
Copy link
Member

Changes
Should fix compatibility issues with browsers that do not support the Intl functions.

Issues
Should fix part of #4055.

@thornbill thornbill added bug Something isn't working regression We broke something labels Oct 16, 2022
Copy link
Contributor

@dmitrylyzo dmitrylyzo left a comment

Choose a reason for hiding this comment

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

LGTM

If we want to support correct percent formatting on older web engines, we need a real polyfill or some custom implementation.
We could use the translation string as the format (as suggested here). But that may not be enough, for example, for Farsi - non-Arabic numbers.
Anyway, I'm not an expert. 🙃

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.6% 0.6% Duplication

@thornbill
Copy link
Member Author

Yeah I think falling back to the previous behavior should be fine for older platforms that do not support the Intl APIs. The issue with the polyfills I saw was that they required importing multiple polyfills and each locale separately... which is a bit of a pain for such a minor usage.

@thornbill thornbill merged commit 622dd76 into jellyfin:master Oct 17, 2022
@thornbill thornbill deleted the fix-intl-compat branch October 17, 2022 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression We broke something
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants