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

[feature request] preventScroll Option on Focus Override #162

Closed
danlevy1 opened this issue Jun 2, 2021 · 7 comments · Fixed by #186
Closed

[feature request] preventScroll Option on Focus Override #162

danlevy1 opened this issue Jun 2, 2021 · 7 comments · Fixed by #186
Assignees

Comments

@danlevy1
Copy link

danlevy1 commented Jun 2, 2021

Description of Feature

Currently, react-focus-lock allows you to pass in focus options (including preventScroll: true) to the returnFocus prop (see #84 and theKashey/focus-lock#23). I am looking for a prop to add focus options to the logic that brings focus back to the focus lock when a focus event occurs outside of the focus lock.

Use-Case

I am creating a Popover component that uses react-focus-lock that opens when an anchor/reference button is clicked. When the Popover is open, the user is able to scroll around the page. If the user scrolls to the point where the Popover is out of view and then clicks on a button outside of the Popover, some browsers (including Chrome) scroll back to the open Popover. The reason is that some browsers focus a button on mousedown, which I'm assuming causes the focus lock inside of the Popover to grab focus again. I would like to disable scrolling when this Popover focus event occurs using preventScroll: true.

Example

Here is a small example of the scroll behavior: https://codesandbox.io/s/react-focus-lock-prevent-scroll-1hjn3?file=/src/App.js

To reproduce:

  1. Make sure the focus lock is enabled (see button at the top)
  2. Scroll down to the bottom of the page
  3. Click and hold on the button called "Button outside of focus lock" to trigger the mousedown event.
  4. Chrome will scroll back to the top of the page where the focus lock is. I want to be able to prevent this part of the reproduction.
@theKashey
Copy link
Owner

A very sound addition very simple to implement. Thank you for providing a good use case, it's really not the best user experience so easy to improve.

@abhimanyu-singh-uber
Copy link
Contributor

Hi @theKashey, It would be really helpful if we can fix this.
I took a jab at it and have raised a PR in focus-lock repo where focus is being done. Please have a look at it if you get a chance.

Thank you.

PR link- theKashey/focus-lock#29

@theKashey
Copy link
Owner

@abhimanyu-singh-uber - thank you for the implementation. I'll handle the rest

@abhimanyu-singh-uber
Copy link
Contributor

Thank you @theKashey

@walliby
Copy link

walliby commented Dec 7, 2021

Wow I just ran into the exact same problem with the exact same use case. Thanks so much for addressing this!

theKashey added a commit that referenced this issue Dec 12, 2021
theKashey added a commit that referenced this issue Dec 12, 2021
theKashey added a commit that referenced this issue Dec 12, 2021
feat: support focusOptions, fixes #162
@theKashey
Copy link
Owner

2.7.0 has been release. A new prop - focusOptions has been added.

@abhimanyu-singh-uber
Copy link
Contributor

@theKashey We would need to merge this PR & release react-focus-lock again for the changes to get in.

The react-focus-lock is taking in the previous version of focus-lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants