Skip to content

Menu button updates for table#986

Merged
mollykreis merged 19 commits into
mainfrom
menu-button-updates-for-table
Jan 26, 2023
Merged

Menu button updates for table#986
mollykreis merged 19 commits into
mainfrom
menu-button-updates-for-table

Conversation

@mollykreis

Copy link
Copy Markdown
Contributor

Pull Request

🤨 Rationale

Some changes need to be made to the menu button before it can be used within the table for the action menu. Specifically, those changes are:

  • adding a beforetoggle event
  • having the menu button find the slotted menu when it is nested within multiple slots

See #956 for more details.

👩‍💻 Implementation

  • Add beforetoggle event to the menu button that is fired before the menu opens or closes
  • Add Angular & Blazor support for the beforetoggle event
  • Change the logic for finding the slotted menu. We can no longer use a filter in the template to find slotted elements with a role of menu because this won't match slot elements. Instead, the template assigns all slotted elements to the slottedMenus array, and the getter for menu finds the menu, which may be nested within slot elements.

🧪 Testing

  • Manually tested that the beforetoggle event handler works correctly in Blazor & Angular
  • Added auto tests for the beforetoggle event works correctly for the web components
  • Basic manual testing in Storybook that the component still behaves as expected
  • Added tests that the focus & menu opening behavior works correctly when the menu is nested in an additional slot

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

Comment thread packages/nimble-components/src/menu-button/specs/README.md Outdated
Comment thread packages/nimble-components/src/menu-button/index.ts Outdated
Comment thread packages/nimble-components/src/menu-button/index.ts Outdated
@mollykreis mollykreis requested a review from rajsite January 24, 2023 21:59
Comment thread packages/nimble-components/src/menu-button/index.ts Outdated
@mollykreis mollykreis merged commit c39e8c8 into main Jan 26, 2023
@mollykreis mollykreis deleted the menu-button-updates-for-table branch January 26, 2023 18:38
@rajsite rajsite mentioned this pull request Jan 26, 2023
1 task
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.

5 participants