Skip to content

Conversation

@LoopyDev
Copy link

UX improvement here! 😅

This lets parent tune buttons retain their hover/tint while a child menu is open and clears it when the submenu closes or another submenu opens.

popover-active-ezgif com-crop

  • Add helper to set/clear data-item-opened/aria-expanded when nested popovers open/close
  • Ensure switching to another submenu tears down the previous one so only one parent stays marked
  • Clear open-state markers on popover destroy to reset tint/hover state

protected override showNestedItems(item: PopoverItem): void {
if (this.nestedPopover !== null && this.nestedPopover !== undefined) {
return;
if (this.nestedPopoverTriggerItem === item) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, add a jsdoc describing this statement

this.nestedPopover.show();
this.flipper?.deactivate();

this.clearOpenedState();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is called twice in this context?

Comment on lines +293 to +296
if (triggerEl) {
triggerEl.dataset.itemOpened = 'true';
triggerEl.setAttribute('aria-expanded', 'true');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsdoc is required here

}

/**
* Clears open-state markers from all popover items
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSDoc repeats function name. Please, describe why we need this method instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants