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

Expose focus-trap configuration #295

Closed
wants to merge 1 commit into from

Conversation

pichfl
Copy link
Contributor

@pichfl pichfl commented Jan 12, 2021

This is an extension to #178 released with 0.2.1 and enables more fine grained control over the focus-trap, which might be necessary to work around issues with clickOutsideDeactivates vs allowOutsideClick described in the projects README.

@pichfl pichfl added breaking enhancement New feature or request labels Jan 12, 2021
This is an extension to #178 released with 0.2.1 and enables more fine grained control over the focus-trap, which might be necessary to work around issues with clickOutsideDeactivates vs allowOutsideClick described in [the projects README](https://github.com/focus-trap/focus-trap#createfocustrapelement-createoptions)

It also documents that configuration option in the projects README.
@pichfl pichfl force-pushed the pichfl/focus-trap-configuration branch from 5d4b127 to a1bdc87 Compare January 14, 2021 12:54
@Turbo87
Copy link
Collaborator

Turbo87 commented Feb 9, 2021

the problem with this is that we are adding the focus-trap API to our own public API and any breaking change to the focus-trap API means that we also have to make a breaking API change.

I would prefer to solve the use cases where focus-trap option changes are necessary by abstracting these option in our own API in some way instead of directly exposing the library. The fact that we're using focus-trap IMHO should be seen as an implementation detail.

@pichfl
Copy link
Contributor Author

pichfl commented Feb 9, 2021

Works for me, I'll make a new PR exposing some of the options and documents these.

@pichfl pichfl closed this Feb 9, 2021
@kevinansfield
Copy link

Did this end up going anywhere? I'm in the process of migrating our app's modals implementation over to ember-promise-modals but we need more control over when modals closed, namely:

  • per-modal config for whether clicks outside will close
  • allow clicks on wormholed elements inside of a modal (dropdowns, etc)

kevinansfield added a commit to TryGhost/ember-promise-modals that referenced this pull request Sep 13, 2021
refs mainmatter#435
refs mainmatter#295

https://github.com/focus-trap/focus-trap is used internally for keeping focus inside a modal and for clicks outside to close the modal. It's `clickOutsideDecactivates` config is exposed allowing for  clicks on wormholed elements such as dropdowns inside a modal to not close the modal unexpectedly, however without the companion `allowOutsideClick` config those clicks are prevented from performing any interaction.

- added `allowOutsideClick` to the options passed through from the `modals` service to the `focus-trap` initialisation
kevinansfield added a commit to TryGhost/ember-promise-modals that referenced this pull request Sep 13, 2021
refs mainmatter#435
refs mainmatter#295

https://github.com/focus-trap/focus-trap is used internally for keeping focus inside a modal and for clicks outside to close the modal. It's `clickOutsideDecactivates` config is exposed allowing for  clicks on wormholed elements such as dropdowns inside a modal to not close the modal unexpectedly, however without the companion `allowOutsideClick` config those clicks are prevented from performing any interaction.

- added `allowOutsideClick` to the options passed through from the `modals` service to the `focus-trap` initialisation
kevinansfield added a commit to TryGhost/ember-promise-modals that referenced this pull request Sep 16, 2021
refs mainmatter#435
refs mainmatter#295

https://github.com/focus-trap/focus-trap is used internally for keeping focus inside a modal and for handling modal closing on certain events. If the consuming app wants to manage deactivation/close-inducing events itself then it needs some way to pass `focus-trap` config through to turn off that functionality.

- added `escapeDeactivates` to the options passed through from the `modals` service to the `focus-trap` initialisation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants