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

[material-ui][Pagination] getItemAriaLabel called with null page number contrary to type definition #43385

Closed
sydneyjodon-wk opened this issue Aug 20, 2024 · 2 comments · Fixed by #43399
Labels
component: pagination This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. package: material-ui Specific to @mui/material typescript

Comments

@sydneyjodon-wk
Copy link
Contributor

sydneyjodon-wk commented Aug 20, 2024

Steps to reproduce

Link to live example: https://codesandbox.io/p/devbox/88qvg2?file=%2Fsrc%2FDemo.tsx

Steps:

  1. Use getItemAriaLabel
  2. Check page args that are passed in on the initial render of Pagination

Current behavior

Hello 👋

It looks like the getItemAriaLabel prop on Pagination is typed as:

getItemAriaLabel?: (type: UsePaginationItem['type'], page: number, selected: boolean) => string;

with the non-null page param, but null seems to be coming through on initial render of Pagination (see demo):

Screenshot 2024-08-20 at 4 22 21 PM ###

Expected behavior

Would it be possible to either change the behavior of passing null in for page or else change the typing of getItemAriaLabel to make it clear that null is an option for page?

Context

We are trying to use strict null safety

Your environment

See https://codesandbox.io/p/devbox/88qvg2?file=%2Fsrc%2FDemo.tsx

Thank you!

Search keywords: getItemAriaLabel page null

@sydneyjodon-wk sydneyjodon-wk added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 20, 2024
@siriwatknp siriwatknp added bug 🐛 Something doesn't work typescript good first issue Great for first contributions. Enable to learn the contribution process. and removed bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 21, 2024
@siriwatknp
Copy link
Member

That's true.

Here is a suggested fix, PR welcome:

diff --git a/packages/mui-material/src/Pagination/Pagination.d.ts b/packages/mui-material/src/Pagination/Pagination.d.ts
index a20361d6b5..50d83916d3 100644
--- a/packages/mui-material/src/Pagination/Pagination.d.ts
+++ b/packages/mui-material/src/Pagination/Pagination.d.ts
@@ -46,7 +46,11 @@ export interface PaginationProps
    * @param {bool} selected If true, the current page is selected.
    * @returns {string}
    */
-  getItemAriaLabel?: (type: UsePaginationItem['type'], page: number, selected: boolean) => string;
+  getItemAriaLabel?: (
+    type: UsePaginationItem['type'],
+    page: UsePaginationItem['page'],
+    selected: boolean,
+  ) => string;
 
   /**
    * Render the item.

@sydneyjodon-wk
Copy link
Contributor Author

@siriwatknp thank you! Here's that PR #43399

@ZeeshanTamboli ZeeshanTamboli added package: material-ui Specific to @mui/material component: pagination This is the name of the generic UI component, not the React module! labels Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pagination This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. package: material-ui Specific to @mui/material typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants