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

fix: Prevent Flashbar from delaying render when not using animations #1656

Merged
merged 22 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8f206b3
fix: Prevent Flashbar from delaying render when not using animations
jperals Oct 13, 2023
699d6f0
Remove unnecessary statement in test
jperals Oct 16, 2023
c85eb92
Increase coverage for collapsible flashbar with animations
jperals Oct 17, 2023
1e090d4
Use disableMotion to disable motion in tests instead of mocking
jperals Oct 17, 2023
dca93ac
Fix import
jperals Oct 17, 2023
8b2b52e
Remove some code duplication
jperals Oct 18, 2023
6af9665
Fix async state update issues in interactive flashbar page
jperals Oct 19, 2023
315370a
Replace ? with ! in test
jperals Oct 19, 2023
a46df27
Merge branch 'main' into fix/flashbar-delayed-render
jperals Oct 19, 2023
de6a191
Replace usePrevious with useEffectOnUpdate
jperals Oct 19, 2023
06f9be3
Refactor test suite to sue describe.each
jperals Oct 19, 2023
b30f6b9
Replace beforeAll and afterAll with beforeEach and afterEach
jperals Oct 19, 2023
d009b6c
Revert "Replace usePrevious with useEffectOnUpdate"
jperals Oct 19, 2023
acbdb5c
Do not animate flash items in collapsible flashbar in Classic
jperals Oct 20, 2023
55b65a7
Compare HTML output of collapsible flashbar with and without motion
jperals Oct 20, 2023
96acd62
Fix non-collapsible flashbar test not covering version with motion
jperals Oct 20, 2023
d959459
Revert "Revert "Replace usePrevious with useEffectOnUpdate""
jperals Oct 20, 2023
4238618
Use findToggleButton to find button in scroll test
jperals Oct 20, 2023
240e352
Use custom list wrapper instead of duplicating code
jperals Oct 20, 2023
a7bb8f7
Add missing attributes back
jperals Oct 20, 2023
ba5c348
Remove leftover .only
jperals Oct 23, 2023
44c41ba
Revert "Compare HTML output of collapsible flashbar with and without …
jperals Oct 23, 2023
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
12 changes: 6 additions & 6 deletions pages/flashbar/interactive.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export default function InteractiveFlashbar() {
};

const add = (type: FlashbarProps.Type, hasHeader = false) => {
setItems(items => [generateItem({ type, id: nextId.current.toString(), dismiss, hasHeader }), ...items]);
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 makes sure that the new item has the correct id and not the next one. Since React batches state updates, the old logic could lead to two items having the same id.

Copy link
Member

Choose a reason for hiding this comment

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

Or you could just embed this logic inside generateItem, so it always produces items with unique ids

const newItem = generateItem({ type, id: nextId.current.toString(), dismiss, hasHeader });
setItems(items => [newItem, ...items]);
nextId.current = nextId.current + 1;
};

Expand All @@ -26,15 +27,14 @@ export default function InteractiveFlashbar() {
};

const addToBottom = (type: FlashbarProps.Type, hasHeader = false) => {
setItems(items => [...items, generateItem({ type, dismiss, hasHeader, id: nextId.current.toString() })]);
const newItem = generateItem({ type, dismiss, hasHeader, id: nextId.current.toString() });
setItems(items => [...items, newItem]);
nextId.current = nextId.current + 1;
};

const removeAndAddToBottom = (type: FlashbarProps.Type, hasHeader = false) => {
setItems(items => [
generateItem({ type, dismiss, hasHeader, id: nextId.current.toString() }),
...items.slice(1, items.length),
]);
const newItem = generateItem({ type, dismiss, hasHeader, id: nextId.current.toString() });
setItems(items => [newItem, ...items.slice(1, items.length)]);
nextId.current = nextId.current + 1;
};

Expand Down
477 changes: 258 additions & 219 deletions src/flashbar/__tests__/collapsible.test.tsx

Large diffs are not rendered by default.

12 changes: 0 additions & 12 deletions src/flashbar/__tests__/common.ts

This file was deleted.

37 changes: 37 additions & 0 deletions src/flashbar/__tests__/common.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React, { useState } from 'react';
import createWrapper, { FlashbarWrapper } from '../../../lib/components/test-utils/dom';
import { render } from '@testing-library/react';
import { FlashbarProps } from '../interfaces';
import Button from '../../../lib/components/button';
import Flashbar from '../../../lib/components/flashbar';

export function createFlashbarWrapper(element: React.ReactElement) {
return createWrapper(render(element).container).findFlashbar()!;
}

export function findList(flashbar: FlashbarWrapper) {
return flashbar.find('ul');
}

export function testFlashDismissal({ stackItems }: { stackItems: boolean }) {
const App = () => {
const [items, setItems] = useState<ReadonlyArray<FlashbarProps.MessageDefinition>>([]);
const onDismiss = () => setItems([]);
const onAdd = () => setItems([{ content: 'The content', id: '1', dismissible: true, onDismiss }]);
return (
<>
<Button onClick={onAdd}>Add an item</Button>
<Flashbar stackItems={stackItems} items={items} />
</>
);
};
const appWrapper = createWrapper(render(<App />).container);
expect(appWrapper.findFlashbar()!.findItems()).toHaveLength(0);
appWrapper.findButton()!.click();
const foundItems = appWrapper.findFlashbar()!.findItems();
expect(foundItems).toHaveLength(1);
foundItems![0]!.findDismissButton()!.click();
expect(appWrapper.findFlashbar()!.findItems()).toHaveLength(0);
}
Loading
Loading