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

AccordionItem does not forward ref #3498

Open
xylish7 opened this issue Jul 18, 2024 · 13 comments · May be fixed by #3718
Open

AccordionItem does not forward ref #3498

xylish7 opened this issue Jul 18, 2024 · 13 comments · May be fixed by #3718

Comments

@xylish7
Copy link

xylish7 commented Jul 18, 2024

NextUI Version

@nextui-org/[email protected]

Describe the bug

Accordion item won't accept a ref prop.

image

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

  1. Adding ref prop to AccordionItem will result in ref not being passed to the AccordionItem since it does not support it.

Expected behavior

AccordionItem should accept the ref prop.

Screenshots or Videos

No response

Operating System Version

Windows

Browser

Chrome

Copy link

linear bot commented Jul 18, 2024

@Tejas-Sands
Copy link

can you give more context , as i am not getting this error
Screenshot 2024-07-18 195842

@xylish7
Copy link
Author

xylish7 commented Jul 22, 2024

Are you able to forward ref to AccordionItems? Will try to recreate the issue in a sandbox 👍

@xylish7
Copy link
Author

xylish7 commented Jul 22, 2024

image

Here is a stack blitz: https://stackblitz.com/edit/stackblitz-starters-yxjqtp?file=app%2Fpage.tsx

Not sure why it fails to render the accordion (rendering a button works), but I don't care about what it renders. The issue is that AccordionItems is not taking a ref parameter.

@AnthonyPaulO
Copy link
Contributor

This is an issue with the underlying aria component library, as their SelectItem component does not expose ref for some reason, unlike the ListboxItem which does.

@xylish7
Copy link
Author

xylish7 commented Jul 29, 2024

I see. Was it reported by someone from nextui team to the aria team?

@xylish7
Copy link
Author

xylish7 commented Aug 26, 2024

@AnthonyPaulO any news on this? I've seen that the AccordionItem is using a button, so not sue what SelectItem has to do with this issue.

@AnthonyPaulO
Copy link
Contributor

AnthonyPaulO commented Aug 26, 2024

@AnthonyPaulO any news on this? I've seen that the AccordionItem is using a button, so not sue what SelectItem has to do with this issue.

AccordionItem itself is a SelectItem behind the scenes, which is an Adobe Aria library component, and it does not expose a ref property. I'm not really a maintainer here so I don't have any relationship with the Aria team, but I guess I could post it as an issue on their site. I'll do that.

@xylish7
Copy link
Author

xylish7 commented Aug 26, 2024

Thanks, as you have more insights into how it works! 🙏🏻

@AnthonyPaulO
Copy link
Contributor

Thanks, as you have more insights into how it works! 🙏🏻

I created the issue here: adobe/react-spectrum#6949

@AnthonyPaulO
Copy link
Contributor

AnthonyPaulO commented Sep 4, 2024

Okay never mind, I must have confused this with another issue so I closed the adobe react issue I created.

After a lot of digging and debugging, the real cause of the ref not working is that it's being wiped out in the useTreeState call in use-accordion.ts. Apparently the Accordion's children are cloned and passed into useTreeState and become a managed collection. This collection is then iterated over to create the individual AccordionItem elements, but the ref of each child in the collection had been stripped out during the useTreeState call so it never gets passed on.

This also occurs in ListBox via the useListState call, so those children can't have refs either, and a quick check shows it's not even implemented though it would be a quick thing to do. I presume all other components with children that use React Stately useXXXState methods suffer from the same ref-stripping issues.

AnthonyPaulO pushed a commit to AnthonyPaulO/nextui that referenced this issue Sep 5, 2024
@AnthonyPaulO AnthonyPaulO linked a pull request Sep 5, 2024 that will close this issue
AnthonyPaulO pushed a commit to AnthonyPaulO/nextui that referenced this issue Sep 5, 2024
@snowystinger
Copy link

I don't think useTreeState is at fault here, we use that hook for our Menu and I can see our ref is maintained
https://codesandbox.io/p/sandbox/s6rs79

One of these is likely missing passing the ref along:

React.Children.map(childrenProp, (child) => {
this one doesn't pass along the ref (might be ok in react 19, can't recall)

Feel free to have a look through our source for more ideas.

@xylish7
Copy link
Author

xylish7 commented Sep 30, 2024

@AnthonyPaulO any news on this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants