commercetools / ui-kit

Component library πŸ’…
https://uikit.commercetools.com
MIT License
145 stars 25 forks source link

DateTime input placeholders show hardcoded date/time format #2074

Closed emmenko closed 2 years ago

emmenko commented 2 years ago

Describe the bug The default placeholder value for DateTime input uses a hardcoded date/time format message.

https://github.com/commercetools/ui-kit/blob/main/packages/components/inputs/date-time-input/src/messages.ts#L4-L13

Expected behavior

CarlosCortizasCT commented 2 years ago

As I'm already trying to understand how the i18n works in all the projects, there's something I don't understand here. The component has defined a default messages for the placeholders as mentioned in this issue, but it also has defined the values for this placeholders in the languages files:

So I was expecting that the right placeholder would be used based on the locale of the user because it will already have loaded translations for that locale.

What am I missing here?

emmenko commented 2 years ago

Yes good question. Locales usually consist of a language and optionally a country code.

en
en-US
en-GB

de
de-DE
de-AT

In your Merchant Center user profile you can select a local/language that also contains a more specific country code.

image

This locale information is then used throughout the application in different ways:

image

Therefore, having a hardcoded value does not make much sense as the format might vary within the same language.

CarlosCortizasCT commented 2 years ago

Thanks for the explanation.

I was confused because the way I'm used to handle locale resolution fallbacks so if no translation is available for the complete locale (ex: de-CH) it would the use the language only one if available (ex: de). This mean you need to have translations defined for all the allowed combinations, which I assume it is something we don't want in our scenario.

That said, I'm still wondering where should be define the placeholder values as I don't find how to get them in the runtime (I mean, if we want to show the users values such as MM/DD/YYYY - HH:mm AM/PM and HH:mm AM/PM). Or, otherwise, we want to show a specific date (maybe today) as a placeholder? (ex: 26/01/2022 - 08:56 AM)

emmenko commented 2 years ago

Yes when it comes to translations it falls back to the language code.

As for getting the format date, I think we should use momentjs (for example with the creationData API https://momentjs.com/docs/#/parsing/creation-data/).

CarlosCortizasCT commented 2 years ago

I still don't get how we can resolve the formats as they are translated. I mean, what we need to show to the user are abbreviations of words composing the format. In english D stands for day, but in german we should use T from tag.

Reading the moment documentations I see we can get the format, but it is always using the same letters (always D for day no matter the locale used) as it is not a translated format.

These are the texts we should be showing to our users for the long date-time format (based on the current translations we have): Locale Value
de TT.MM.JJJJ - SS:mm
en MM/DD/YYYY - HH:mm AM/PM
es DD/MM/AAAA - HH:mm
fr-FR JJ/MM/AAAA - HH:mm
ja εΉ΄/月/ζ—₯ - εˆε‰/午後 ζ™‚:εˆ†
zh-CN εΉ΄/月/ζ—₯ - ε°ζ—ΆοΌšεˆ†ι’Ÿ

I found we could get to moment's date-time formats with this API. If I try to get formats with it, these are the results:

image

These don't match our expectations so I'm still wondering how we can get the right values without translations.

Maybe I'm overcomplicating things here, but just want to be sure how we should proceed.

emmenko commented 2 years ago

Thanks for looking into it.

From my side the issue is more about the actual date format (for example using / / or . . or the order of day/month/year) rather then the "translation" itself.

Given that D=day, M=month, Y=year is the de-facto international format, I would just stick to that nomenclature, regardless of the "correct" day/month letters, as long as the format itself is matching the user language+country code.

What do you think?

CarlosCortizasCT commented 2 years ago

Well, from my point of view, I think it is more useful for users if we can provide the translated pattern message although it means for us to manage more translations files (only pattern values needed). I'm not sure if users using chinese, arabian or russian languages actually have D, M, and Y as standard. But maybe this is complicating things more than actually is needed.

But reading again the problem that originated this issue:

a user based in England has their dates correctly localized when a value is present, but otherwise their placeholder defaults to the US formatting of MM/DD/YYYY over DD/MM/YYYY.

if the user has selected the language en in the Merchant Center profile setting, I don't see how can we show the right format (british or american) either way: nor with the moment API, nor with the messages. I mean, the user would need to select a more specific language(en-GB or en-US) for us to be able to show the right format anyway (maybe we should not provide the en options at all).

emmenko commented 2 years ago

If you only select en, it's probably the same as en-US from an Intl point of view.

However, if you select something like en-GB, then things do actually differ

image

What I would expect here with en-GB selected is to show the format DD/MM/YYYY, as opposed to MM/DD/YYYY when en is selected.

CarlosCortizasCT commented 2 years ago

Sure. I just wanted to point that the user based in England will need to select en-GB anyway (I'm assuming now he selected only en).

If we go for the moment option I guess we would need to check the components using placeholder patterns (maybe DateInput, DateRangeInput and DateTimeInput) and have them use a new function to resolve the value based on user's locale.

If we go for the translations option, that would require to add a new i18n/data/en-GB.json file with this content (more files to come if needed for other languages):

"UIKit.DateInput.placeholder": "DD/MM/YYYY",
"UIKit.DateRangeInput.placeholder": "DD/MM/YYYY - DD/MM/YYYY",
"UIKit.DateTimeInput.placeholder": "DD/MM/YYYY - HH:mm AM/PM"

I would be happy to work on this once the path to follow it's decided.

emmenko commented 2 years ago

FYI: as discussed, waiting for UX feedback on how to proceed. cc @FilPob

briantomkins commented 2 years ago

@emmenko can we get an update here? Please note that currently en-AU and en-NZ should also be DD/MM/YYYY only US territories use MM/DD for dates in english.

mmaltsev-ct commented 2 years ago

Reviewed the progress. To be done:

The work is to be continued in the following branch: https://github.com/commercetools/ui-kit/commit/9ca7646ab214e792e26aec7d80c81e0dfb89fcad

mmaltsev-ct commented 2 years ago

Link to the original support ticket: https://jira.commercetools.com/browse/SUPPORT-15104

CarlosCortizasCT commented 2 years ago

This issue has been addressed with the already closed PR.

It will be generally available with the next UI-Kit release.

briantomkins commented 2 years ago

HI @CarlosCortizasCT what are the next steps to have this fix adopted across the merchant centre apps? Including the

CarlosCortizasCT commented 2 years ago

Hi @briantomkins

So there is already a new public release for ui-kit which includes this change (v15.0.2) and now there is some ongoing work on updating the Merchant Center apps to this new version.

emmenko commented 2 years ago

I think this can be closed now.