-
Notifications
You must be signed in to change notification settings - Fork 98
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
base: main
Are you sure you want to change the base?
Conversation
Preview is ready. |
Visual Tests Report is ready. |
}; | ||
|
||
function validateCustomKeysets(language: string, data: KeysetData): void { | ||
const trustedData = i18n.data.en; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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 {Lang, configure} from './configure'; | ||
import {registerCustomKeysets} from './registerCustomKeysets'; | ||
|
There was a problem hiding this comment.
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
Closes #1065