Accordion-item focus behavior inside Accordion

[YonatanKra]

In Accordion, focusing on the child sets the private property activeItemIndex to the focused item's index.
I've opened a PR to simplify the code (#6616 ), but then noticed some edge cases that could be troubling.
Because it sets the current active index, if I'm in single mode and change the items in the accordion it will open the focused item regardless of the state before the addition.

Here's a unit test that shows the issue:

it('should set focused element as the active item', async function () {
			function triggerAccordionUpdate() {
				const newItem = document.createElement('vwc-accordion-item') as AccordionItem;
				element.appendChild(newItem);
			}

			accordion.expandmode = 'single';
			toggleAccordionItem(accordionItem2);
			const item2ExpandedBeforeFocus = accordionItem2.expanded;
			const item1ExpandedBeforeFocus = accordionItem1.expanded;

			accordionItem1.dispatchEvent(new FocusEvent('focus'));
			triggerAccordionUpdate();

			expect(item1ExpandedBeforeFocus).toBeFalsy();
			expect(item2ExpandedBeforeFocus).toBeTruthy();

			expect(accordionItem1.expanded).toBeTruthy();
			expect(accordionItem2.expanded).toBeFalsy();
		});

The activeItemIndex is used in the focusItem method that's supposed to set the focus to the current active item but it is also used to determine the current expanded item in setItems:

this.activeItemIndex !== index
                            ? (item.expanded = false)
                            : (item.expanded = true);

Is this the wanted behavior?

[chrisdholt]

thanks for this @YonatanKra, I'd like to test this against the changes going into the main branch for accordion to see if the issue persists after the slight refactor. As a note for you and @yinonov our main branch is tracking our current alpha versions. The archives/fast-element-1 branch is tracking the stable versions. I plan on trying to move over as many changes as we can to stable which have gone in, though some have necessitated breaking changes with the accordion-item. Currently we're dealing with an issue that's blocking that pipeline so we're trying to pri investigating what's going on there...

[YonatanKra]

Thanks!
We are indeed using the stable version, so any fix there will propagate to our design system.
I saw the changes made to accordion and accordion-item. I believe the inversion of control from item to accordion that is introduced is the better solution. On the other hand, they are, as you say, breaking the current stable API.

Regarding the current issue with activeItemIndex, I solved it locally.
Would you consider a PR directly to archives/fast-element-1 regarding this issue?

[chrisdholt]

Thanks! We are indeed using the stable version, so any fix there will propagate to our design system. I saw the changes made to accordion and accordion-item. I believe the inversion of control from item to accordion that is introduced is the better solution. On the other hand, they are, as you say, breaking the current stable API.

Regarding the current issue with activeItemIndex, I solved it locally. Would you consider a PR directly to archives/fast-element-1 regarding this issue?

Totally open - I do want to figure out if it repro's in the latest implementations given we may wholesale take some of those fixes. Additionally we need to figure out the error in that branch preventing publishing. Feel free to throw a PR, but it may be held simply given the need to figure out the build error.

[YonatanKra]

@chrisdholt
I've added this #6620, which fixes this problem (and another one I stumbled upon).
I could also go on to fix #6585 .
I see there's an open PR for #6585 - didn't dive deep into it, but it doesn't seem to break the API so maybe could be applied to stable too?

[janechu]

Unfortunately @microsoft/fast-foundation is being deprecated, refer to #6955. I see this was mentioned in a PR, we will be addressing open PRs and merging what we can before we snap an archive branch to preserve the latest state of Foundation, however to bring us up to date I am closing out issues.