Accordion doesn't support JSX content

[Aaronius]

🐛 Bug Report

The Accordion component doesn't seem to support JSX content.

🤔 Expected Behavior

I would expect that when I render this:

<Accordion>
  <Item title="Thing">
    <p>paragraph content</p>
  </Item>
</Accordion>

that I would get an accordion that works properly by allowing me to expand and collapse the item.

😯 Current Behavior

I get the error Unknown element <p> in collection.. If I remove the p tag and leave the paragraph content text, the accordion works fine.

💁 Possible Solution

🔦 Context

I'm trying to display content that's more complex than a simple string of text.

💻 Code Sample

https://codesandbox.io/s/accordion-content-j4q5i?file=/src/App.js

🌍 Your Environment

Software	Version(s)
react-spectrum	3.0.0-alpha.1 of the accordion component
Browser	Chrome 90
Operating System	Mac Catalina

🧢 Your Company/Team

Adobe Experience Platform Data Collection

🕷 Tracking Issue (optional)

[snowystinger]

it appears to be that the act of opening a leaf node causes the issue
given that all the elements within an Accordion will be leafs, we either need to fix that code in useTreeState so it doesn’t break or we need to manage our own open state, I haven’t looked at useTreeState to see if it’s better to fix it there or not

in addition, instead of item.props.children, we should really be using item.rendered like MenuItem does
https://github.com/adobe/react-spectrum/blob/main/packages/%40react-spectrum/accordion/src/Accordion.tsx#L93 -> https://github.com/adobe/react-spectrum/blob/main/packages/%40react-spectrum/menu/src/MenuItem.tsx#L74

a simple way to verify what I’m saying is to comment out the useAccordion https://github.com/adobe/react-spectrum/blob/main/packages/%40react-aria/accordion/src/useAccordion.ts#L50
and set isOpen to true in AccordionItem, it renders open just fine, it’s only when you click to toggle that things go south

[snowystinger]

@Aaronius just copying our conversation here as well
Author:
Okay, interesting. Well, I’ll share what I’ve found, though it’s not entirely clear how it intersects with what you wrote and you probably already are aware of it. When you expand the AccordionItem, TreeCollection is being created and visits each item that is expanded (

react-spectrum/packages/@react-stately/tree/src/TreeCollection.ts

Lines 29 to 33 in 4b75cbe

	if (node.childNodes && (node.type === 'section' || expandedKeys.has(node.key))) {
	for (let child of node.childNodes) {
	visit(child);
	}
	}

). In that same snippet, it calls node.childNodes in order to iterate over each child node (because those child nodes may also be expanded items, I’m assuming). The node.childNodes property is this one here:

react-spectrum/packages/@react-stately/collections/src/CollectionBuilder.ts

Lines 181 to 200 in 4b75cbe

	childNodes: iterable(function *() {
	if (!partialNode.hasChildNodes) {
	return;
	}
	let index = 0;
	for (let child of partialNode.childNodes()) {
	// Ensure child keys are globally unique by prepending the parent node's key
	if (child.key != null) {
	child.key = `${node.key}${child.key}`;
	}
	child.index = index;
	let nodes = builder.getFullNode(child, builder.getChildState(state, child), node.key, node);
	for (let node of nodes) {
	index++;
	yield node;
	}
	}
	})

which calls builder.getFullNode for the p child, which is where it errors out because type is p and there’s no getCollectionNode on the p component.
The reason we don’t get the issue with Menu is because there’s never an item node in expandedKeys, so node.childNodes is never accessed on the item (see the first snippet I shared).

[snowystinger]

I think the intersection here is that the TreeCollection lines you noted are for things like Section, which would have a structure like this -> <Section><Item/><Item/></Section>
the check for expandedKeys.has(node.key) isn’t quite right though for things that should be a leaf, i think the distinction and reason I’m suggesting it may be better to track this state separately of the treeState is that expandedKeys has a meaning that it expects elements like Section and like Item inside those keys

[AatiqUrRehman]

any workaround for this issue to show jsx content inside accordion body.

[nozokada]

It apparently has to do with JSX with non-empty title so here's my hacky workaround. Not a good solution if you need the title to still appear when it's expanded.
https://codesandbox.io/s/accordion-content-forked-ncf8lo?file=/src/App.js

<Accordion
  // Setting Accordion item title to empty when it's expanded
  // to work around the issue that it breaks on rendering
  // JSX content with non-empty title
  // https://github.com/adobe/react-spectrum/issues/1989
  onExpandedChange={(keys) => {
    keys.forEach((key) => {
      accordionItemTitles = { ...accordionItemTitles, [key]: "" };
    });

    Object.keys(titles).forEach((key) => {
      if (!keys.has(key)) {
        accordionItemTitles = {
          ...accordionItemTitles,
          [key]: titles[key]
        };
      }
    });

    setAccordionItemTitles(accordionItemTitles);
  }}
>
  <Item key="item 1" title={accordionItemTitles["item 1"]}>
    <p>Item 1 Content</p>
  </Item>
  <Item key="item 2" title={accordionItemTitles["item 2"]}>
    <p>Item 2 Content</p>
  </Item>
  <Item key="item 3" title={accordionItemTitles["item 3"]}>
    <p>Item 3 Content</p>
  </Item>
</Accordion>

[danielsimao]

It apparently has to do with JSX with non-empty title so here's my hacky workaround. Not a good solution if you need the title to still appear when it's expanded. https://codesandbox.io/s/accordion-content-forked-ncf8lo?file=/src/App.js

<Accordion
  // Setting Accordion item title to empty when it's expanded
  // to work around the issue that it breaks on rendering
  // JSX content with non-empty title
  // https://github.com/adobe/react-spectrum/issues/1989
  onExpandedChange={(keys) => {
    keys.forEach((key) => {
      accordionItemTitles = { ...accordionItemTitles, [key]: "" };
    });

    Object.keys(titles).forEach((key) => {
      if (!keys.has(key)) {
        accordionItemTitles = {
          ...accordionItemTitles,
          [key]: titles[key]
        };
      }
    });

    setAccordionItemTitles(accordionItemTitles);
  }}
>
  <Item key="item 1" title={accordionItemTitles["item 1"]}>
    <p>Item 1 Content</p>
  </Item>
  <Item key="item 2" title={accordionItemTitles["item 2"]}>
    <p>Item 2 Content</p>
  </Item>
  <Item key="item 3" title={accordionItemTitles["item 3"]}>
    <p>Item 3 Content</p>
  </Item>
</Accordion>

This is possible workaround but affects the component accessibility. I would really appreciate if this issue could be solved.

[binaryartifex]

Just discovered this issue. I submitted a duplicate (#3882) just a few hours ago. The accordian/disclosure component is a very widely used component and its a damn shame that ive had my team going gang busters with react-aria until we got a flying head kick to the face with a +12 month old issue. Completely understand devs are under the pump, but this is such a prevalent component pattern its a shame that its been tucked away in the backlog basement for so long...

[snowystinger]

Related #3817
We do accept contributions. Accordion isn’t particularly high on our list at the moment because there are well-defined aria patterns using dl, dd, and dt elements. If there are cross-browser inconsistencies with it, that would be a good candidate for contributing something.

[jbltx]

Answered in #3882, the quick workaround is to specify hasChildItems={false} on every Item component. I believe the problem comes from this statement.
Obviously, it is just a workaround until a real fix is done.

<Accordion
      {...props}
      expandedKeys={openKeys}
      onExpandedChange={setOpenKeys}
    >
      <Item key="files" title="Your files" hasChildItems={false}>
        <p>files</p>
      </Item>
      <Item key="shared" title="Shared with you" hasChildItems={false}>
        <p>shared</p>
      </Item>
      <Item key="last" title="Last item" hasChildItems={false}>
        <p>last</p>
      </Item>
  </Accordion>

[danielsimao]

Answered in #3882, the quick workaround is to specify hasChildItems={false} on every Item component. I believe the problem comes from this statement. Obviously, it is just a workaround until a real fix is done.

<Accordion
      {...props}
      expandedKeys={openKeys}
      onExpandedChange={setOpenKeys}
    >
      <Item key="files" title="Your files" hasChildItems={false}>
        <p>files</p>
      </Item>
      <Item key="shared" title="Shared with you" hasChildItems={false}>
        <p>shared</p>
      </Item>
      <Item key="last" title="Last item" hasChildItems={false}>
        <p>last</p>
      </Item>
  </Accordion>

It worked perfectly! Thank you @jbltx. Thank you react-spectrum for open-source all these great solutions. Cheers

[LFDanLu]

Just to reiterate on what @snowystinger mentioned previously: Accordion isn't on our near term milestones so we are open to contributions for this/Accordion in general.

[steveoh]

there are well-defined aria patterns using dl, dd, and dt elements.

I can't find these patterns or I misunderstood that they accomplish the accordion use case. Would you clarify and share some docs or examples?

[snowystinger]

Looks like I misspoke from memory. This is what I was actually thinking of #6635 (reply in thread)