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

[On-week] Add persisting page size and sorting for Management tables #186997

Merged
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c03a113
[Spacetime] Add persisting page size for Management tables
ElenaStoeva Jun 26, 2024
c726758
Add persisting page size to IM and ILM
ElenaStoeva Jun 27, 2024
1d1c7fe
Use a different page size local storage for each table
ElenaStoeva Jun 27, 2024
69dc047
Move onTableChange handler to hook
ElenaStoeva Jun 27, 2024
0dea176
Add persistent sort
ElenaStoeva Jun 28, 2024
5737a57
Fix local storage variable delimeter
ElenaStoeva Jul 3, 2024
05eb300
Add initial sort for enrich policies table
ElenaStoeva Jul 3, 2024
79ee0b6
Merge branch 'main' into on-week/persisting-table-page-size
ElenaStoeva Aug 5, 2024
68442c9
Refactor changes, add validation and tests
ElenaStoeva Aug 5, 2024
8e155d6
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Aug 5, 2024
573b4c9
Add more documentation and some fixes
ElenaStoeva Aug 6, 2024
b0408ef
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine Aug 6, 2024
e8838d1
Fix failing tests and return original page size options in tables
ElenaStoeva Aug 7, 2024
0cbfb37
Merge branch 'on-week/persisting-table-page-size' of https://github.c…
ElenaStoeva Aug 7, 2024
cfbcb3d
Fix ILM test failures
ElenaStoeva Aug 7, 2024
f4688fd
Merge branch 'main' into on-week/persisting-table-page-size
ElenaStoeva Aug 7, 2024
219031f
Fix type error
ElenaStoeva Aug 7, 2024
e7350b1
Small fixes
ElenaStoeva Aug 8, 2024
0cb54bb
Merge branch 'main' into on-week/persisting-table-page-size
ElenaStoeva Aug 8, 2024
d6c8a52
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Aug 8, 2024
62d8903
Fix type errors
ElenaStoeva Aug 8, 2024
228813c
Resolve merge conflicts
ElenaStoeva Aug 8, 2024
ada6dd7
Update readme
ElenaStoeva Aug 8, 2024
096abab
Address feedback
ElenaStoeva Aug 14, 2024
a2aa291
Fix type errors
ElenaStoeva Aug 15, 2024
4a22c21
Merge branch 'main' into on-week/persisting-table-page-size
ElenaStoeva Aug 15, 2024
7754207
Address feedback
ElenaStoeva Aug 20, 2024
2aeb917
Fix page size url params in Index management
ElenaStoeva Aug 20, 2024
6948a64
Merge branch 'main' into on-week/persisting-table-page-size
ElenaStoeva Aug 20, 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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,7 @@ packages/shared-ux/router/types @elastic/appex-sharedux
packages/shared-ux/storybook/config @elastic/appex-sharedux
packages/shared-ux/storybook/mock @elastic/appex-sharedux
packages/shared-ux/modal/tabbed @elastic/appex-sharedux
packages/shared-ux/table_persist @elastic/appex-sharedux
packages/kbn-shared-ux-utility @elastic/appex-sharedux
x-pack/plugins/observability_solution/slo @elastic/obs-ux-management-team
x-pack/packages/kbn-slo-schema @elastic/obs-ux-management-team
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@
"@kbn/shared-ux-storybook-config": "link:packages/shared-ux/storybook/config",
"@kbn/shared-ux-storybook-mock": "link:packages/shared-ux/storybook/mock",
"@kbn/shared-ux-tabbed-modal": "link:packages/shared-ux/modal/tabbed",
"@kbn/shared-ux-table-persist": "link:packages/shared-ux/table_persist",
"@kbn/shared-ux-utility": "link:packages/kbn-shared-ux-utility",
"@kbn/slo-plugin": "link:x-pack/plugins/observability_solution/slo",
"@kbn/slo-schema": "link:x-pack/packages/kbn-slo-schema",
Expand Down
51 changes: 51 additions & 0 deletions packages/shared-ux/table_persist/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# @kbn/shared-ux-table-persist

This package contains the `useEuiTablePersist` hook that can be used in components
containing Eui tables for storing their page size ("rows per page" value) or
the current sort criteria in local storage so that the user's preferences for
these properties can persist.

The package also exports some table-related constants (e.g. `DEFAULT_PAGE_SIZE_OPTIONS`)
for use across tables in Kibana Stack Management for consistency.

Usage:

```
interface Props {
items: T[];
}

const MyTableComponent: FunctionComponent<Props> = ({ items }) => {
const columns = [
{
field: 'name',
name: 'Name',
sortable: true,
},
...
];

const { pageSize, sorting, onTableChange } = useEuiTablePersist<T>({
tableId: 'myTableId',
initialPageSize: 50,
initialSort: {
field: 'name',
direction: 'asc',
},
});

const pagination = {
pageSize,
pageSizeOptions: DEFAULT_PAGE_SIZE_OPTIONS,
};

return (
<EuiInMemoryTable
items={items}
columns={columns}
pagination={pagination}
sorting={sorting}
/>
);
};
```
9 changes: 9 additions & 0 deletions packages/shared-ux/table_persist/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { useEuiTablePersist, DEFAULT_PAGE_SIZE_OPTIONS } from './src';
13 changes: 13 additions & 0 deletions packages/shared-ux/table_persist/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test',
rootDir: '../../..',
roots: ['<rootDir>/packages/shared-ux/table_persist'],
};
5 changes: 5 additions & 0 deletions packages/shared-ux/table_persist/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "shared-common",
"id": "@kbn/shared-ux-table-persist",
"owner": "@elastic/appex-sharedux"
}
6 changes: 6 additions & 0 deletions packages/shared-ux/table_persist/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@kbn/shared-ux-table-persist",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
12 changes: 12 additions & 0 deletions packages/shared-ux/table_persist/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const DEFAULT_PAGE_SIZE_OPTIONS = [25, 50, 100];
export const DEFAULT_INITIAL_PAGE_SIZE = 50;

export const LOCAL_STORAGE_PREFIX = 'tablePersist';
10 changes: 10 additions & 0 deletions packages/shared-ux/table_persist/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { useEuiTablePersist } from './use_table_persist';
export { DEFAULT_PAGE_SIZE_OPTIONS } from './constants';
48 changes: 48 additions & 0 deletions packages/shared-ux/table_persist/src/storage.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

type IStorageEngine = typeof window.localStorage;

export class Storage {
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
constructor(private readonly engine: IStorageEngine, private readonly prefix: string) {}

encode(val: unknown) {
return JSON.stringify(val);
}

decode(val: string | null) {
if (typeof val === 'string') {
return JSON.parse(val);
}
}

encodeKey(key: string) {
return `${this.prefix}:${key}`;
}

set(key: string, val: unknown) {
this.engine.setItem(this.encodeKey(key), this.encode(val));
return val;
}

has(key: string) {
return this.engine.getItem(this.encodeKey(key)) != null;
}

get<T>(key: string, _default?: T) {
if (this.has(key)) {
return this.decode(this.engine.getItem(this.encodeKey(key)));
} else {
return _default;
}
}
}

export function createStorage(deps: { engine: IStorageEngine; prefix: string }) {
return new Storage(deps.engine, deps.prefix);
}
15 changes: 15 additions & 0 deletions packages/shared-ux/table_persist/src/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export interface PersistData<T> {
pageSize?: number;
sort?: {
field: keyof T;
direction: 'asc' | 'desc';
};
}
81 changes: 81 additions & 0 deletions packages/shared-ux/table_persist/src/use_table_persist.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { Criteria } from '@elastic/eui';
import { PropertySort } from '@elastic/eui/src/services/sort/property_sort';
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
import {
DEFAULT_INITIAL_PAGE_SIZE,
LOCAL_STORAGE_PREFIX,
DEFAULT_PAGE_SIZE_OPTIONS,
} from './constants';
import { createStorage } from './storage';
import { validatePersistData } from './validate_persist_data';
import { PersistData } from './types';

interface EuiTablePersistProps<T> {
/** A unique id that will be included in the local storage variable for this table. */
tableId: string;
/** (Optional) Specifies a custom onTableChange handler. */
customOnTableChange?: (change: Criteria<T>) => void;
/** (Optional) Specifies a custom initial table sorting. */
initialSort?: PropertySort;
/** (Optional) Specifies a custom initial page size for the table.
* Defaults to {@link DEFAULT_INITIAL_PAGE_SIZE} */
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
initialPageSize?: number;
/** (Optional) Specifies custom page size options for the table.
* Defaults to {@link DEFAULT_PAGE_SIZE_OPTIONS} */
pageSizeOptions?: number[];
}

/**
* A hook that stores and retrieves from local storage the table page size and sort criteria.
* Returns the persisting page size and sort and the onTableChange handler that should be passed
* as props to an Eui table component.
*/
export const useEuiTablePersist = <T extends object>({
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
tableId,
customOnTableChange,
initialSort,
initialPageSize,
pageSizeOptions,
}: EuiTablePersistProps<T>) => {
const storage = createStorage({
engine: window.localStorage,
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
prefix: LOCAL_STORAGE_PREFIX,
});

const storedPersistData = storage.get(tableId, undefined);

const { pageSize: storagePageSize, sort: storageSort }: PersistData<T> = validatePersistData(
storedPersistData,
pageSizeOptions ?? DEFAULT_PAGE_SIZE_OPTIONS
);

const pageSize = storagePageSize ?? initialPageSize ?? DEFAULT_INITIAL_PAGE_SIZE;
const sort = storageSort ?? initialSort;
const sorting = sort && { sort };

const onTableChange = (nextValues: Criteria<T>) => {
if (customOnTableChange) {
customOnTableChange(nextValues);
}
const newPersistData: PersistData<T> = {};
newPersistData.pageSize = nextValues.page?.size ?? storagePageSize;
const newSort = nextValues?.sort;
newPersistData.sort = newSort?.field
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not easy to parse the double ternary, could we make it more explicit like the below suggestion? Also I would keep the "next" prefix everywhere instead of "new" and "next"

let nextSort: Criteria<T>['sort'];
if (nextValues.sort?.field) {
  nextSort = nextValues.sort;
} else if (!nextValues.sort?.direction) {
  // If both field and direction are undefined, there is no change on sort so use the stored one
  nextSort = storageSort;
}

const nextPageSize = nextValues.page?.size ?? storagePageSize;

const nextPersistData: PersistData<T> = {
  pageSize: nextPageSize,
  sort: nextSort,
};

if (nextPersistData.pageSize !== storagePageSize || nextPersistData.sort !== storageSort) {
  storage.set(tableId, nextPersistData);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I updated it as suggested, with some modifications due to some recent changes.

On a side note, I just want to note that with the current logic, if the consumer provides an initial sort, when the user removes a sort (the change criteria would contain just the direction property, with undefined field), then we store undefined sort in local storage, so next time when the user visits this page, the initial sort will be restored (rather than no sort to be preserved). Just wanted to make sure this is the desired behaviour.

? newSort
: newSort?.direction
? undefined // If field is an empty string and direction has value, it means sort is removed
: storageSort; // If both field and direction are undefined, there is no change on sort so use the stored one
if (newPersistData.pageSize !== storagePageSize || newPersistData.sort !== storageSort) {
storage.set(tableId, newPersistData);
}
};

return { pageSize, sorting, onTableChange };
};
38 changes: 38 additions & 0 deletions packages/shared-ux/table_persist/src/validate_persist_data.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

/**
* A utility function used to validate a table persist data.
* If any of the properties is not valid, it is returned with undefined value.
*
* @param data The data to be validated
* @param pageSizeOptions The table page size options that are available
*/
export const validatePersistData = (data: any, pageSizeOptions: number[]) => {
const pageSize = data?.pageSize;
const sort = data?.sort;

let validatedPageSize = pageSize;
let validatedSort = sort;

if (pageSize) {
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
if (typeof pageSize !== 'number' || !pageSizeOptions.includes(pageSize)) {
validatedPageSize = undefined;
}
}

if (sort) {
const field = sort.field;
const direction = sort.direction;
if (!(typeof field === 'string') || !(direction === 'asc' || direction === 'desc')) {
ElenaStoeva marked this conversation as resolved.
Show resolved Hide resolved
validatedSort = undefined;
}
}

return { pageSize: validatedPageSize, sort: validatedSort };
};
Loading