From 247dfdf7251a62545a1a3e690c3a466e34852713 Mon Sep 17 00:00:00 2001 From: nicolethoen Date: Wed, 5 Oct 2022 15:53:18 -0400 Subject: [PATCH 1/4] fix(Pagination): prevented regenerating random id on each render --- .../components/Pagination/OptionsToggle.tsx | 3 +- .../src/components/Pagination/Pagination.tsx | 3 +- .../__snapshots__/OptionsToggle.test.tsx.snap | 2 +- .../__snapshots__/Pagination.test.tsx.snap | 62 +++++++++---------- .../Pagination/examples/PaginationBottom.tsx | 2 +- .../Pagination/examples/PaginationCompact.tsx | 2 +- .../examples/PaginationDisabled.tsx | 2 +- .../examples/PaginationIndeterminate.tsx | 2 +- .../Pagination/examples/PaginationNoItems.tsx | 2 +- .../Pagination/examples/PaginationOnePage.tsx | 2 +- .../Pagination/examples/PaginationSticky.tsx | 2 +- .../Pagination/examples/PaginationTop.tsx | 2 +- 12 files changed, 42 insertions(+), 44 deletions(-) diff --git a/packages/react-core/src/components/Pagination/OptionsToggle.tsx b/packages/react-core/src/components/Pagination/OptionsToggle.tsx index 4d26714132f..a8372398bf6 100644 --- a/packages/react-core/src/components/Pagination/OptionsToggle.tsx +++ b/packages/react-core/src/components/Pagination/OptionsToggle.tsx @@ -44,7 +44,6 @@ export interface OptionsToggleProps extends React.HTMLProps { widgetId?: string; } -let toggleId = 0; export const OptionsToggle: React.FunctionComponent = ({ itemsTitle = 'items', optionsToggle, @@ -95,7 +94,7 @@ export const OptionsToggle: React.FunctionComponent = ({ onToggle={onToggle} isDisabled={isDisabled || (itemCount && itemCount <= 0)} isOpen={isOpen} - id={`${widgetId}-toggle-${toggleId++}`} + {...(widgetId && { id: `${widgetId}-toggle` })} className={isDiv ? styles.optionsMenuToggleButton : toggleClasses} parentRef={parentRef} aria-haspopup="listbox" diff --git a/packages/react-core/src/components/Pagination/Pagination.tsx b/packages/react-core/src/components/Pagination/Pagination.tsx index 6fd8fe5cc44..9b196f20166 100644 --- a/packages/react-core/src/components/Pagination/Pagination.tsx +++ b/packages/react-core/src/components/Pagination/Pagination.tsx @@ -177,7 +177,6 @@ const handleInputWidth = (lastPage: number, node: HTMLDivElement) => { } }; -let paginationId = 0; export class Pagination extends React.Component { static displayName = 'Pagination'; paginationRef = React.createRef(); @@ -319,7 +318,7 @@ export class Pagination extends React.Component diff --git a/packages/react-core/src/components/Pagination/__tests__/Generated/__snapshots__/OptionsToggle.test.tsx.snap b/packages/react-core/src/components/Pagination/__tests__/Generated/__snapshots__/OptionsToggle.test.tsx.snap index 43011732a81..1a9314b8664 100644 --- a/packages/react-core/src/components/Pagination/__tests__/Generated/__snapshots__/OptionsToggle.test.tsx.snap +++ b/packages/react-core/src/components/Pagination/__tests__/Generated/__snapshots__/OptionsToggle.test.tsx.snap @@ -16,7 +16,7 @@ exports[`OptionsToggle should match snapshot (auto-generated) 1`] = ` data-ouia-component-id="OUIA-Generated-DropdownToggle-1" data-ouia-component-type="PF4/DropdownToggle" data-ouia-safe="true" - id="''-toggle-0" + id="''-toggle" type="button" >
{ { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="compact-example" onPerPageSelect={onPerPageSelect} isCompact /> diff --git a/packages/react-core/src/components/Pagination/examples/PaginationDisabled.tsx b/packages/react-core/src/components/Pagination/examples/PaginationDisabled.tsx index a105a8d2d98..4073a9c2d8a 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationDisabled.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationDisabled.tsx @@ -25,7 +25,7 @@ export const PaginationDisabled: React.FunctionComponent = () => { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="disabled-example" onPerPageSelect={onPerPageSelect} isDisabled /> diff --git a/packages/react-core/src/components/Pagination/examples/PaginationIndeterminate.tsx b/packages/react-core/src/components/Pagination/examples/PaginationIndeterminate.tsx index 6b26fd0ac0d..e12a96aae61 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationIndeterminate.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationIndeterminate.tsx @@ -30,7 +30,7 @@ export const PaginationIndeterminate: React.FunctionComponent = () => { many )} - widgetId="pagination-indeterminate" + widgetId="indeterminate-example" perPage={perPage} page={page} onSetPage={onSetPage} diff --git a/packages/react-core/src/components/Pagination/examples/PaginationNoItems.tsx b/packages/react-core/src/components/Pagination/examples/PaginationNoItems.tsx index 0b4fa6f94c8..d2a6157fb8b 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationNoItems.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationNoItems.tsx @@ -25,7 +25,7 @@ export const PaginationNoItems: React.FunctionComponent = () => { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="no-items-example" onPerPageSelect={onPerPageSelect} /> ); diff --git a/packages/react-core/src/components/Pagination/examples/PaginationOnePage.tsx b/packages/react-core/src/components/Pagination/examples/PaginationOnePage.tsx index ed8d11569e8..b1a7c94e903 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationOnePage.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationOnePage.tsx @@ -25,7 +25,7 @@ export const PaginationOnePage: React.FunctionComponent = () => { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="one-page-example" onPerPageSelect={onPerPageSelect} /> ); diff --git a/packages/react-core/src/components/Pagination/examples/PaginationSticky.tsx b/packages/react-core/src/components/Pagination/examples/PaginationSticky.tsx index 45affd06b1d..b5b32552647 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationSticky.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationSticky.tsx @@ -33,7 +33,7 @@ export const PaginationSticky: React.FunctionComponent = () => { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="sticky-example" onPerPageSelect={onPerPageSelect} isSticky > diff --git a/packages/react-core/src/components/Pagination/examples/PaginationTop.tsx b/packages/react-core/src/components/Pagination/examples/PaginationTop.tsx index fdd3c7891d3..3ff580ece15 100644 --- a/packages/react-core/src/components/Pagination/examples/PaginationTop.tsx +++ b/packages/react-core/src/components/Pagination/examples/PaginationTop.tsx @@ -25,7 +25,7 @@ export const PaginationTop: React.FunctionComponent = () => { perPage={perPage} page={page} onSetPage={onSetPage} - widgetId="pagination-options-menu-top" + widgetId="top-example" onPerPageSelect={onPerPageSelect} /> ); From 694d9083d1d00a86a39cb1e2833396b17a594e42 Mon Sep 17 00:00:00 2001 From: nicolethoen Date: Thu, 6 Oct 2022 13:48:48 -0400 Subject: [PATCH 2/4] fix duplicate ids in existing demos --- .../src/components/Pagination/Pagination.tsx | 6 +- .../Pagination/PaginationOptionsMenu.tsx | 2 +- .../__snapshots__/Pagination.test.tsx.snap | 62 +++++++++---------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/react-core/src/components/Pagination/Pagination.tsx b/packages/react-core/src/components/Pagination/Pagination.tsx index 9b196f20166..b54da5a613a 100644 --- a/packages/react-core/src/components/Pagination/Pagination.tsx +++ b/packages/react-core/src/components/Pagination/Pagination.tsx @@ -210,7 +210,7 @@ export class Pagination extends React.Component undefined, onPerPageSelect: () => undefined, onFirstClick: () => undefined, @@ -318,7 +318,7 @@ export class Pagination extends React.Component @@ -355,7 +355,7 @@ export class Pagination extends React.Component
Date: Thu, 6 Oct 2022 20:57:24 -0400 Subject: [PATCH 3/4] fix(Pagination): update column management table demo pagination --- .../src/docs/demos/table-demos/ColumnManagement.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-table/src/docs/demos/table-demos/ColumnManagement.jsx b/packages/react-table/src/docs/demos/table-demos/ColumnManagement.jsx index 8a883f8c32a..518e6bbb777 100644 --- a/packages/react-table/src/docs/demos/table-demos/ColumnManagement.jsx +++ b/packages/react-table/src/docs/demos/table-demos/ColumnManagement.jsx @@ -425,7 +425,7 @@ export const ColumnManagementAction = () => { - {renderPagination()} + {renderPagination(PaginationVariant.top)} @@ -476,7 +476,7 @@ export const ColumnManagementAction = () => { ))} - {renderPagination()} + {renderPagination(PaginationVariant.bottom)} {renderModal()} From 53d111d1d938a1d26b3a2df9ebf63d0c86fea15d Mon Sep 17 00:00:00 2001 From: nicolethoen Date: Tue, 25 Oct 2022 10:32:05 -0400 Subject: [PATCH 4/4] add unit test --- .../components/Pagination/__tests__/Pagination.test.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/react-core/src/components/Pagination/__tests__/Pagination.test.tsx b/packages/react-core/src/components/Pagination/__tests__/Pagination.test.tsx index 29f5347134b..d6b672c7ed3 100644 --- a/packages/react-core/src/components/Pagination/__tests__/Pagination.test.tsx +++ b/packages/react-core/src/components/Pagination/__tests__/Pagination.test.tsx @@ -112,6 +112,13 @@ describe('Pagination', () => { const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); }); + + test('should not update generated options menu id on rerenders', () => { + const { rerender } = render(); + const id = screen.getByLabelText("test label").getAttribute("id"); + rerender(); + expect(screen.getByLabelText("test label")).toHaveAttribute("id", id); + }); }); describe('API', () => {