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

🐞 documentation does not include focus in default events. But, should focus be included in default events? #395

Open
1 task done
eliprand opened this issue Aug 8, 2024 · 0 comments
Assignees
Labels
bug A verified and reproducible bug. triage Has not been reviewed yet and should not be worked on.

Comments

@eliprand
Copy link

eliprand commented Aug 8, 2024

What happened?

We recently upgraded an app that was 3+ years old and upgraded to the latest version of this package. We first started using the default events as the list in the documentation matched what we would expect.
However, the actual behavior was different from previous, older versions. Our timeout was not triggering unless we used another app/window. Digging quickly into the code, we quickly found out that focus had been added to the list of default event.
At the very least, we should update the docs to match the code (happy to open a PR for that 😉 ).
But our bigger question is: should focus really be in the default events?
I looked through the issues and I did not find an issue asking for it (very possible I missed it).

The fix for us was pretty easy, we just used DEFAULT_EVENTS (thank you for exporting it...) and simply removed focus from the list that we passed to the useIdleTimeout().
But if the user is on the tab but not doing anything, are they really active?

Reproduction Steps

  1. Visit the Props documentation
  2. Confirm the list of events
  3. Check the code
  4. confirm that the 2 lists are different

Relevant log output

No response

Screenshots or Additional Context

No response

Module Version

5.7.2

What browsers are you seeing the problem on? Select all that apply.

Firefox, Chrome, Safari, Microsoft Edge, Other

What devices are you seeing the problem on?

Desktop, Mobile, Tablet, Wearable, Other

Verification

  • I have checked for existing closed issues and discussions.
@eliprand eliprand added bug A verified and reproducible bug. triage Has not been reviewed yet and should not be worked on. labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A verified and reproducible bug. triage Has not been reviewed yet and should not be worked on.
Projects
None yet
Development

No branches or pull requests

2 participants