Fix ActionMenu item activation

[camertron]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

In working on the experimental SelectPanel in dotcom, we have noticed a number of issues using the keyboard to activate items in multiple- and single-select modes. Since SelectPanel is based on ActionMenu, I decided to take another look at ActionMenu to inform my work on SelectPanel. In so doing, I discovered a number of deficiencies in ActionMenu's custom element code that I believe are causing the issues we're seeing in SelectPanel.

Integration

No changes necessary in production.

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

One of the big issues my work uncovered is that browsers appear to fire both a PointerEvent and a KeyboardEvent when you press 'Enter' on an ActionMenu item. This is causing the item to be activated twice, and is why we're seeing the item fail to toggle when we press 'Enter' on it (i.e. the first event checks and the second one unchecks so quickly it appears nothing happens). I don't remember two events firing before, so it's possible some browser-level thing changed recently 🤷

It turns out we can use PointerEvents almost exclusively, as they fire when clicking and pressing 'Enter.' I say "almost" because activating menu items is also possible by pressing 'Space' on anchor tag items (eg. items that use the <a> tag), which does not cause a PointerEvent but instead a KeyboardEvent (confused yet?) By listening for keydown specifically on anchor items and click for everything else, all interactions via the mouse and keyboard work as expected.

I also simplified how activating items works by removing the #activateItem method. Previously, #handleEvent would call #activateItem and then #handleItemActivated. The #activateItem method contained some logic to handle issues to override the default button click behavior, which included calling click() on the button to simulate a mouse-driven click (as opposed to a keyboard-driven one). This was causing another form of the double activation issue I mentioned earlier, as handleEvent would call #activateItem which would call click() on the item, which would trigger handleEvent again. This would eventually result in two calls to #handleItemActivated, which would check and then uncheck the item in rapid succession, appearing to have no effect.

Accessibility

• No new axe scan violation - This change does not introduce any new axe scan violations.

Merge checklist

• Added/updated tests
  - [ ] Added/updated documentation
  - [ ] Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: 72805fe

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR