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.

Sign up for GitHub

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

chore: Add alert/flash content replacement runtime API #2647

Merged
merged 36 commits into from
Sep 13, 2024
Merged

chore: Add alert/flash content replacement runtime API #2647

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
a239e13
feat: Add alert content replacement runtime API
gethinwebster Aug 13, 2024
f1cc542
Fix a11y issue in page
gethinwebster Sep 5, 2024
3fe30e7
Some test refactoring
gethinwebster Sep 5, 2024
49aee65
Update alert test (and fix bug)
gethinwebster Sep 5, 2024
630e672
Tweak `unmount` signature
gethinwebster Sep 5, 2024
d48a512
Simplify demo page
gethinwebster Sep 5, 2024
8f48792
Remove stray testid
gethinwebster Sep 5, 2024
7680b0c
add apis export
pan-kot Sep 6, 2024
1d309e1
refactor use-visual-refresh mock
pan-kot Sep 6, 2024
5c88b5d
refactor test selectors in alert test
pan-kot Sep 6, 2024
e703307
refactor runtime content tests: use createWrapper
pan-kot Sep 6, 2024
fb6b140
remove exposed apis
pan-kot Sep 6, 2024
85dd6fc
safer ref types
pan-kot Sep 6, 2024
6b7303d
remove public test utils for replacement nodes
pan-kot Sep 6, 2024
41c7be5
better test pages
pan-kot Sep 9, 2024
f6c3c7d
fix tests for unmount
pan-kot Sep 9, 2024
5ee57a0
refactor tests
pan-kot Sep 9, 2024
55c5923
use native private methods
pan-kot Sep 9, 2024
1b2a313
add test cov for content removal
pan-kot Sep 9, 2024
2b588f1
fix multi plugin registering and add a test
pan-kot Sep 9, 2024
b7f37b1
store listener cleanups separately
pan-kot Sep 9, 2024
417801c
sync replacer
pan-kot Sep 9, 2024
45c4d2b
run replacer when type changes
pan-kot Sep 9, 2024
4696fe3
improve warn messages
pan-kot Sep 9, 2024
acd5f3e
updated api to avoid mixed types
pan-kot Sep 9, 2024
681fe47
codestyle
pan-kot Sep 9, 2024
a0987e2
remove actionsref
pan-kot Sep 10, 2024
823c373
rework api
pan-kot Sep 10, 2024
1ee7cc8
async api
pan-kot Sep 10, 2024
6a0e3c3
review suggestion
pan-kot Sep 10, 2024
bbab06e
added content swap to playground page
pan-kot Sep 10, 2024
facfc6c
codecov
pan-kot Sep 10, 2024
55af3ac
remove unneeded cleanup from listener type
pan-kot Sep 12, 2024
bd57707
test coverage
pan-kot Sep 12, 2024
ea6ff69
codestyle improvmeents
pan-kot Sep 12, 2024
538df4b
avoid calling listener when provider is not defined
pan-kot Sep 12, 2024
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
134 changes: 134 additions & 0 deletions pages/alert/runtime-content.page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
pan-kot marked this conversation as resolved.
Show resolved Hide resolved
// SPDX-License-Identifier: Apache-2.0

import React, { useContext, useMemo, useState } from 'react';
import { render, unmountComponentAtNode } from 'react-dom';

import {
Alert,
AlertProps,
Box,
Button,
Checkbox,
ExpandableSection,
FormField,
Select,
SpaceBetween,
} from '~components';
import awsuiPlugins from '~components/internal/plugins';

import AppContext, { AppContextType } from '../app/app-context';
import ScreenshotArea from '../utils/screenshot-area';

type PageContext = React.Context<AppContextType<{ loading: boolean; hidden: boolean; type: AlertProps.Type }>>;

awsuiPlugins.alertContent.registerContentReplacer({
id: 'awsui/alert-test-action',
runReplacer(context, replacer) {
console.log('mount');

const doReplace = () => {
replacer.restoreHeader();
replacer.restoreContent();
if (context.type === 'error' && context.contentRef.current?.textContent?.match('Access denied')) {
replacer.hideHeader();
replacer.replaceContent(container => {
console.log('render replacement content');
render(
<SpaceBetween size="s">
<Box>---REPLACEMENT--- Access denied message! ---REPLACEMENT---</Box>
<ExpandableSection headerText="Original message">
{context.contentRef.current?.textContent}
</ExpandableSection>
</SpaceBetween>,
container
);
});
}
};

doReplace();

return {
update() {
console.log('update');
doReplace();
},
unmount({ replacementContentContainer }) {
console.log('unmount');
unmountComponentAtNode(replacementContentContainer);
},
};
},
});

const alertTypeOptions = ['error', 'warning', 'info', 'success'].map(type => ({ value: type }));

export default function () {
const {
urlParams: { loading = false, hidden = false, type = 'error' },
setUrlParams,
} = useContext(AppContext as PageContext);
const [unrelatedState, setUnrelatedState] = useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

This unrelated state does not demonstrate the any issues, because all alert content is primitive strings. Try placing JSX in the first alert and see what happens

Copy link
Member

Choose a reason for hiding this comment

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

I added content swap to test page. When set - the content between the first and the second alert swaps. I observed no issues with this.

Copy link
Member

Choose a reason for hiding this comment

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

I provided specific steps to reproduce the issue:

Try placing JSX in the first alert and see what happens

You did not follow these steps and report you do not see the issue.

I am not sure what else I can do here. I surrender on this thread.

Copy link
Member

Choose a reason for hiding this comment

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

Please clarify what is the issue I should see if placing JSX. The replacement is already JSX. If I use some <Box>content</Box> instead of 'content' that seems to work alright.

Copy link
Member

Choose a reason for hiding this comment

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

This is the issue

Screen.Recording.2024-09-12.at.10.23.37.mov

I would say changing the "unrelated state" should not trigger any updates, none of these console logs should be happening.

If you think it is fine, handling possible impact of this risk is on you.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, it is clear now. I think this is inline with how React works. If the JSX is memoized with memo or useMemo - the behaviour is not observed.

Copy link
Member

Choose a reason for hiding this comment

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

The consuming team may put some expensive code inside the doRender function. The quip proposal mentions there might be a network call there. Are we okay with that many network calls on unrelated state changes?

Copy link
Member

Choose a reason for hiding this comment

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

If the JSX is memoized with memo or useMemo - the behaviour is not observed.

This means all existing Alert usage will have to be changed. I do not think it is a reasonable mitigation

const [contentSwapped, setContentSwapped] = useState(false);

const content1 = useMemo(() => (loading ? <Box>Loading...</Box> : <Box>Content</Box>), [loading]);
const content2 = loading ? <Box>Loading...</Box> : <Box>There was an error: Access denied because of XYZ</Box>;

return (
<Box margin="m">
<h1>Alert runtime actions</h1>
<SpaceBetween size="m">
<SpaceBetween size="s">
<Checkbox onChange={e => setUrlParams({ loading: e.detail.checked })} checked={loading}>
Content loading
</Checkbox>
<Checkbox onChange={e => setUrlParams({ hidden: e.detail.checked })} checked={hidden}>
Unmount all
</Checkbox>
<Checkbox onChange={e => setUnrelatedState(e.detail.checked)} checked={unrelatedState}>
Unrelated state
</Checkbox>
<Checkbox onChange={e => setContentSwapped(e.detail.checked)} checked={contentSwapped}>
Swap content
</Checkbox>
<FormField label="Alert type">
<Select
options={alertTypeOptions}
selectedOption={alertTypeOptions.find(option => option.value === type) ?? alertTypeOptions[0]}
onChange={e => setUrlParams({ type: e.detail.selectedOption.value as AlertProps.Type })}
/>
</FormField>
</SpaceBetween>

<hr />

<ScreenshotArea gutters={false}>
{hidden ? null : (
<SpaceBetween size="m">
<Alert
type={type}
statusIconAriaLabel={type}
dismissAriaLabel="Dismiss"
header="Header"
action={<Button>Action</Button>}
>
{!contentSwapped ? content1 : content2}
</Alert>

<Alert
type={type}
statusIconAriaLabel={type}
dismissAriaLabel="Dismiss"
header="Header"
action={<Button>Action</Button>}
>
{!contentSwapped ? content2 : content1}
</Alert>
</SpaceBetween>
)}
</ScreenshotArea>
</SpaceBetween>
</Box>
);
}
130 changes: 130 additions & 0 deletions pages/flashbar/runtime-content.page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import React, { useContext, useState } from 'react';
import { render, unmountComponentAtNode } from 'react-dom';

import {
Box,
Button,
Checkbox,
ExpandableSection,
Flashbar,
FlashbarProps,
FormField,
Select,
SpaceBetween,
} from '~components';
import awsuiPlugins from '~components/internal/plugins';

import AppContext, { AppContextType } from '../app/app-context';
import ScreenshotArea from '../utils/screenshot-area';

type PageContext = React.Context<
AppContextType<{ loading: boolean; hidden: boolean; stackItems: boolean; type: FlashbarProps.Type }>
>;

awsuiPlugins.flashContent.registerContentReplacer({
id: 'awsui/flashbar-test-action',
runReplacer(context, replacer) {
console.log('mount');

const doReplace = () => {
replacer.restoreHeader();
replacer.restoreContent();
if (context.type === 'error' && context.contentRef.current?.textContent?.match('Access denied')) {
replacer.hideHeader();
replacer.replaceContent(container => {
console.log('render replacement content');
render(
<SpaceBetween size="s">
<Box>---REPLACEMENT--- Access denied message! ---REPLACEMENT---</Box>
<ExpandableSection headerText="Original message">
{context.contentRef.current?.textContent}
</ExpandableSection>
</SpaceBetween>,
container
);
});
}
};

doReplace();

return {
update() {
console.log('update');
doReplace();
},
unmount({ replacementContentContainer }) {
console.log('unmount');
unmountComponentAtNode(replacementContentContainer);
},
};
},
});

const messageTypeOptions = ['error', 'warning', 'info', 'success'].map(type => ({ value: type }));

export default function () {
const {
urlParams: { loading = false, hidden = false, stackItems = false, type = 'error' },
setUrlParams,
} = useContext(AppContext as PageContext);
const [unrelatedState, setUnrelatedState] = useState(false);

return (
<Box margin="m">
<h1>Flashbar runtime actions</h1>
<SpaceBetween size="m">
<SpaceBetween size="s">
<Checkbox onChange={e => setUrlParams({ loading: e.detail.checked })} checked={loading}>
Content loading
</Checkbox>
<Checkbox onChange={e => setUrlParams({ hidden: e.detail.checked })} checked={hidden}>
Unmount all
</Checkbox>
<Checkbox onChange={e => setUrlParams({ stackItems: e.detail.checked })} checked={stackItems}>
Stack items
</Checkbox>
<Checkbox onChange={e => setUnrelatedState(e.detail.checked)} checked={unrelatedState}>
Unrelated state
</Checkbox>
<FormField label="Message type">
<Select
options={messageTypeOptions}
selectedOption={messageTypeOptions.find(option => option.value === type) ?? messageTypeOptions[0]}
onChange={e => setUrlParams({ type: e.detail.selectedOption.value as FlashbarProps.Type })}
/>
</FormField>
</SpaceBetween>

<hr />

<ScreenshotArea gutters={false}>
{hidden ? null : (
<Flashbar
stackItems={stackItems}
items={[
{
type,
statusIconAriaLabel: type,
header: 'Header',
content: loading ? 'Loading...' : 'Content',
action: <Button>Action</Button>,
},
{
type,
statusIconAriaLabel: type,
header: 'Header',
content: loading ? 'Loading...' : 'There was an error: Access denied because of XYZ',
action: <Button>Action</Button>,
},
]}
/>
)}
</ScreenshotArea>
</SpaceBetween>
</Box>
);
}
34 changes: 34 additions & 0 deletions src/alert/__tests__/alert.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,25 @@ import '../../__a11y__/to-validate-a11y';
import Alert, { AlertProps } from '../../../lib/components/alert';
import Button from '../../../lib/components/button';
import { DATA_ATTR_ANALYTICS_ALERT } from '../../../lib/components/internal/analytics/selectors';
import { useVisualRefresh } from '../../../lib/components/internal/hooks/use-visual-mode';
import createWrapper from '../../../lib/components/test-utils/dom';

import styles from '../../../lib/components/alert/styles.css.js';

jest.mock('../../../lib/components/internal/hooks/use-visual-mode', () => ({
...jest.requireActual('../../../lib/components/internal/hooks/use-visual-mode'),
useVisualRefresh: jest.fn().mockReturnValue(false),
}));

function renderAlert(props: AlertProps = {}) {
const { container } = render(<Alert {...props} />);
return { wrapper: createWrapper(container).findAlert()!, container };
}

beforeEach(() => {
jest.mocked(useVisualRefresh).mockReset();
});

describe('Alert Component', () => {
describe('structure', () => {
it('has no header container when no header is set', () => {
Expand Down Expand Up @@ -153,4 +163,28 @@ describe('Alert Component', () => {
expect(wrapper.getElement()).toHaveAttribute(DATA_ATTR_ANALYTICS_ALERT, 'success');
});
});

describe('icon size', () => {
test('classic - big if has header and content', () => {
const { wrapper } = renderAlert({ header: 'Header', children: ['Content'] });
expect(wrapper.findByClassName(styles['icon-size-normal'])).toBeFalsy();
expect(wrapper.findByClassName(styles['icon-size-big'])).toBeTruthy();
});
test('classic - normal if only header', () => {
const { wrapper } = renderAlert({ header: 'Header' });
expect(wrapper.findByClassName(styles['icon-size-big'])).toBeFalsy();
expect(wrapper.findByClassName(styles['icon-size-normal'])).toBeTruthy();
});
test('classic - normal if only content', () => {
const { wrapper } = renderAlert({ children: ['Content'] });
expect(wrapper.findByClassName(styles['icon-size-big'])).toBeFalsy();
expect(wrapper.findByClassName(styles['icon-size-normal'])).toBeTruthy();
});
test('visual refresh - always normal', () => {
jest.mocked(useVisualRefresh).mockReturnValue(true);
const { wrapper } = renderAlert({ header: 'Header', children: ['Content'] });
expect(wrapper.findByClassName(styles['icon-size-big'])).toBeFalsy();
expect(wrapper.findByClassName(styles['icon-size-normal'])).toBeTruthy();
});
});
});
62 changes: 62 additions & 0 deletions src/alert/__tests__/runtime-content-utils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import { AlertWrapper } from '../../../lib/components/test-utils/dom';
import FlashWrapper from '../../../lib/components/test-utils/dom/flashbar/flash.js';

import alertStyles from '../../../lib/components/alert/styles.selectors.js';
import flashbarStyles from '../../../lib/components/flashbar/styles.selectors.js';

export const expectContent = (
wrapper: AlertWrapper | FlashWrapper,
stylesCss: Record<string, string>,
{
header,
headerReplaced,
content,
contentReplaced,
}: {
header?: string | false;
headerReplaced?: boolean;
content?: string | false;
contentReplaced?: boolean;
}
) => {
if (header) {
if (headerReplaced) {
if (wrapper.findHeader()) {
expect(wrapper.findHeader()?.getElement()).toHaveClass(stylesCss.hidden);
}
expect(findReplacementHeader(wrapper)?.getElement().textContent).toBe(header);
} else {
expect(findReplacementHeader(wrapper)?.getElement()).toHaveClass(stylesCss.hidden);
expect(wrapper.findHeader()?.getElement().textContent).toBe(header);
}
} else if (header === false) {
if (wrapper.findHeader()) {
expect(wrapper.findHeader()?.getElement()).toHaveClass(stylesCss.hidden);
}
expect(findReplacementHeader(wrapper)?.getElement()).toHaveClass(stylesCss.hidden);
}
if (content) {
if (contentReplaced) {
expect(wrapper.findContent()?.getElement()).toHaveClass(stylesCss.hidden);
expect(findReplacementContent(wrapper)?.getElement().textContent).toBe(content);
} else {
expect(findReplacementContent(wrapper)?.getElement()).toHaveClass(stylesCss.hidden);
expect(wrapper.findContent()?.getElement().textContent).toBe(content);
}
} else if (content === false) {
expect(wrapper.findContent()?.getElement()).toHaveClass(stylesCss.hidden);
expect(findReplacementContent(wrapper)?.getElement()).toHaveClass(stylesCss.hidden);
}
};

function findReplacementHeader(wrapper: AlertWrapper | FlashWrapper) {
const styles = wrapper instanceof AlertWrapper ? alertStyles : flashbarStyles;
return wrapper.findByClassName(styles['header-replacement']);
}

function findReplacementContent(wrapper: AlertWrapper | FlashWrapper) {
const styles = wrapper instanceof AlertWrapper ? alertStyles : flashbarStyles;
return wrapper.findByClassName(styles['content-replacement'])!;
}
Loading
Loading