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: Print a warning when both I18nProvider and own i18n string are missing #3172

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pages/alert/permutations.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const longText =
const longTextWithLink = (
<>
Lorem ipsum dolor sit amet, <Link href="#">consectetur</Link> adipisicing{' '}
<Link external={true} href="#">
<Link external={true} href="#" externalIconAriaLabel="(opens in new tab)">
elit
</Link>
, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud
Expand Down
1 change: 1 addition & 0 deletions pages/app-layout/utils/content-blocks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import styles from '../styles.scss';
export function Breadcrumbs() {
return (
<BreadcrumbGroup
expandAriaLabel="Show path"
items={[
{ text: 'Home', href: '#' },
{ text: 'Service', href: '#' },
Expand Down
2 changes: 1 addition & 1 deletion pages/key-value-pairs/permutations.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const permutations = createPermutations<KeyValuePairsProps>([
{
label: 'CNAMEs',
value: (
<Link external={true} href="#">
<Link external={true} href="#" externalIconAriaLabel="(opens in new tab)">
abc.service23G24.xyz
</Link>
),
Expand Down
12 changes: 12 additions & 0 deletions pages/steps/permutations-utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ export const loadingStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -357,6 +358,7 @@ export const loadingSteps2Interactive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand All @@ -382,6 +384,7 @@ export const loadingSteps2Interactive: ReadonlyArray<StepsProps.Step> = [
Gathered Security Group IDs:{' '}
<Popover
header={'Security Group IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -414,6 +417,7 @@ export const loadingSteps3Interactive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand All @@ -439,6 +443,7 @@ export const loadingSteps3Interactive: ReadonlyArray<StepsProps.Step> = [
Gathered Security Group IDs:{' '}
<Popover
header={'Security Group IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -476,6 +481,7 @@ export const successfulStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand All @@ -501,6 +507,7 @@ export const successfulStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Gathered Security Group IDs:{' '}
<Popover
header={'Security Group IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -538,6 +545,7 @@ export const blockedStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand All @@ -563,6 +571,7 @@ export const blockedStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Gathered Security Group IDs:{' '}
<Popover
header={'Security Group IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -601,6 +610,7 @@ export const failedStepsInteractive: ReadonlyArray<StepsProps.Step> = [
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -634,6 +644,7 @@ export const failedStepsWithRetryTextInteractive: ReadonlyArray<StepsProps.Step>
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down Expand Up @@ -672,6 +683,7 @@ export const failedStepsWithRetryButtonInteractive: ReadonlyArray<StepsProps.Ste
Listed EC2 instances:{' '}
<Popover
header={'EC2 Instance IDs'}
dismissAriaLabel="Close"
content={
<Box fontSize="body-s">
<ul>
Expand Down
6 changes: 2 additions & 4 deletions src/autosuggest/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ const InternalAutosuggest = React.forwardRef((props: InternalAutosuggestProps, r
);

const i18n = useInternalI18n('autosuggest');
const errorIconAriaLabel = i18n('errorIconAriaLabel', restProps.errorIconAriaLabel);
const selectedAriaLabel = i18n('selectedAriaLabel', restProps.selectedAriaLabel);
const recoveryText = i18n('recoveryText', restProps.recoveryText);

if (restProps.recoveryText && !onLoadItems) {
warnOnce('Autosuggest', '`onLoadItems` must be provided for `recoveryText` to be displayed.');
Expand Down Expand Up @@ -188,8 +186,8 @@ const InternalAutosuggest = React.forwardRef((props: InternalAutosuggestProps, r
...props,
isEmpty,
isFiltered,
recoveryText,
errorIconAriaLabel,
getRecoveryText: () => i18n('errorIconAriaLabel', restProps.errorIconAriaLabel),
getErrorIconAriaLabel: () => i18n('recoveryText', restProps.recoveryText),
Copy link
Member Author

Choose a reason for hiding this comment

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

Make recovery text and error label lazy-loadable, only when this feature is used

Comment on lines +189 to +190
Copy link
Member

Choose a reason for hiding this comment

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

The property names seem to have gotten mixed up here

Suggested change
getRecoveryText: () => i18n('errorIconAriaLabel', restProps.errorIconAriaLabel),
getErrorIconAriaLabel: () => i18n('recoveryText', restProps.recoveryText),
getRecoveryText: () => i18n('recoveryText', restProps.recoveryText),
getErrorIconAriaLabel: () => i18n('errorIconAriaLabel', restProps.errorIconAriaLabel),

onRecoveryClick: handleRecoveryClick,
filteringResultsText: filteredText,
hasRecoveryCallback: !!onLoadItems,
Expand Down
10 changes: 6 additions & 4 deletions src/cards/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,12 @@ const Cards = React.forwardRef(function <T = any>(
selectionType,
isItemDisabled,
onSelectionChange,
ariaLabels: {
itemSelectionLabel: ariaLabels?.itemSelectionLabel,
selectionGroupLabel: i18n('ariaLabels.selectionGroupLabel', ariaLabels?.selectionGroupLabel),
},
ariaLabels: selectionType
? {
itemSelectionLabel: ariaLabels?.itemSelectionLabel,
selectionGroupLabel: i18n('ariaLabels.selectionGroupLabel', ariaLabels?.selectionGroupLabel),
}
: {},
});
const hasToolsHeader = header || filter || pagination || preferences;
const hasFooterPagination = isMobile && variant === 'full-page' && !!pagination;
Expand Down
2 changes: 1 addition & 1 deletion src/form/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export default function InternalForm({
}: InternalFormProps) {
const baseProps = getBaseProps(props);
const i18n = useInternalI18n('form');
const errorIconAriaLabel = i18n('errorIconAriaLabel', errorIconAriaLabelOverride);
const errorIconAriaLabel = errorText ? i18n('errorIconAriaLabel', errorIconAriaLabelOverride) : undefined;
const analyticsComponentMetadata: GeneratedAnalyticsMetadataFormFragment = {
component: {
name: 'awsui.Form',
Expand Down
21 changes: 20 additions & 1 deletion src/i18n/__tests__/i18n.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,20 @@
import React from 'react';
import { render } from '@testing-library/react';

import { clearMessageCache } from '@cloudscape-design/component-toolkit/internal/testing';

import { I18nProvider, I18nProviderProps } from '../../../lib/components/i18n';
import { MESSAGES, TestComponent } from './test-component';
import { MESSAGES, SimpleTestComponent, TestComponent } from './test-component';

beforeEach(() => {
clearMessageCache();
jest.spyOn(console, 'warn').mockImplementation(() => {});
});

afterEach(() => {
expect(console.warn).not.toHaveBeenCalled();
jest.restoreAllMocks();
});

describe('with custom "lang" on <html>', () => {
afterEach(() => {
Expand Down Expand Up @@ -145,3 +157,10 @@ it('allows nesting providers', () => {
expect(container.querySelector('#top-level-string')).toHaveTextContent('My custom string');
expect(container.querySelector('#nested-string')).toHaveTextContent('nested string');
});

it('prints a warning when a string is not provided neither via prop nor I18nProvider', () => {
render(<SimpleTestComponent />);
expect(console.warn).toHaveBeenCalledTimes(1);
expect(console.warn).toHaveBeenCalledWith(expect.stringMatching(/topLevelString.*I18nProvider/));
jest.mocked(console.warn).mockReset();
});
6 changes: 6 additions & 0 deletions src/i18n/__tests__/test-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,9 @@ export function TestComponent(props: TestComponentProps) {
</ul>
);
}

export function SimpleTestComponent(props: TestComponentProps) {
const i18n = useInternalI18n('test-component');

return <span id="top-level-string">{i18n('topLevelString', props.topLevelString)}</span>;
}
13 changes: 12 additions & 1 deletion src/i18n/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

import React, { useContext } from 'react';

import { warnOnce } from '@cloudscape-design/component-toolkit/internal';

import { isDevelopment } from '../internal/is-development';
import { I18nFormatArgTypes } from './messages-types';

export type CustomHandler<ReturnValue, FormatFnArgs> = (formatFn: (args: FormatFnArgs) => string) => ReturnValue;
Expand All @@ -20,7 +23,15 @@ interface InternalI18nContextProps {

export const InternalI18nContext = React.createContext<InternalI18nContextProps>({
locale: null,
format: <T>(_namespace: string, _component: string, _key: string, provided: T) => provided,
format: <T>(_namespace: string, component: string, key: string, provided: T) => {
if (isDevelopment && !provided) {
warnOnce(
component,
`Localization is not provided for key ${key}. Provide the value as a prop or use I18nProvider`
);
Comment on lines +28 to +31
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the change. Everything else is to fix appeared warnings

}
return provided;
},
});

export function useLocale(): string | null {
Expand Down
28 changes: 9 additions & 19 deletions src/internal/components/dropdown-status/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import styles from './styles.css.js';

export { DropdownStatusProps };

export interface DropdownStatusPropsExtended extends DropdownStatusProps {
export interface DropdownStatusPropsExtended extends Omit<DropdownStatusProps, 'recoveryText' | 'errorIconAriaLabel'> {
isEmpty?: boolean;
isNoMatch?: boolean;
isFiltered?: boolean;
Expand All @@ -30,27 +30,16 @@ export interface DropdownStatusPropsExtended extends DropdownStatusProps {
* in case recoveryText was automatically provided by i18n.
*/
hasRecoveryCallback?: boolean;

getErrorIconAriaLabel: () => string | undefined;
getRecoveryText: () => string | undefined;
}

function DropdownStatus({ children }: { children: React.ReactNode }) {
return <div className={styles.root}>{children}</div>;
}

type UseDropdownStatus = ({
statusType,
empty,
loadingText,
finishedText,
filteringResultsText,
errorText,
recoveryText,
isEmpty,
isNoMatch,
isFiltered,
noMatch,
hasRecoveryCallback,
onRecoveryClick,
Comment on lines -40 to -52
Copy link
Member Author

Choose a reason for hiding this comment

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

There is no point to destructure the object in a type declaration

}: DropdownStatusPropsExtended) => DropdownStatusResult;
type UseDropdownStatus = (statusProps: DropdownStatusPropsExtended) => DropdownStatusResult;

export interface DropdownStatusResult {
isSticky: boolean;
Expand All @@ -65,29 +54,30 @@ export const useDropdownStatus: UseDropdownStatus = ({
finishedText,
filteringResultsText,
errorText,
recoveryText,
getRecoveryText,
isEmpty,
isNoMatch,
isFiltered,
noMatch,
onRecoveryClick,
hasRecoveryCallback = false,
errorIconAriaLabel,
getErrorIconAriaLabel,
}): DropdownStatusResult => {
const previousStatusType = usePrevious(statusType);
const statusResult: DropdownStatusResult = { isSticky: true, content: null, hasRecoveryButton: false };

if (statusType === 'loading') {
statusResult.content = <InternalStatusIndicator type={'loading'}>{loadingText}</InternalStatusIndicator>;
} else if (statusType === 'error') {
const recoveryText = getRecoveryText();
statusResult.hasRecoveryButton = !!recoveryText && hasRecoveryCallback;
statusResult.content = (
<span>
<InternalStatusIndicator
type="error"
__display="inline"
__animate={previousStatusType !== 'error'}
iconAriaLabel={errorIconAriaLabel}
iconAriaLabel={getErrorIconAriaLabel()}
>
{errorText}
</InternalStatusIndicator>{' '}
Expand Down
2 changes: 1 addition & 1 deletion src/link/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ const InternalLink = React.forwardRef(
sharedProps['aria-labelledby'] = `${sharedProps.id} ${infoId} ${infoLinkLabelFromContext}`;
}

const renderedExternalIconAriaLabel = i18n('externalIconAriaLabel', externalIconAriaLabel);
const renderedExternalIconAriaLabel = external ? i18n('externalIconAriaLabel', externalIconAriaLabel) : undefined;
const content = (
<>
{children}
Expand Down
3 changes: 0 additions & 3 deletions src/multiselect/embedded.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ const EmbeddedMultiselect = React.forwardRef(
filteringType,
ariaLabel,
selectedOptions,
deselectAriaLabel,
virtualScroll,
filteringText = '',
...restProps
Expand All @@ -58,8 +57,6 @@ const EmbeddedMultiselect = React.forwardRef(
options,
selectedOptions,
filteringType,
disabled: false,
deselectAriaLabel,
controlId: formFieldContext.controlId,
ariaLabelId,
footerId,
Expand Down
10 changes: 5 additions & 5 deletions src/multiselect/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ const InternalMultiselect = React.forwardRef(
options,
selectedOptions,
filteringType,
disabled,
deselectAriaLabel,
controlId,
ariaLabelId,
footerId,
Expand Down Expand Up @@ -122,9 +120,11 @@ const InternalMultiselect = React.forwardRef(
iconUrl: option.iconUrl,
iconSvg: option.iconSvg,
tags: option.tags,
dismissLabel: i18n('deselectAriaLabel', deselectAriaLabel?.(option), format =>
format({ option__label: option.label ?? '' })
),
dismissLabel: hideTokens
? undefined
: i18n('deselectAriaLabel', deselectAriaLabel?.(option), format =>
format({ option__label: option.label ?? '' })
),
}));

const ListComponent = virtualScroll ? VirtualList : PlainList;
Expand Down
Loading
Loading