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

Bug: calcite-input showing Arabic characters for numbers only, add an option to switch to western numbers #3079

Closed
2 tasks
AdelheidF opened this issue Sep 20, 2021 · 70 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. i18n-l10n issues dealing with internationalization/localization p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@AdelheidF
Copy link

AdelheidF commented Sep 20, 2021

Original issue submitted by Olga.

In FF and Safari when language is set to ar numbers in the input boxes appear in Arabic. Chrome doesn't have this problem, it is always showing English numbers. We only want to see English numbers, no matter the user's language.

image

image
When dragging the dot of color picker over the colored area the values update with other Arabic numbers.
Same when using up and down arrows.

https://jsbin.com/dutarurohu/edit?html,output

I'm using beta.65, but I also see it with beta.59.

Actual Behavior

Showing Arabic numbers

Expected Behavior

only English numbers

Reproduction Steps and Sample

Sample:

Contact me

Relevant Info

Version: @esri/calcite-components@beta.65

  • CDN
  • NPM package
@AdelheidF AdelheidF added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 20, 2021
@github-actions
Copy link
Contributor

More information is required to proceed with this issue:

This issue will be automatically closed in three days if the information is not provided. Thanks for your understanding.

@github-actions github-actions bot added the need more info Issues that are missing information and/or a clear, actionable description. label Sep 20, 2021
@benelan
Copy link
Member

benelan commented Sep 21, 2021

Reproducible in Firefox: https://jsbin.com/yarikuquji/edit?html,output

@benelan benelan removed the need more info Issues that are missing information and/or a clear, actionable description. label Sep 21, 2021
@julio8a julio8a added this to the Sprint 10/11 – 10/22 milestone Sep 22, 2021
@julio8a julio8a removed the needs triage Planning workflow - pending design/dev review. label Sep 22, 2021
@julio8a
Copy link

julio8a commented Sep 23, 2021

@AdelheidF, where does the requirement to only support English numbers come from?
@babakbolour mentioned that showing Arabic characters is the best practice, see comment here: #2765 (comment)

@AdelheidF
Copy link
Author

AdelheidF commented Sep 23, 2021

where does the requirement to only support English numbers come from?

Map Viewer issue 3854

Expected:

All the inputted number should display as western digit to consistent with others on New Map Viewer.

calcite-date-picker seems to be mixed. I thought we said there that that's OK. Input is still western...
image

Also, Chrome displays only western for calcite-input. Should be the same between browsers anyway.

@babakbolour
Copy link

@AdelheidF here is my perspective on this: What Map Viewer has implemented or restricted users to (English numbers), is Map Viewer (or even Esri app) app specific. Calcite is not only for internal esri-developed apps, but also for public to use and create solutions/GIS apps. For that, it would be best to not exclude or restrict our users to only use English/western digits.
Also, @raje9276 is saying Chrome has now fixed their bug and now show Arabic western digits.

@AdelheidF
Copy link
Author

AdelheidF commented Sep 23, 2021

Ok, is there an option to change it to western?

It doesn't really matter to me, I'm trying to follow what the i18n team (Olga in this case) is suggesting.

@babakbolour
Copy link

@AdelheidF let me talk to Olga and get back to you and clarify.

@babakbolour
Copy link

@AdelheidF I chatted with Olga on what she recommended originally and the different recommendations between Olga's and mine. What Olga did not know when she gave her recommendation, is that Calcite is now a customer-facing product/platform, released to global non-Esri people. Given this new information, she agrees with my recommendation - to support Arabic indic digit in Calcite input. pls let us know if you have additional info or if we can help in anyways - coding, testing, etc cc: @julio8a

@AdelheidF
Copy link
Author

Talked to Olga again and we do want western numbers inside the Map Viewer, because every other piece displayed in that app uses western numbers.

Could we add an option so we can switch between western and Arabic numbers for calcite-input?

@AdelheidF AdelheidF changed the title Bug: calcite-input showing Arabic characters for numbers Bug: calcite-input showing Arabic characters for numbers only, add an option to switch to western numbers Sep 24, 2021
@driskull
Copy link
Member

There's already a way in HTML to set a language for a specific element using the lang="en" tag on an element. Changing this should affect the language used within the component. If it doesn't, then we need to support that.

@AdelheidF
Copy link
Author

AdelheidF commented Oct 13, 2021

Are you saying that we should check if the currently user language is Arabic and then overwrite it with English (+RTL) instead when using input?
Are English and Arabic using the same decimal and thousand separators?

If this is the case this is not going to work, we cannot add language specific behavior everywhere. A prop that would make the input component use only English numbers in any language is OK, because the prop can stay for all languages.

@AdelheidF AdelheidF reopened this Oct 13, 2021
@driskull
Copy link
Member

@AdelheidF what i'm saying is that we don't need to add another property for this, we should use the native HTML one (lang).

If a developer wants to always use english, they can use lang="en". Otherwise, it will use whatever the HTML lang is.

The component should support whatever lang is set or inherited on the component and the browser should render numbers for that lang. If the component isn't doing that currently, it should update to do so using this issue.

Currently, calcite-input has a locale property that probably needs to be deprecated and removed.

@Prop() locale?: string = document.documentElement.lang || "en";

This prop is problematic because it only looks at the document.lang whereas it should probably look at the element first and then use closest() to get the lang.

@driskull
Copy link
Member

Looks like if you set the locale="en" on the input in Firefox its fixed. I think we just need to get rid of locale on input and support lang

@driskull
Copy link
Member

#2321

@AdelheidF
Copy link
Author

AdelheidF commented Oct 13, 2021

Sorry, I still don't understand how this should work.

Below, you can see that French has a comma decimal. If I overwrite locale (and in the future lang) with "en" I'm loosing the correct decimal separator. To make it work properly I'd have to only overwrite it if the page is Arabic.

Tested a bit using FF
image
image
image
image
image

@jcfranco could you comment here? You will run into the same issue with SymbolStyler.

@dasa
Copy link
Member

dasa commented Oct 13, 2021

Can the locale include the numbering system? This is how we override the default numbering system for ar (arab) in the API by using the locale value of ar-u-nu-latn.

@babakbolour
Copy link

@dasa which API is that? just wondering if all browsers honor this 'u' extension

@dasa
Copy link
Member

dasa commented Oct 18, 2021

Sorry, I was referring to the JS API, but yes, all browsers support the extension: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/NumberFormat/NumberFormat

@benelan benelan removed this from the Sprint 10/11 – 10/22 milestone Nov 1, 2021
@benelan
Copy link
Member

benelan commented Jun 27, 2022

@anveshmekala I put in a PR that fixes the issue

@benelan
Copy link
Member

benelan commented Jun 27, 2022

@Olga8614 @annierm18 - Can we have some clarity on this issue, we are looking at two options:

  1. Arab users see arabic numerals by default in Firefox [and Safari], and can change the numbering system to latin using a property. In Safari and Chrome it is already latin by default, and can be changed to arabic using the property.
  2. Arab users see latin numerals by default in all browsers, and they can change the numbering system to arabic using a property.

It sounds like most Esri products went with option 2, however Calcite Components also serves non-Esri users. We usually prefer to maintain browser defaults, but I'd appreciate advice on the best option here since there are mixed opinions. Thanks!

@driskull
Copy link
Member

I'd prefer to maintain browser defaults as well. We should be consistent with native HTML elements.

@AdelheidF
Copy link
Author

AdelheidF commented Jun 28, 2022

It does not make sense to have it work differently between browsers. Calcite components should be better than native HTML elements. And since most users of calcite are Esri products I prefer having the default to be Latin. All numbers appearing otherwise on the page, e.g. in text, will be Latin. Number input is a very commonly used component and we'd have to add numbering-system="latn" to all of the existing ones too. Also, this would prevent having issues when this is forgotten, which is probably a common case. I'd like the default to be Latin (option 2).

@babakbolour
Copy link

There are two sides to this! from one point of view, we need to adapt to customer environment and not force something on them. If they are used to seeing Arabic digits in their choice of browser (Firefox) with everything they do, then we probably need to honor it. on the other hand, it would be good to be consistent WITHIN Esri apps. So we may need to be consistent with JSAPI-enabled esri apps.

@driskull
Copy link
Member

It seems like we are trying to fix a firefox bug. Is there a firefox issue for this that we can maybe track and remove this logic once its fixed?

@benelan
Copy link
Member

benelan commented Jun 28, 2022

Sometimes browsers just make different choices, which is always fun. But I'll look for a FF bug.

@babakbolour - it sounds like you're saying we should go with option 1 then, and that all of the Esri teams that use the CC need to set the numberingSystem prop to "latn"?

@driskull
Copy link
Member

Sometimes browsers just make different choices, which is always fun. But I'll look for a FF bug.

Yeah, it kind of sucks but I lean toward just honoring what they return for new Intl.NumberFormat().resolvedOptions().numberingSystem.

@dasa
Copy link
Member

dasa commented Jun 28, 2022

The default for Safari is also "arab".

@AdelheidF
Copy link
Author

AdelheidF commented Jun 28, 2022

Does calcite-slider also support numbering-system?

https://codepen.io/afreitag/pen/jOapXNW

@AdelheidF
Copy link
Author

AdelheidF commented Jun 28, 2022

I noticed calcite-stepper, calcite-date-picker, and calcite-time-picker don't seem to use Arabic numbers by default in FF and should probably support numbering-system. Are there any other components that show numbers? If we decide to go with Arabic as the default for FF they should probably be all the same. Though I still very much prefer a calcite default that's the same across all browsers.

https://codepen.io/afreitag/pen/jOapXNW

image

@anveshmekala
Copy link
Contributor

To provide more clarity, the default numbering system for locale='ar' is Arabic in Firefox and Safari. CLDR lists arabic-indic digits as default for locale='ar'. Chrome is the browser that had overwritten the numbering system for Arabic locale to be latin. Please refer to the comments section in https://bugs.chromium.org/p/chromium/issues/detail?id=1194438#:~:text=I%20talk%20to,stick%20with%20Latn.

@anveshmekala
Copy link
Contributor

CLDR reference for arabic https://icu4c-demos.unicode.org/icu-bin/locexp?d_=en&_=ar

@jcfranco
Copy link
Member

I agree that we should try to align whenever possible with browser defaults, but it doesn't mean we can't make adjustments to make it easier for our users (e.g., calcite-button's default button type is button vs submit). WRT the defaults, I think we should provide one for consistency and document it clearly. Otherwise, we are forcing users to be aware of this and (IMO unnecessarily), set it. We could then revisit this approach if browsers align on a default in the future.

@benelan benelan added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Jul 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Installed and assigned for verification.

@benelan
Copy link
Member

benelan commented Jul 8, 2022

Alright, thanks for your patience everyone. The default numbering system for arabic is now latin in all browsers. We will be releasing beta.84 on Monday which will include this change. Users can still use the arab numbering system like:

<calcite-input numbering-system="arab" type="number" value="5"></calcite-input>

@benelan benelan removed their assignment Jul 8, 2022
@AdelheidF
Copy link
Author

calcite-input works now with next.515, will someone take care of calcite-slider too?

image

@jcfranco
Copy link
Member

jcfranco commented Jul 9, 2022

I thought we had already created follow-up issues for this. 🥲 @benelan can we create an issue to update additional components that display numbers? Aside from calcite-slider, we may also need to update calcite-alert. There may be others.

@AdelheidF Thanks for the reminder. 🏆

@benelan
Copy link
Member

benelan commented Jul 9, 2022

new issue: #4893

@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Jul 15, 2022
@geospatialem
Copy link
Member

Verified the default numbering system for Arabic is Latin across browsers, and when using the numberingSystem prop can set to "arab" on beta.86. 👍🏻

Follow-up for the numberingSystem prop for alert, slider, stepper, date-picker, and time-picker will be relayed in #4893.

@Olga8614
Copy link

Olga8614 commented Jun 1, 2023

cc @eriklharper

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. i18n-l10n issues dealing with internationalization/localization p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests