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

[EuiSuperDatePicker] Ensure i18n coverage #5398

Closed
1 of 2 tasks
Bamieh opened this issue Nov 22, 2021 · 7 comments · Fixed by #5743
Closed
1 of 2 tasks

[EuiSuperDatePicker] Ensure i18n coverage #5398

Bamieh opened this issue Nov 22, 2021 · 7 comments · Fixed by #5743
Assignees

Comments

@Bamieh
Copy link
Member

Bamieh commented Nov 22, 2021

Hello,

The date picker component we use in Kibana is not fully internationalized. The coverage of this component varies between releases (elastic/kibana#63742)

image
image
image
image

Context: (elastic/kibana#63742 (comment))

@thompsongl
Copy link
Contributor

thompsongl commented Nov 22, 2021

Thanks, @Bamieh!
Now that #5339 is merged we'll have even better control over text strings and instructions

Relates to #3901

@cchaos cchaos added this to the Elastic Stack 8.1 milestone Nov 23, 2021
@cchaos
Copy link
Contributor

cchaos commented Nov 23, 2021

This will probably get covered by #5399. I think all those places mentioned are specific to EuiSuperDatePicker not the plain EuiDatePicker.

@chandlerprall
Copy link
Contributor

#5399 doesn't catch this one 😞, the mentioned edge case applies in this situation,

For example, you can do something like and it will be allowed as only children are verified, and inside MyComponent the rule will see customLabel comes from a prop and assume it's safe.

For many of these we could extend the lint rule to specify component+prop pairs to also validate (and would need to locate those situations so we can configure them), but that wouldn't catch these tabs either.

For this setup, https://github.com/elastic/eui/blob/main/src/components/date_picker/super_date_picker/date_popover/date_popover_content.tsx#L60

const renderTabs = [
    {
      id: DATE_MODES.ABSOLUTE,
      name: 'Absolute',
      content: (
        <EuiAbsoluteTab
          dateFormat={dateFormat}
          timeFormat={timeFormat}
          locale={locale}
          value={value}
          onChange={onChange}
          roundUp={roundUp}
          position={position}
          utcOffset={utcOffset}
        />
      ),
      'data-test-subj': 'superDatePickerAbsoluteTab',
      'aria-label': `${ariaLabel} Absolute`,
    },
    ...

    <EuiTabbedContent
      className="euiDatePopoverContent"
      tabs={renderTabs}
      ...

we would need an understanding of where tabs[number].name originates. This might be doable through some configurations, but would definitely need to be explored.

@pgayvallet
Copy link

pgayvallet commented Mar 18, 2022

We just received feedback from a customer using the ja_JP locale that some translations are missing from the date picker.

Screenshot 2022-03-18 at 09 29 16

Looking at the code

https://github.com/elastic/eui/blob/main/src/components/date_picker/super_date_picker/quick_select_popover/quick_select.tsx#L32-L38

It's not just missing i18n keys in Kibana's i18n_eui_mapping.tsx file, as the labels are currently hardcoded in EUI.

@Bamieh
Copy link
Member Author

Bamieh commented Mar 18, 2022

++ It would be great if we can prioritize this (even if it was not a full-proof solution for now). especially since we've added French locale in Kibana for the 8.2 release and the EuiSuperDatepicker is a super visible component in kibana

@cee-chen
Copy link
Member

Heya @Bamieh! It would be awesome if you could take a quick peek at the before/after screenshots in #5743 and confirm that the upcoming strings available for translations (the ones that are using weird babelfish characters) are all correct. In particular I just want to verify that the number inputs do not need to be translated and are fine remaining as is.

Thank you!

@Bamieh
Copy link
Member Author

Bamieh commented Mar 24, 2022

Hey @constancecchen thank you for the follow up! it looks good I only have 1 concern I raised in the PR otherwise LGTM :elasticheart:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants