Skip to content

Commit

Permalink
[EuiBasicTable][EuiInMemoryTable][EuiDataGrid] Revert page size `numb…
Browse files Browse the repository at this point in the history
…er | 'all'` type to just `number` (#5699)

* Revert "Update EuiDataGrid pagination props to accept 'all'"

This reverts commit c476dc8.

* Revert "Update EuiBasicTable and EuiInMemoryTable props to accept 'all'"

This reverts commit 8efabe6.

* Revert "Update documentation examples to use 'all'"

This reverts commit 2a05385.

* Revert "Update EuiTablePagination to accept number | 'all' instead of 0 for all"

This reverts commit 6903978.

* Add changelog entry

* Re-add removed tests/snapshots

* Re-add datagrid pagination checks

* Re-add prop documentation changes

* Restore pageSize checks that would otherwise result in Infinity

* Revert token revert

* Update docs (thanks Caroline for the catch)

* [EuiInMemoryTable] Account for pageSizes of 0

0 is falsey and thus is failing a bunch of checks/fallthroughs, switching to `??` and `!=` null generally solves the problem
  • Loading branch information
Constance authored Mar 8, 2022
1 parent da1a5fa commit 0432bfd
Show file tree
Hide file tree
Showing 25 changed files with 91 additions and 100 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

- Fixed non-searchable `EuiSelectable`s not selecting items with the Enter & Space keys ([#5693](https://github.com/elastic/eui/pull/5693))

**Breaking changes**

- Removed the `'all'` option in `EuiTablePagination.itemsPerPage` and `itemsPerPageOptions` in `EuiBasicTable` and `EuiDataGrid` due to Typescript issues. Use `0` instead to represent a "Show all" option ([#5699](https://github.com/elastic/eui/issues/5699))

## [`50.0.0`](https://github.com/elastic/eui/tree/v50.0.0)

- Updated `EuiComboBox` to WAI-ARIA 1.2 pattern and improved keyboard navigation ([#5636](https://github.com/elastic/eui/pull/5636))
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/datagrid/in_memory_pagination.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ export default () => {
sorting={{ columns: sortingColumns, onSort }}
pagination={{
...pagination,
pageSizeOptions: [10, 50, 'all'],
pageSizeOptions: [10, 50, 100],
onChangeItemsPerPage: onChangeItemsPerPage,
onChangePage: onChangePage,
}}
Expand Down
12 changes: 6 additions & 6 deletions src-docs/src/views/pagination/paginated_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ export default () => {
const totalItemCount = raw_data.length;
const startIndex = pageIndex * pageSize;
const pageOfItems =
pageSize === 'all'
? raw_data
: raw_data.slice(
pageSize > 0
? raw_data.slice(
startIndex,
Math.min(startIndex + pageSize, totalItemCount)
);
)
: raw_data;

const columns = [
{
Expand Down Expand Up @@ -93,11 +93,11 @@ export default () => {
pageIndex,
pageSize,
totalItemCount,
pageSizeOptions: [10, 'all'],
pageSizeOptions: [10, 0],
};

const resultsCount =
pageSize === 'all' ? (
pageSize === 0 ? (
<strong>All</strong>
) : (
<>
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/pagination/pagination_example.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export const PaginationExample = {
<Link to="/tabular-content/tables#adding-pagination-to-a-table">
tables
</Link>
. If you pass <EuiCode>{"'all'"}</EuiCode> in as one of the{' '}
. If you pass <EuiCode>0</EuiCode> in as one of the{' '}
<EuiCode>itemsPerPageOptions</EuiCode>, it will create a &quot;Show
all&quot; option and hide the pagination.
</p>
Expand Down
10 changes: 4 additions & 6 deletions src-docs/src/views/pagination/table_pagination.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@ import { EuiTablePagination } from '../../../../src';
export default () => {
const totalEntries = 1250;
const [activePage, setActivePage] = useState(0);
const [rowSize, setRowSize] = useState<number | 'all'>(50);
const [rowSize, setRowSize] = useState(50);
const [pageCount, setPageCount] = useState(Math.ceil(totalEntries / 50));

const goToPage = (pageNumber: number) => setActivePage(pageNumber);
const changeItemsPerPage = (pageSize: number | 'all') => {
const pageCount =
pageSize === 'all' ? 1 : Math.ceil(totalEntries / pageSize);
setPageCount(pageCount);
const changeItemsPerPage = (pageSize: number) => {
setPageCount(Math.ceil(totalEntries / pageSize));
setRowSize(pageSize);
setActivePage(0);
};
Expand All @@ -25,7 +23,7 @@ export default () => {
onChangePage={goToPage}
itemsPerPage={rowSize}
onChangeItemsPerPage={changeItemsPerPage}
itemsPerPageOptions={[10, 20, 'all']}
itemsPerPageOptions={[10, 20, 0]}
/>
);
};
2 changes: 1 addition & 1 deletion src-docs/src/views/tables/data_store.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const createDataStore = () => {

let pageOfItems;

if ((!pageIndex && !pageSize) || pageSize === 'all') {
if (!pageIndex && !pageSize) {
pageOfItems = items;
} else {
const startIndex = pageIndex * pageSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const Table = () => {
columns={columns}
pagination={{
...pagination,
pageSizeOptions: [10, 20, 'all'],
pageSizeOptions: [10, 20, 0],
}}
sorting={sorting}
/>
Expand Down
4 changes: 2 additions & 2 deletions src-docs/src/views/tables/paginated/paginated.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,12 @@ export const Table = () => {
pageIndex,
pageSize,
totalItemCount,
pageSizeOptions: [10, 'all'],
pageSizeOptions: [10, 0],
showPerPageOptions,
};

const resultsCount =
pageSize === 'all' ? (
pageSize === 0 ? (
<strong>All</strong>
) : (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ exports[`PaginationBar render - show all pageSize 1`] = `
<EuiTablePagination
activePage={0}
aria-label="aria-label"
itemsPerPage="all"
itemsPerPage={0}
itemsPerPageOptions={
Array [
1,
5,
"all",
0,
]
}
onChangeItemsPerPage={[Function]}
Expand Down
4 changes: 2 additions & 2 deletions src/components/basic_table/basic_table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,8 @@ describe('EuiBasicTable', () => {
],
pagination: {
pageIndex: 0,
pageSize: 'all',
pageSizeOptions: [1, 5, 'all'],
pageSize: 0,
pageSizeOptions: [1, 5, 0],
totalItemCount: 2,
},
onChange: () => {},
Expand Down
16 changes: 7 additions & 9 deletions src/components/basic_table/basic_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export interface Criteria<T> {
*/
page?: {
index: number;
size: number | 'all';
size: number;
};
/**
* If the shown items are sorted, this describes the sort criteria
Expand All @@ -182,7 +182,7 @@ export interface CriteriaWithPagination<T> extends Criteria<T> {
*/
page: {
index: number;
size: number | 'all';
size: number;
};
}

Expand Down Expand Up @@ -459,7 +459,7 @@ export class EuiBasicTable<T = any> extends Component<
this.changeSelection([]);
}

onPageSizeChange(size: number | 'all') {
onPageSizeChange(size: number) {
this.clearSelection();
const currentCriteria = this.buildCriteria(this.props);
const criteria: CriteriaWithPagination<T> = {
Expand Down Expand Up @@ -660,10 +660,9 @@ export class EuiBasicTable<T = any> extends Component<
const itemCount = items.length;
const totalItemCount = pagination ? pagination.totalItemCount : itemCount;
const page = pagination ? pagination.pageIndex + 1 : 1;
const pageCount =
typeof pagination?.pageSize === 'number'
? Math.ceil(pagination.totalItemCount / pagination.pageSize)
: 1;
const pageCount = pagination?.pageSize
? Math.ceil(pagination.totalItemCount / pagination.pageSize)
: 1;

let captionElement;
if (tableCaption) {
Expand Down Expand Up @@ -949,8 +948,7 @@ export class EuiBasicTable<T = any> extends Component<
const rows = items.map((item: T, index: number) => {
// if there's pagination the item's index must be adjusted to the where it is in the whole dataset
const tableItemIndex =
hasPagination(this.props) &&
typeof this.props.pagination.pageSize === 'number'
hasPagination(this.props) && this.props.pagination.pageSize > 0
? this.props.pagination.pageIndex * this.props.pagination.pageSize +
index
: index;
Expand Down
4 changes: 2 additions & 2 deletions src/components/basic_table/in_memory_table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ describe('EuiInMemoryTable', () => {
},
],
pagination: {
initialPageSize: 'all',
pageSizeOptions: [1, 2, 3, 'all'],
initialPageSize: 0,
pageSizeOptions: [1, 2, 3, 0],
},
};
const component = render(<EuiInMemoryTable {...props} />);
Expand Down
22 changes: 11 additions & 11 deletions src/components/basic_table/in_memory_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ function isEuiSearchBarProps<T>(
export type Search = boolean | EuiSearchBarProps;

interface PaginationOptions extends EuiTablePaginationProps {
pageSizeOptions?: Array<number | 'all'>;
pageSizeOptions?: number[];
initialPageIndex?: number;
initialPageSize?: number | 'all';
initialPageSize?: number;
pageIndex?: number;
pageSize?: number | 'all';
pageSize?: number;
}

type Pagination = boolean | PaginationOptions;
Expand Down Expand Up @@ -114,8 +114,8 @@ interface State<T> {
search?: Search;
query: Query | null;
pageIndex: number;
pageSize?: number | 'all';
pageSizeOptions?: Array<number | 'all'>;
pageSize?: number;
pageSizeOptions?: number[];
sortName: ReactNode;
sortDirection?: Direction;
allowNeutralSort: boolean;
Expand Down Expand Up @@ -161,15 +161,15 @@ const getInitialPagination = (pagination: Pagination | undefined) => {
const initialPageIndex =
pagination === true
? 0
: pagination.pageIndex || pagination.initialPageIndex || 0;
: pagination.pageIndex ?? pagination.initialPageIndex ?? 0;
const initialPageSize =
pagination === true
? defaultPageSize
: pagination.pageSize || pagination.initialPageSize || defaultPageSize;
: pagination.pageSize ?? pagination.initialPageSize ?? defaultPageSize;

if (
showPerPageOptions &&
initialPageSize &&
initialPageSize != null &&
(!pageSizeOptions || !pageSizeOptions.includes(initialPageSize))
) {
throw new Error(
Expand Down Expand Up @@ -389,7 +389,7 @@ export class EuiInMemoryTable<T> extends Component<
onTableChange = ({ page, sort }: Criteria<T>) => {
let { index: pageIndex, size: pageSize } = (page || {}) as {
index: number;
size: number | 'all';
size: number;
};

// don't apply pagination changes that are otherwise controlled
Expand Down Expand Up @@ -583,7 +583,7 @@ export class EuiInMemoryTable<T> extends Component<
: matchingItems;

const visibleItems =
typeof pageSize === 'number' && this.props.pagination
pageSize && this.props.pagination
? (() => {
const startIndex = pageIndex * pageSize;
return sortedItems.slice(
Expand Down Expand Up @@ -640,7 +640,7 @@ export class EuiInMemoryTable<T> extends Component<
? undefined
: {
pageIndex,
pageSize: pageSize || 1,
pageSize: pageSize ?? 1,
pageSizeOptions,
totalItemCount,
showPerPageOptions,
Expand Down
4 changes: 2 additions & 2 deletions src/components/basic_table/pagination_bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ describe('PaginationBar', () => {
...requiredProps,
pagination: {
pageIndex: 0,
pageSize: 'all' as const,
pageSizeOptions: [1, 5, 'all' as const],
pageSize: 0,
pageSizeOptions: [1, 5, 0],
totalItemCount: 5,
},
onPageSizeChange: () => {},
Expand Down
15 changes: 7 additions & 8 deletions src/components/basic_table/pagination_bar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@ export interface Pagination {
pageIndex: number;
/**
* The maximum number of items that can be shown in a single page.
* Pass `'all'` to display the selected "Show all" option and hide the pagination.
* Pass `0` to display the selected "Show all" option and hide the pagination.
*/
pageSize: number | 'all';
pageSize: number;
/**
* The total number of items the page is "sliced" of
*/
totalItemCount: number;
/**
* Configures the page size dropdown options.
* Pass `'all'` as one of the options to create a "Show all" option.
* Pass `0` as one of the options to create a "Show all" option.
*/
pageSizeOptions?: Array<number | 'all'>;
pageSizeOptions?: number[];
/**
* Hides the page size dropdown
*/
Expand Down Expand Up @@ -64,10 +64,9 @@ export const PaginationBar = ({
const pageSizeOptions = pagination.pageSizeOptions
? pagination.pageSizeOptions
: defaults.pageSizeOptions;
const pageCount =
pagination.pageSize === 'all'
? 1
: Math.ceil(pagination.totalItemCount / pagination.pageSize);
const pageCount = pagination.pageSize
? Math.ceil(pagination.totalItemCount / pagination.pageSize)
: 1;

useEffect(() => {
if (pageCount < pagination.pageIndex + 1) {
Expand Down
16 changes: 8 additions & 8 deletions src/components/datagrid/data_grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,9 @@ export const EuiDataGrid = forwardRef<EuiDataGridRefProps, EuiDataGridProps>(
const ariaLabelledById = useGeneratedHtmlId();

const ariaPage = pagination ? pagination.pageIndex + 1 : 1;
const ariaPageCount =
typeof pagination?.pageSize === 'number'
? Math.ceil(rowCount / pagination.pageSize)
: 1;
const ariaPageCount = pagination?.pageSize
? Math.ceil(rowCount / pagination.pageSize)
: 1;
const ariaLabel = useEuiI18n(
'euiDataGrid.ariaLabel',
'{label}; Page {page} of {pageCount}.',
Expand Down Expand Up @@ -410,10 +409,11 @@ export const EuiDataGrid = forwardRef<EuiDataGridRefProps, EuiDataGridProps>(
renderCellValue={renderCellValue}
columns={columns}
rowCount={
inMemory.level === 'enhancements' && // if `inMemory.level === enhancements` then we can only be sure the pagination's pageSize is available in memory
typeof pagination?.pageSize === 'number' // If pageSize is set to 'all' instead of a number, then all rows are being displayed
? pagination?.pageSize || rowCount
: rowCount // otherwise, all of the data is present and usable
inMemory.level === 'enhancements'
? // if `inMemory.level === enhancements` then we can only be sure the pagination's pageSize is available in memory
pagination?.pageSize || rowCount
: // otherwise, all of the data is present and usable
rowCount
}
onCellRender={onCellRender}
/>
Expand Down
10 changes: 5 additions & 5 deletions src/components/datagrid/data_grid_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,19 +772,19 @@ export interface EuiDataGridPaginationProps {
pageIndex: number;
/**
* How many rows should initially be shown per page.
* Pass `'all'` to display the selected "Show all" option and hide the pagination.
* Pass `0` to display the selected "Show all" option and hide the pagination.
*/
pageSize: number | 'all';
pageSize: number;
/**
* An array of page sizes the user can select from.
* Pass `'all'` as one of the options to create a "Show all" option.
* Pass `0` as one of the options to create a "Show all" option.
* Leave this prop undefined or use an empty array to hide "Rows per page" select button.
*/
pageSizeOptions?: Array<number | 'all'>;
pageSizeOptions?: number[];
/**
* A callback for when the user changes the page size selection
*/
onChangeItemsPerPage: (itemsPerPage: number | 'all') => void;
onChangeItemsPerPage: (itemsPerPage: number) => void;
/**
* A callback for when the current page index changes
*/
Expand Down
Loading

0 comments on commit 0432bfd

Please sign in to comment.