Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix focusin-based tests #2861

Merged
merged 1 commit into from
Sep 1, 2024
Merged

Conversation

Telroshan
Copy link
Collaborator

Description

Even though we call .focus on elements in these 2 tests, the focusin event might not be fired.

How to reproduce it?

Disclaimer: the CI suddenly constantly fails on this, and I have no idea why, specifically why it worked until now and suddenly doesn't anymore.
To reproduce that issue, run the tests locally through npx serve (thus browser context) with a 20x CPU slowdown throttling. Refresh the tests page, and click outside it before these tests run.
Adding logs reveal that, even though the document.activeElement gets properly updated to the focused element, the focusin event won't fire, until the window gets focused back (clicking inside it).
I guess that's an expected browser-UX feature, where the browser waits for the user to actually focus the element (thus focus the browser window first) before notifying it.
The issue here is that we don't really know how things work when running headless through mocha-chrome, but I'm thinking this could explain why the pipeline fails.

Suggested solution

The suggested solution here simply doubles the focusin event by manually triggering it with htmx.trigger.
This doesn't change the test as, what we're testing is not the focusin event itself being fired (that's the browser's job), just ensuring that htmx behaves correctly once that event has been fired.

Checklist

  • I have read the contribution guidelines
  • I have targeted this PR against the correct branch (master for website changes, dev for
    source changes)
  • This is either a bugfix, a documentation update, or a new feature that has been explicitly
    approved via an issue
  • I ran the test suite locally (npm run test) and verified that it succeeded

@Telroshan Telroshan added bug Something isn't working devops Changes to the repository structure or project management ready for review Issues that are ready to be considered for merging labels Sep 1, 2024
Copy link
Collaborator

@alexpetros alexpetros left a comment

Choose a reason for hiding this comment

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

Yup, totally reasonable

@alexpetros alexpetros merged commit 326ff3b into bigskysoftware:dev Sep 1, 2024
1 check passed
@Telroshan Telroshan deleted the fix-focusin-tests branch September 1, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working devops Changes to the repository structure or project management ready for review Issues that are ready to be considered for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants