nextui icon indicating copy to clipboard operation
nextui copied to clipboard

AccordionItem does not forward ref

Open xylish7 opened this issue 1 year ago • 10 comments

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

xylish7 avatar Jul 18 '24 07:07 xylish7

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

Tejas-Sands avatar Jul 18 '24 14:07 Tejas-Sands

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

xylish7 avatar Jul 22 '24 12:07 xylish7

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.

xylish7 avatar Jul 22 '24 13:07 xylish7

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.

AnthonyPaulO avatar Jul 24 '24 16:07 AnthonyPaulO

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

xylish7 avatar Jul 29 '24 09:07 xylish7

@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.

xylish7 avatar Aug 26 '24 17:08 xylish7

@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.

AnthonyPaulO avatar Aug 26 '24 18:08 AnthonyPaulO

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

xylish7 avatar Aug 26 '24 19:08 xylish7

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

I created the issue here: https://github.com/adobe/react-spectrum/issues/6949

AnthonyPaulO avatar Aug 26 '24 20:08 AnthonyPaulO

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 avatar Sep 04 '24 22:09 AnthonyPaulO

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:

https://github.com/nextui-org/nextui/blob/5fd001c241a9324c246fdadfe23a332cc051bc88/packages/components/accordion/src/use-accordion.ts#L133 this one doesn't pass along the ref (might be ok in react 19, can't recall)

https://github.com/nextui-org/nextui/blob/5fd001c241a9324c246fdadfe23a332cc051bc88/packages/components/accordion/src/accordion.tsx#L38

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

snowystinger avatar Sep 11 '24 00:09 snowystinger

@AnthonyPaulO any news on this one?

xylish7 avatar Sep 30 '24 22:09 xylish7

closing - inactivity

wingkwong avatar Jan 02 '25 06:01 wingkwong

Is there a known workaround for this?

mohidmakhdoomi avatar Jan 21 '25 17:01 mohidmakhdoomi

@mohidmakhdoomi that's how I managed to work-around it to add my defaults, I hope it helps.

wrapper:

export const Accordion = (props: AccordionProps) => {
  return <HerouiAccordion {...props} data-heroui-force="accordion" />
}

globals.css:

@layer base {
  [data-heroui-force="accordion"] {
    @apply p-4;
  }

  [data-heroui-force="accordion"] [data-slot="content"] {
    @apply p-0;
  }

  [data-heroui-force="accordion"] [data-slot="trigger"] {
    @apply p-0;
  }

  [data-heroui-force="accordion"] [data-slot="base"] {
    @apply p-2;
  }
}

gofri avatar Jul 21 '25 15:07 gofri