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

feat: add function to register custom keysets [WIP] #1844

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

NikitaShkaruba
Copy link

Closes #1065

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@gravity-ui-bot
Copy link
Contributor

Visual Tests Report is ready.

@NikitaShkaruba NikitaShkaruba changed the title feat: add function to register custom keysets feat: add function to register custom keysets [WIP] Sep 9, 2024
src/components/utils/configure.ts Outdated Show resolved Hide resolved
src/components/utils/configure.ts Outdated Show resolved Hide resolved
src/components/__tests__/i18n.test.tsx Outdated Show resolved Hide resolved
};

function validateCustomKeysets(language: string, data: KeysetData): void {
const trustedData = i18n.data.en;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We provide complete keysets in English language. So we can compare custom keysets with it.

The code below looks kinda verbose, but IDK how to improve it, and it's very reliable and transparent about what needs to be fixed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think creating your first custom keyset might be daunting, because you'll only get a error one at a time. So maybe in our errors we should also show the templated English keyset. In the end our errors will look like this:

Custom keysets 'it' validation error: excess component 'Alert' keys for found ["nonexistent_key","nonexistent_key2"]

English keysets example:
{"Alert":{"label_close":"Close"},"Breadcrumbs":{"label_more":"Show more"},"ClipboardButton":{"startCopy":"Copy","endCopy":"Copied!"},"Dialog":{"close":"Close dialog"},"AvatarStack":{"more":["and {{count}} more","and {{count}} more","and {{count}} more"]},"g-clear-button":{"label_clear-button":"Clear"},"Pagination":{"button_previous":"Previous","button_next":"Next","button_first":"First","label_input-placeholder":"Page #","label_page-of":"of","label_select_size":"Select page size"},"Select":{"label_clear":"Clear","label_show-error-info":"Show popup with error info"},"g-user-label":{"label_remove-button":"Remove"},"PinInput":{"label_one-of":"{{number}} of {{count}}, "},"Table":{"label_empty":"No data","label-actions":"Actions","label-row-select":"Select"},"TableColumnSetupInner":{"button_apply":"Apply","button_reset":"Reset","button_switcher":"Columns"},"withTableSettings":{"label_settings":"Table settings"},"TableColumnSetup":{"button_switcher":"Columns"},"Toaster":{"label_close-button":"Close"}}

What do you think?

import {i18n} from '../../i18n';

export const registerCustomKeysets = (language: string, data: KeysetData) => {
validateCustomKeysets(language, data);
Copy link
Author

@NikitaShkaruba NikitaShkaruba Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've decided to throw errors on startup, because this way developers will not have weird translation errors in their production, when we'll add new key to our component in a new major release for example. I think that their CI or dev environment manual tests will easily catch it.

What do you guys, think?

export enum Lang {
Ru = 'ru',
En = 'en',
}

interface Config {
lang: `${Lang}`;
lang: `${Lang}` | AutocompleteSafeString;
Copy link
Author

@NikitaShkaruba NikitaShkaruba Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is concerning, but is needed for selecting custom keysets via configure. I've read all the codes, and I think it's alright. But what do you think, guys?

};

function validateCustomKeysets(language: string, data: KeysetData): void {
const trustedData = i18n.data.en;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I think creating your first custom keyset might be daunting, because you'll only get a error one at a time. So maybe in our errors we should also show the templated English keyset. In the end our errors will look like this:

Custom keysets 'it' validation error: excess component 'Alert' keys for found ["nonexistent_key","nonexistent_key2"]

English keysets example:
{"Alert":{"label_close":"Close"},"Breadcrumbs":{"label_more":"Show more"},"ClipboardButton":{"startCopy":"Copy","endCopy":"Copied!"},"Dialog":{"close":"Close dialog"},"AvatarStack":{"more":["and {{count}} more","and {{count}} more","and {{count}} more"]},"g-clear-button":{"label_clear-button":"Clear"},"Pagination":{"button_previous":"Previous","button_next":"Next","button_first":"First","label_input-placeholder":"Page #","label_page-of":"of","label_select_size":"Select page size"},"Select":{"label_clear":"Clear","label_show-error-info":"Show popup with error info"},"g-user-label":{"label_remove-button":"Remove"},"PinInput":{"label_one-of":"{{number}} of {{count}}, "},"Table":{"label_empty":"No data","label-actions":"Actions","label-row-select":"Select"},"TableColumnSetupInner":{"button_apply":"Apply","button_reset":"Reset","button_switcher":"Columns"},"withTableSettings":{"label_settings":"Table settings"},"TableColumnSetup":{"button_switcher":"Columns"},"Toaster":{"label_close-button":"Close"}}

What do you think?

@NikitaShkaruba
Copy link
Author

The "backend" looks solid. Let's talk about "frontend".

I see 2 solutions:

  1. We can add a custom language to the header selector in storybook, and to have it supported via registerCustomKeysets.
image Something like `It [custom] 🇮🇹` or `Fr [custom] 🇫🇷 `, because those languages are beautiful. Also we might add `Rs [custom] 🇷🇸`, because i've seen it in some tests.
  1. We can add a separate header to the left panel, and let the user create custom keysets here. Something like this.
image


import {Lang, configure} from './configure';
import {registerCustomKeysets} from './registerCustomKeysets';

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok, that I don't have describe, or should I encapsulate everything in it? I haven't done it, because jest splits tests by files anyway

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

Successfully merging this pull request may close these issues.

[RFC] Providing own locales for built-in texts
3 participants