Esri / calcite-design-system

A monorepo containing the packages for Esri's Calcite Design System
https://developers.arcgis.com/calcite-design-system/
Other
290 stars 75 forks source link

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

Closed AdelheidF closed 2 years ago

AdelheidF commented 3 years ago

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

github-actions[bot] commented 3 years ago

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.

benelan commented 3 years ago

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

julio8a commented 3 years ago

@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: https://github.com/Esri/calcite-components/issues/2765#issuecomment-926079026

AdelheidF commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

babakbolour commented 3 years ago

@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 commented 3 years ago

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?

driskull commented 3 years ago

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 commented 3 years ago

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.

driskull commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

https://github.com/Esri/calcite-components/issues/2321

AdelheidF commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

dasa commented 3 years ago

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

Olga8614 commented 2 years ago

Another bug reported for the mixture of Western and Indic numerals: https://github.com/ArcGIS/I18N-MOR-Bugs/issues/2401

eriklharper commented 2 years ago

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.

This is what I would try first before exploring any kind of numbering system override prop. Because input uses the browser's Intl.NumberFormat api under the hood, passing in a locale string for the lang prop that specifies the numbering system should work and would be the way I'd recommend achieving something like this. You can use a template string in your JS to still include the passed-in lang by writing something like

<calcite-input lang="${document.lang}-u-nu-latn">. (insert backticks around the template string, darn Github formatting)

AdelheidF commented 2 years ago

adding that lang="ar-u-nu-latn" doesn't seem to fix it in FF

eriklharper commented 2 years ago

Use locale instead of lang and it should work. Input hasn't been updated to use lang yet so you still have to set it with locale.

This example shows the default Arabic display (first one) and Arabic with latin numbering (second one) in Firefox using the locale prop:

image
eriklharper commented 2 years ago

Based on this, it seems like we can close this issue since western numbers can be set in the locale string. What do you think @AdelheidF @jcfranco @babakbolour ?

jcfranco commented 2 years ago

Can the locale include the numbering system?

@dasa I was not aware of this. This is awesome, thanks for sharing!

Based on this, it seems like we can close this issue since western numbers can be set in the locale string. What do you think

@eriklharper Agreed.

Sidebar: I'll create a separate issue to update calcite-input to use lang. It'd be good to do this for consistency.

AdelheidF commented 2 years ago

OK, adding locale="ar-u-nu-latn" does work, but I really don't think it's good that we expect every client to now check the user's locale and add that prop only for ar to every calcite-input box. Same issue if we change the prop to lang. It should have at least a prop that can stay the same for all locales. Something like latin-numbers=true, and I'd prefer this be the default because it's how we get the same behavior across browsers.

Anyway, calcite-color-picker, e.g., has to make this change too. image

dasa commented 2 years ago

I really don't think it's good that we expect every client to now check the user's locale and add that prop only for ar to every calcite-input box

You could add -u-nu-latn to every locale, but I agree that it'd be nicer if this wan't necessary and the API covered up that the browsers don't have the same default for the ar locale.

eriklharper commented 2 years ago

One solution could be to update your arabic entry in that list to use ar-u-nu-latn instead of just plain ar. I'm guessing that this might not work because you are relying on the html lang="ar" attribute which wouldn't match the name in the json, but if the json file encoded a key/value pair you could store the key as ar which would match the html lang attribute and then have the value set to ar-u-nu-latn:

{ key: "ar", value: "ar-u-nu-latn" }

You may not have it set up this way, but if so, this could work.

I can see the benefit in adding a number-system property, I can see that being useful.

AdelheidF commented 2 years ago

We already have an issue for calcite-color-picker: https://github.com/Esri/calcite-components/issues/2476

github-actions[bot] commented 2 years ago

Installed and assigned for verification.

AdelheidF commented 2 years ago

How was this solved now? Is there a new prop? And what do I have to pass in to get English numbers for Arabic? Can I pass this in for all locals or do I have to do something just for Arabic? What is the default in FF and Chrome?

anveshmekala commented 2 years ago

we added a prop numbering-system for calcite-input . To get english numbers for a given locale use numbering-system=latn . Numbering System is specific to locale and to set english numbers for a given locale you need to append the attribute given default is not latn. Default values are set by CLDR.

geospatialem commented 2 years ago

Verified in Firefox with next.

verified in next

AdelheidF commented 2 years ago

@geospatialem Hi, could I see your test app? How did you get color-picker to use the Latin numbering-system with FF? I also don't see it implemented here: https://github.com/Esri/calcite-components/blob/master/src/components/color-picker/color-picker.tsx#L1014 I think color-picker still needs the draft PR from here https://github.com/Esri/calcite-components/issues/2476

benelan commented 2 years ago

It looks fixed to me by bumping the sample you provided to beta.83 - https://jsbin.com/dutarurohu/edit?html,output

The draft PR is something I recommended which will allow users to set the numberingSystem prop on color-picker which will pass it through to the internal input. Based on the UI screenshots in #2476, it looked like there may be a need to set locale and numbering system separately, so that PR will help there. But using the default numberingSystem should fix this particular issue - https://github.com/Esri/calcite-components/blob/dc475ead3da98356247156a3b316b320770b4196/src/utils/locale.ts#L58

AdelheidF commented 2 years ago

Oh, that's great. So the default is Latin numbers, I thought that wasn't in the plan originally.

All good then, thanks.

babakbolour commented 2 years ago

Reading the above, it looks great. I assume someone will write up the how-to in the documentation, for the customers. or do we need a KB? if later, maybe Aine in my team can help.

benelan commented 2 years ago

Once the #4801 merges and the next beta release happens, the numberingSystem prop will be documented in the color-picker component reference. Is that what you are referring to?

babakbolour commented 2 years ago

@benelan yes, assuming this solution is for color-picker only (and not for calcite inout or other components)

benelan commented 2 years ago

Input already has the prop documented here: https://developers.arcgis.com/calcite-design-system/components/input/#component-api-properties-numberingSystem

color-picker uses input internally, so essentially we are adding the same prop to color-picker and then passing that value to the input. After the next beta release, the numberingSystem prop will be documented for color-picker as well. As of now, there is no way to specify the numbering system for color-picker without going into the shadowRoot. However, internally setting the default numbering system to latin fixed this issue in the meantime.

dasa commented 2 years ago

Was this verified in Firefox when navigator.language started with "ar-"?

AdelheidF commented 2 years ago

tried with navigator.language = "en-US"

<html lang="ar" dir="rtl">
...
        <calcite-input type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input locale="ar-u-nu-latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input numbering-system="latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>

beta.82 image

beta.83 image

<html lang="ar-u-nu-latn" dir="rtl">
...
        <calcite-input type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input locale="ar-u-nu-latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input numbering-system="latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>

beta.82 image

beta.83 image

then I changed the browser language to Arabic (navigator.language="ar")

<html dir="rtl">
...
        <calcite-input type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input locale="ar-u-nu-latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input numbering-system="latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>

beta.82 image

beta.83 image

-> I would have expected all 3 input boxes to show Latin numbers with beta.83

<html lang="ar" dir="rtl">
...
        <calcite-input type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input locale="ar-u-nu-latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>
        <calcite-input numbering-system="latn" type="number" min="1" max="5" value="5" scale="s" step="1">
        </calcite-input>

beta.82 image

beta.83 image

-> I would have expected all 3 input boxes to show Latin numbers with beta.83

I'm not sure how to set the navigator language to "ar-...".

dasa commented 2 years ago

I'm not sure how to set the navigator language to "ar-...".

That's fine, I was assuming you would have a country code too.

benelan commented 2 years ago

@AdelheidF - I'm not able to repro the last two screenshots in your comment, can you provide a sample? -https://codepen.io/benesri/pen/KKoPExR?editors=1000 image

benelan commented 2 years ago

@dasa In firefox I can set navigator.language to ar- and it works for me in https://codepen.io/benesri/pen/KKoPExR?editors=1000 image

AdelheidF commented 2 years ago

I have no idea why it's working differently when I try in FF with ar language settings. If it's just me, it's OK.

image

image

benelan commented 2 years ago

Huh, weird. I am on version FF 101.0.1 as well

I only changed this option: image

It looks like you also changed this one which may be why we are seeing different results: image

So maybe the browser language setting takes precedent over navigator.language? Not sure what we can do about that.

AdelheidF commented 2 years ago

That's correct, but if I change both of those settings to ar I still see the same thing. I would think this is what true ar users would see too then.

image

benelan commented 2 years ago

Yeah it looks like we will need to map ar to ar-u-nu-latn, which is what the JSAPI does.

anveshmekala commented 2 years ago

The issue is with the default we set in utils for the number formatter and it ignores the ar-u-nu-latn convention. Removing the default would fix the issue for a scenario where the navigator.language = 'ar' and the locale='ar-u-nu-latn' .

And the below example will still return Arabic numbers which are expected.

<html lang="ar"> 
  <calcite-input type="number" min="1" max="5" value="5" scale="s" step="1">
</html>
benelan commented 2 years ago

@anveshmekala I put in a PR that fixes the issue