Accordion Item: Console error thrown if item is in shadow dom

[paulcpederson]

Actual Behavior

If I have an accordion, and then a component that uses shadow dom inside which renders an accordion item, I see an error in the console

TypeError: Cannot read properties of null (reading 'querySelectorAll')
    at p.getItemPosition (p-3393c6b5.entry.js:6:6848)
    at p.componentDidLoad (p-3393c6b5.entry.js:6:4617)

getItemPosition calls querySelectorAll on the element's parent which is not valid here: https://github.com/Esri/calcite-components/blob/4b4d8b014665172d7e9ac08da4471af837a37b38/src/components/accordion-item/accordion-item.tsx#L295

Expected Behavior

I would expect the accordion element to be able to work across Shadow DOM boundaries. I think this would be as straight forward as using the closestElementCrossShadowBoundary utility to look up the parent accordion, rather than assuming it's there.

If that's not possible I think it would be fine to do an early return if the parent doesn't exist. The component seems to work fine (probably because we allow multiple items to be open at a time, so none of the registration seems to matter).

Reproduction Sample

https://codepen.io/paulcp/pen/MWBYdew?editors=1011

Reproduction Steps

1. Open the console
2. Notice the error

Reproduction Version

All

Relevant Info

All

Regression?

No response

Impact

Minor annoyance, I am happy to open a pr if the team has insufficient bandwidth and agrees with the direction outlined above.

Esri team

ArcGIS Online

[benelan]

Yeah I agree using closestElementCrossShadowBoundary is what we want. We would definitely appreciate a PR if you have the time, thanks!

[driskull]

Updating to use closestElementCrossShadowBoundary will work to get the correct itemPosition. However, if an accordion item is moved position after being loaded the first time then that index won't be updated.

Ideally, we want to refactor this to just use a mutation observer on the accordion and get item positions at that point. Issue: #6038

Probably a major refactor so I guess its fine to fix this for now. cc @jcfranco