"onChange" action doesn't informative

[epotockiy]

onChange function get only one parameter uuid which doesn't informative for outside components, could you replace it by an accordion item index or add this index as the second parameter of the onChange function

[ryami333]

Gidday @epotockiy - thanks for your issue :)

This feature is actually not quite as trivial as it may seem on the surface. The complexity comes down to the concept of 'order'. In other words, determining which is the 0th (and subsequent) AccordionItem in the application.

Order by... order.

<Accordion>
    <AccordionItem>Zero</AccordionItem>
    <AccordionItem>One</AccordionItem>
    <AccordionItem>Two</AccordionItem>
</Accordion>

Easy, right? The 0th is the 0th, and the 2nd is the 2nd. In this very simple case, we could use React.children to cycle through the first-generation children of Accordion, and use the array index as uuid.

But what if...

Order by depth

<Accordion>
    <span>
        <span>
            <AccordionItem>Foo</AccordionItem>
        </span>
    </span>
   <AccordionItem>Bar</AccordionItem>
</Accordion>

So, which is the 0th AccordionItem here? You might think that the obvious answer is Foo because it's the top-most, reading down the DOM. To the next person though, the intuitive answer might be Bar because it's closer to the root Accordion component in terms of depth. Whichever way we choose could be somewhat contentious.

That's just a minor detail though. Whichever way we went, I don't know how we would actually implement this anyway. I'm not sure whether you can actually use React.children recursively, and even if you could I would be concerned about performance in large DOM trees.

There's one more domain to cater for too, perhaps the most complex one:

Order by time

class App extends Component {
    state = {
        loaded: false,
    }

    componentDidMount() {
        setTimeout(
            () => this.setState({ loaded: true }),
            1000
        );
    }

    render() {
        return (
            <Accordion>
                {loaded && (
                    <AccordionItem>Foo</AccordionItem>
                )
                <AccordionItem>Bar</AccordionItem>
            </Accordion>
        )
    }
}

So which is the 0th AccordionItem here? Foo because it's 'first' in the DOM or Bar because it mounted one second before Foo? Again, the answer which seems intuitive to you might seem counterintuitive to the next user.

So where does that leave us?

So, yes - I agree that the uuid provided by onChange is not informative, but it's also not obvious how to make it informative. Currently, AccordionItems generate their own uuid behind-the-scenes. But we might have a solution: customisable (but optional) uuid prop on AccordionItem, so we leave it up to the user to decide what's informative to them (but also the added responsibility of ensuring that uuids are unique).

Eg:

/* Copy and paste warning! The following feature (`uuid` prop) is not yet implemented! */

const items = [
    { uuid: 'foo', title: 'Foo!' },
    { uuid: 'bar', title: 'Bar!' },
];

return (
    <Accordion>
        {items.map(item => (
            <AccordionItem uuid={item.uuid}>{item.title}</AccordionItem>
        )}
    </Accordion>
);

Then, onChange would be passed foo or bar.

Would this kind of solution suit your needs?

[epotockiy]

Yes it is. It would be perfect if you improvement implemented in the nearest future.

[ryami333]

Thanks for your feedback @epotockiy - we have this on our roadmap 😄

[vincentaudebert]

@epotockiy feel free to contribute with a PR if you want :)

[vincentaudebert]

This will go in v2.4.0