Skip to content

chore: fix types for pagination navigation #10263

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

Merged
merged 1 commit into from
Apr 19, 2024
Merged

chore: fix types for pagination navigation #10263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 12 additions & 12 deletions packages/react-core/src/components/Pagination/Navigation.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export interface NavigationProps extends React.HTMLProps<HTMLElement> {
/** Label for the English word "of". */
ofWord?: string;
/** The number of the current page. */
page: string | number;
page: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

lgtm overall but cc @tlabaj to double check that this prop change isn't also a breaking change (while onPageInput was a type fix).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This interface is never exported as a public API, so this will not be a breaking change.

/** The title of a page displayed beside the page number. */
pagesTitle?: string;
/** The title of a page displayed beside the page number (the plural form). */
Expand All @@ -55,13 +55,13 @@ export interface NavigationProps extends React.HTMLProps<HTMLElement> {
/** Function called when user clicks to navigate to previous page. */
onPreviousClick?: (event: React.SyntheticEvent<HTMLButtonElement>, page: number) => void;
/** Function called when user inputs page number. */
onPageInput?: (event: React.SyntheticEvent<HTMLButtonElement>, page: number) => void;
onPageInput?: (event: React.KeyboardEvent<HTMLInputElement>, page: number) => void;
/** Function called when page is changed. */
onSetPage: OnSetPage;
}

export interface NavigationState {
userInputPage?: React.ReactText;
userInputPage?: number | string;
}

class Navigation extends React.Component<NavigationProps, NavigationState> {
Expand Down Expand Up @@ -93,7 +93,7 @@ class Navigation extends React.Component<NavigationProps, NavigationState> {
onPageInput: () => undefined as any
};

private static parseInteger(input: React.ReactText, lastPage: number): number {
private static parseInteger(input: number | string, lastPage: number): number {
// eslint-disable-next-line radix
let inputPage = Number.parseInt(input as string, 10);
if (!Number.isNaN(inputPage)) {
Expand All @@ -105,14 +105,14 @@ class Navigation extends React.Component<NavigationProps, NavigationState> {

private onChange(event: React.FormEvent<HTMLInputElement>, lastPage: number): void {
const inputPage = Navigation.parseInteger(event.currentTarget.value, lastPage);
this.setState({ userInputPage: Number.isNaN(inputPage as number) ? event.currentTarget.value : inputPage });
this.setState({ userInputPage: Number.isNaN(inputPage) ? event.currentTarget.value : inputPage });
}

private onKeyDown(
event: React.KeyboardEvent<HTMLInputElement>,
page: number | string,
page: number,
lastPage: number,
onPageInput: (event: React.SyntheticEvent<HTMLButtonElement>, page: number) => void
onPageInput: (event: React.KeyboardEvent<HTMLInputElement>, page: number) => void
): void {
const allowedKeys = [
'Tab',
Expand All @@ -126,9 +126,9 @@ class Navigation extends React.Component<NavigationProps, NavigationState> {
'ArrowDown'
];
if (event.key === KeyTypes.Enter) {
const inputPage = Navigation.parseInteger(this.state.userInputPage, lastPage) as number;
onPageInput(event, Number.isNaN(inputPage) ? (page as number) : inputPage);
this.handleNewPage(event, Number.isNaN(inputPage) ? (page as number) : inputPage);
const inputPage = Navigation.parseInteger(this.state.userInputPage, lastPage);
onPageInput(event, Number.isNaN(inputPage) ? page : inputPage);
this.handleNewPage(event, Number.isNaN(inputPage) ? page : inputPage);
} else if (!/^\d*$/.test(event.key) && !allowedKeys.includes(event.key)) {
event.preventDefault();
}
Expand Down Expand Up @@ -206,7 +206,7 @@ class Navigation extends React.Component<NavigationProps, NavigationState> {
isDisabled={isDisabled || page === firstPage || page === 0}
data-action="previous"
onClick={(event) => {
const newPage = (page as number) - 1 >= 1 ? (page as number) - 1 : 1;
const newPage = page - 1 >= 1 ? page - 1 : 1;
onPreviousClick(event, newPage);
this.handleNewPage(event, newPage);
this.setState({ userInputPage: newPage });
Expand Down Expand Up @@ -244,7 +244,7 @@ class Navigation extends React.Component<NavigationProps, NavigationState> {
aria-label={toNextPageAriaLabel}
data-action="next"
onClick={(event) => {
const newPage = (page as number) + 1 <= lastPage ? (page as number) + 1 : lastPage;
const newPage = page + 1 <= lastPage ? page + 1 : lastPage;
onNextClick(event, newPage);
this.handleNewPage(event, newPage);
this.setState({ userInputPage: newPage });
Expand Down
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Pagination/Pagination.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export interface PaginationProps extends React.HTMLProps<HTMLDivElement>, OUIAPr
/** Function called when user clicks on navigate to next page. */
onNextClick?: (event: React.SyntheticEvent<HTMLButtonElement>, page: number) => void;
/** Function called when user inputs page number. */
onPageInput?: (event: React.SyntheticEvent<HTMLButtonElement>, page: number) => void;
onPageInput?: (event: React.KeyboardEvent<HTMLInputElement>, page: number) => void;
Copy link
Contributor Author

@jonkoops jonkoops Apr 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is a breaking change to the type of a public API, but the type that was here before was incorrect.

/** Function called when user selects number of items per page. */
onPerPageSelect?: OnPerPageSelect;
/** Function called when user clicks on navigate to previous page. */
Expand Down