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][FocusTrap] getTabbable function has a return type of string[] instead of HTMLElement[] #42231

Closed
KalmarLorand opened this issue May 14, 2024 · 6 comments · Fixed by #42237
Labels
bug 🐛 Something doesn't work component: FocusTrap The React component. good first issue Great for first contributions. Enable to learn the contribution process. package: material-ui Specific to @mui/material typescript

Comments

@KalmarLorand
Copy link
Contributor

KalmarLorand commented May 14, 2024

Steps to reproduce

Link to live example: (required)

Steps:

  1. Create a Focustrap component
  2. Pass in the getTabbable function that returns an array of nodes Returns an array of ordered tabbable nodes
  3. Notice the Typescript Linting error.

Current behavior

Based on the description of the getTabbable prop of the FocusTrap component:
Returns an array of ordered tabbable nodes (i.e. in tab order) within the root. For instance, you can provide the "tabbable" npm dependency.
However, the type definition of this function expects it to return a ReadonlyArray<string>.
When returning a HTML Element array, it throws:

Type '(root: HTMLElement) => HTMLAnchorElement[]' is not assignable to type '(root: HTMLElement) => readonly string[]'.
  Type 'HTMLAnchorElement[]' is not assignable to type 'readonly string[]'.
    Type 'HTMLAnchorElement' is not assignable to type 'string'.typescript(2322)
FocusTrap.types.d.ts(12, 5): The expected type comes from property 'getTabbable' which is declared here on type 'IntrinsicAttributes & Pick<FocusTrapProps, keyof FocusTrapProps> & Pick<InferProps<any>, string | number | symbol> & Pick<...>'

Expected behavior

The getTabbable prop should accept a function returning an array of HTMLElements.

Search keywords: getTabbable, FocusTrap

@KalmarLorand KalmarLorand added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 14, 2024
@mnajdova
Copy link
Member

Nice catch, would you like to introduce a PR with the change?

@mnajdova mnajdova added bug 🐛 Something doesn't work typescript component: FocusTrap The React component. good first issue Great for first contributions. Enable to learn the contribution process. and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 14, 2024
@KalmarLorand
Copy link
Contributor Author

Nice catch, would you like to introduce a PR with the change?

Sure thing. I'll open a PR today. thanks

@KalmarLorand
Copy link
Contributor Author

Nice catch, would you like to introduce a PR with the change?

I just created the PR:
42237

@PRANJALI-SANKPAL
Copy link

Is this issue resolved? If not I want to work on it

@kmr-ankitt
Copy link

Is this issue solved? I want to give it a try

@rkumar261
Copy link

Is this issue already resolved? If not, I'm interested in working on it.

@ZeeshanTamboli ZeeshanTamboli added the package: material-ui Specific to @mui/material label Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: FocusTrap The React component. 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.

6 participants