qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
764 stars 259 forks source link

Replace cldr by intl #10668

Open goldim opened 2 months ago

goldim commented 2 months ago

This is my first try to move qooxdoo from CLDR to Intl API and not final I think. This allows us to get rid of big dependency which weights too much (500 MB). During migration I encountered bunch of problems, questions and ambiguities which I tried to solve on my own. And that's why I am asking you help me to change some of my solutions if that's required. Also I've created a discussion but didn't see a big involvement. No problem, I think the PR is right place. I've added tests for most methods which have not been tested. I would like to emphasize that some solutions via Intl API are elegant if you look at methods like getStartWeek, isWeekend, getWeekendStart, getWeekendEnd and will notice that there are no hardcoded values and exceptions. But Intl API has some disadvantages. There are following moments which I would like to clarify with you:

hkollmann commented 2 months ago
  1. Yes you should rebuild and add npm-shrinkwrap!
goldim commented 2 months ago

@hkollmann Fixed.

goldim commented 2 months ago

Just noticed that Intl.Locale is only supported since 124 version of Firefox and got failed in 123 of Ubuntu. I think to back old implementation of some methods.

derrell commented 2 months ago

Unfortunately, I have no way to test this, as I have almost no experience with localization, and only the most basic of usage of it in my apps. I'm concerned, however, about the list of variants that aren't covered by this change. It's very likely to affect some users. My suggestion is to mark as deprecated, in the upcoming release of 7.x, as many of the things that are known to be issues with the new implementation as possible, and then actually release this change as part of 8.0. Some users will likely encounter non-BC changes that will require altering their code.

Since this PR is marked as "do not merge - v8", I will approve this PR as is. I added the additional "do not merge" tag just in as an added warning, probably not necessary, but doesn't hurt.

goldim commented 2 months ago

@derrell it could be tested with our apps like widget browser for example. To see if the localization works set other locale and see how a datefield calendar is changed. Btw I can not fine an demo application in which you could change locale or country. I remember there were flags of 5 or 6 languages.