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

SelectItem missing ref prop while ListboxItem implements/exposes it. #6949

Closed
AnthonyPaulO opened this issue Aug 26, 2024 · 8 comments
Closed
Labels
waiting Waiting on Issue Author

Comments

@AnthonyPaulO
Copy link

Provide a general summary of the issue here

Unlike ListBoxItem, SelectItem doesn't implement/expose a ref prop.

🤔 Expected Behavior?

SelectItem should implement/expose a ref prop.

😯 Current Behavior

ref prop is missing.

💁 Possible Solution

Implement ref prop.

🔦 Context

No response

🖥️ Steps to Reproduce

The following is a sandbox link to a NextUI implementation that uses an AccordionItem component, which is really just an Aria SelectItem component behind the scenes. As you can see, it's missing the ref prop due to the underlying SelectItem component not implementing/exposing it.

https://stackblitz.com/edit/stackblitz-starters-yxjqtp?file=app%2Fpage.tsx

Version

React Aria 3.34.3

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Windows 11

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

@snowystinger
Copy link
Member

Thanks for the Issue. Is this actually one of our components? https://github.com/nextui-org/nextui/blob/canary/packages/components/accordion/src/accordion-item.tsx I'm not seeing any of our components in this file.

Also, not sure what a SelectItem is, that's not one of our components. We have one in a tailwind starter, but that can be edited and changed as you need.

The stackblitz doesn't seem to be building. Can you check you saved it in a good state?

@xylish7
Copy link

xylish7 commented Aug 28, 2024

was the one to raise the issue in the nextui repo (nextui-org/nextui#3498). The stackbliz won't build because for whatever reasons it doesn't want to render the AccordionItem, but The issue is with that ref prop missing on AccordionItem (the IDE is highlighting it).

I also told @AnthonyPaulO that the AccordionItem doesn't seem to implement any SelectItem. You can follow the entire conversation here: nextui-org/nextui#3498.

P.S. @snowystinger I really appreciate your quick responses to issues 🙏

@snowystinger
Copy link
Member

We can wait for them to explain.

In the meantime, I think you're just getting a Typescript error? is it not actually attached either?

I see the AccordionItem does take a ref https://github.com/nextui-org/nextui/blob/canary/packages/components/accordion/src/accordion-item.tsx#L13 so I'm a bit surprised by the TS error.

It looks like it's passed through to a button https://github.com/nextui-org/nextui/blob/4f8ae50cf441da5b5685b6573714a3f7c9ab3ea2/packages/components/accordion/src/use-accordion-item.ts#L158 so I'd be surprised if it wasn't attached either.

Are you on an older version of Next? it looks like they used to use a different forwardRef signature about a year ago from looking at the git blame.

@snowystinger snowystinger added the waiting Waiting on Issue Author label Aug 28, 2024
@xylish7
Copy link

xylish7 commented Aug 29, 2024

I thought it was only an issue with Typescript but it was not actually being forwarded. I will upgrade all dependencies to latest and come back with the result.

@xylish7
Copy link

xylish7 commented Aug 29, 2024

Here is a stackblizz example that shows my problem: https://stackblitz.com/edit/vitejs-vite-pfvddt?file=src%2FApp.tsx.

It seems like it's not a typescript issue, @AnthonyPaulO please confirm that it is indeed a react-aria issue and not a nextui one.

image

@AnthonyPaulO
Copy link
Author

AnthonyPaulO commented Sep 2, 2024

We can wait for them to explain.

In the meantime, I think you're just getting a Typescript error? is it not actually attached either?

I see the AccordionItem does take a ref https://github.com/nextui-org/nextui/blob/canary/packages/components/accordion/src/accordion-item.tsx#L13 so I'm a bit surprised by the TS error.

It looks like it's passed through to a button https://github.com/nextui-org/nextui/blob/4f8ae50cf441da5b5685b6573714a3f7c9ab3ea2/packages/components/accordion/src/use-accordion-item.ts#L158 so I'd be surprised if it wasn't attached either.

Are you on an older version of Next? it looks like they used to use a different forwardRef signature about a year ago from looking at the git blame.

Sorry I was on vacation, back now.

I have to go back and see what the exact issue was, but now that I think about it and recall a bit more I believe it was due to aria library not exposing the ref property in the typescript definition file. I'll check after I settle back in, but I think it missing the ref prop definition was the issue.

@AnthonyPaulO
Copy link
Author

AnthonyPaulO commented Sep 4, 2024

Okay I don't know what I was smoking, I dug through this and must have confused this with another issue so I'm closing this bug report. The real issue is that useTreeState wipes out the children refs, so it's an issue with our usage of Adobe's React Stately and not with the components themselves. My apologies!

@xylish7
Copy link

xylish7 commented Sep 10, 2024

Thanks for looking into it again and also thanks @snowystinger!

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

No branches or pull requests

5 participants
@snowystinger @AnthonyPaulO @xylish7 and others