Add Menu target `click()` support #986

clintcs · 2025-07-22T16:52:25Z

🚀 Description

Minor

Menu can no longer be opened by setting its open attribute via the "click" handler of an element outside Menu.
If you need to open this way, you can call click() on Menu's target in your handler. This change was made to support the ability to open sub-Menu targets programmatically by calling click().

Patch

Sub-Menus of Menu can now be opened by programmatically calling click() on sub-Menu targets.

📋 Checklist

I have followed the Contributing Guidelines.
I have added tests to cover new or updated functionality.
I have added or updated Storybook stories.
I have localized new strings.
I have followed the ARIA Authoring Practices Guide or met with the Accessibility Team.
I have included a changeset.
I have scheduled a design review.
I have reviewed the Storybook and Visual Test Report links below.

🔬 Testing

Sub-Menus of Menu can now be opened by programmatically calling `click()` on sub-Menu targets

Navigate to Menu in Storybook.
Use DevTools to call click() on the top-level Menu's target.
Use DevTools to call click() on the sub-Menu's target.
Verify the sub-Menu is open.
Use DevTools to call click() on the sub-Menu's target.
Verify the sub-Menu is closed.

changeset-bot · 2025-07-22T16:52:28Z

🦋 Changeset detected

Latest commit: 0f716fd

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

This PR includes changesets to release 1 package

Name	Type
@crowdstrike/glide-core	Minor

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

clintcs · 2025-07-22T17:01:05Z

src/menu.ts

-      // `true` in the consumer's handler isn't overwritten by `#onDocumentClick()`
-      // handler setting it to `false`.
-      document.addEventListener('click', this.#onDocumentClick, {
-        capture: true,


The reason calling click() on sub-Menu targets wasn't supported was because this listener listened for "click" in its capture phase, and this.#onDocumentClick() closes Menu when a click doesn't come from Menu.

Because it listened in the capture phase, we had to rely on #onTargetSlotMouseUp() (which is called before "click" is dispatched) to determine in this.#onDocumentClick() if the click came from a Menu target. And "mouseup" events, of course, aren't dispatched for programmatic clicks. So we had no way to set this.#isTargetSlotMouseUp when a target was clicked via click().

Now that this.#onDocumentClick() is called in the bubble phase, we can ditch this.#isTargetSlotMouseUp and #onTargetSlotMouseUp(), and simply set this.#isTargetSlotClick in #onTargetSlotClick(). After #onTargetSlotClick() is called, the event bubbles up to this.#onDocumentClick().

clintcs · 2025-07-22T17:06:19Z

src/menu.test.interactions.ts

 });

-// See the comment in `connectedCallback()` for an explanation.
-it('opens when opened programmatically via the click handler of another element', async () => {


Menu can no longer be opened by setting its open attribute via the "click" handler of an element outside Menu. If you need to open Menu via the "click" handler of an element outside Menu, you can call click() on Menu's target. This change was made to support opening sub-Menu targets programmatically via click().

clintcs · 2025-07-22T17:11:25Z

src/menu.test.interactions.ts

+  // `sendMouse()` is just a thin abstraction over Playwright's equivalent API. So
+  // the ultimate cause is likely Playwright or Chromium's DevTools Protocol, which
+  // Playwright relies on. Either that or I'm missing something obvious.
+  targets[1]?.click();


As a bonus, we can now call click() in tests—which I believe will resolve most if not all of the flakiness we've seen.

That said, there still appears to be systemic issues causing sporadic flakiness of various tests, and we've been seeing for some time. The cause isn't clear to me.

I had hoped that Playwright's support for component testing would be stable by now. When it finally is (and when they support web components with it), we'll move to it and hopefully things will stabilize. Until then, there are a couple things we can try:

Chromium's new headless mode.

Break our tests out into smaller suites.

Chromium's new headless mode isn't quite there yet in terms of quality. Either that or it doesn't work well with Web Test Runner. But we can break out the tests out now—which probably isn't a bad idea anyway because doing so could cut our build times in half. So I'll do that soon.

clintcs · 2025-07-22T17:16:16Z

.changeset/good-pianos-heal.md

+
+Menu can no longer be opened by setting its `open` attribute via the "click" handler of an element outside Menu.
+If you need to open this way, you can call `click()` on Menu's target in your handler.
+This change was made to support the ability to open sub-Menu targets programmatically by calling `click()`.


It was nice to be able to support setting open this way. But supporting it precluded consumers being able to call click() on sub-Menu targets.

And there's an alternative. But there's no alternative to not being able to call click() on a sub-Menu target. So, overall, a net improvement for consumers.

clintcs · 2025-07-22T17:38:35Z

src/menu.test.interactions.ts

-  expect(tooltips[2]?.open).to.be.true;
-  expect(tooltips[3]?.open).to.be.true;
-  expect(tooltips[4]?.open).to.be.false;
-  expect(tooltips[5]?.open).to.be.false;


Flaky and nice but not necessary to assert in this test.

src/menu.test.interactions.ts

ynotdraw · 2025-07-22T17:58:56Z

src/menu.test.interactions.ts

+  // `sendMouse()` is just a thin abstraction over Playwright's equivalent API. So
+  // the ultimate cause is likely Playwright or Chromium's DevTools Protocol, which
+  // Playwright relies on. Either that or I'm missing something obvious.
+  targets[1]?.click();


src/menu.test.interactions.ts

Co-authored-by: Tony Ward <8069555+ynotdraw@users.noreply.github.com>

clintcs force-pushed the menu-target-click-support branch from 95e0a8f to b6c90cc Compare July 22, 2025 16:54