Skip to content

Custom tooltip assertion on disabled elements && GREATLY improve assertion times#643

Open
Miexil wants to merge 2 commits intomasterfrom
mb/tooltip-assert-disabled
Open

Custom tooltip assertion on disabled elements && GREATLY improve assertion times#643
Miexil wants to merge 2 commits intomasterfrom
mb/tooltip-assert-disabled

Conversation

@Miexil
Copy link
Member

@Miexil Miexil commented Feb 16, 2026

What does this PR do?

Introduces a couple of updates to the custom tooltip assertions:

  1. It is now possible to use the custom tooltip assertions (await asser.tooltip('selector').exists()) on disabled elements
    The previous implementation was raising errors when we tried to do so, telling us that triggerEvent from ember TestHelpers cannot be triggered on a disabled element. To bypass this, in the logic, we now check if the target element has a disabled attribute. If it's the case, we now use a regular, JS based, element.dispatchEvent to trigger the mouseover event. If not disabled, we keep using triggerEvent from the test-helpers (mainly for safety, prevent breaking changes).

  2. ASSERTION TIMES HAVE !!!! GREATLY !!!! been improved :
    Thanks to a suggestion from @aprentout to use Sinon's fake timers to bypass the animation & rendering delays.
    Running a tooltip assertion is now divided by 7! From ~350ms to ~50ms per test.

Old test times:
Screenshot 2026-02-16 at 11 51 20

Newt test times:
Screenshot 2026-02-16 at 15 14 10

ℹ️
Tests have been added in the enable-tooltip modifier to check the "disabled" state for lack of a better place 😇

Good PR checklist

  • Title makes sense
  • Is against the correct branch
  • Only addresses one issue
  • Properly assigned
  • Added/updated tests
  • Added/updated documentation
  • Migrated touched components to Glimmer Components
  • Properly labeled

Copy link
Contributor

@Elodie-DeMatteis-Upf Elodie-DeMatteis-Upf left a comment

Choose a reason for hiding this comment

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

LGTM: so nice!

Copy link
Contributor

@JulienVannier66 JulienVannier66 left a comment

Choose a reason for hiding this comment

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

👏 🎉

Copy link

@edouardmisset edouardmisset left a comment

Choose a reason for hiding this comment

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

Great work :) ⌛️ :)

Comment on lines 10 to 18
const element = document.querySelector(selector) as HTMLElement;
const clock = sinon.useFakeTimers();

if (element?.hasAttribute('disabled')) {
element.dispatchEvent(new Event(trigger || 'mouseover', { bubbles: true }));
} else {
await triggerEvent(selector, trigger || 'mouseover');
}

Choose a reason for hiding this comment

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

suggestion:

  1. You might want to wrap this block in a try / catch because if dispatchEvent or triggerEvent fails, you might get a memory leak from sinon's fake timer (same with L. 57).
  2. Typecasting might be "dangerous". If the element is null, what should we do? Try to trigger an event (triggerEvent) with a selector we know is not found?

Copy link
Member Author

Choose a reason for hiding this comment

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

1- wrapped in try/finally
2- IMO we don't need anything specific here ; these are to be used within tests - if something breaks during tests, then the developer needs to adjust their code to pass :)

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants