Option to opt-out from Focus Trap

[zeppelin]

Some addons like Ember Power Select render outside the component they're invoked in, making it impossible to use with EPM. (Although EPS can be forced to render in place, but could be difficult to deal with for various reasons). I saw no option in Focus Trap to exclude certain selectors.

I know it would reduce accesibility, but spending time with code that handle modals if we don't have EPM also takes away precious time we could otherwise spend with making our app more accessible.

It doesn't even have to be public API, just an option that's one would only use if really needed it.

Should I open a PR?

[kevinansfield]

Added a comment to this old issue that is along the same lines #295 (comment)

It is possible to prevent clicks on ember-basic-dropdown components closing the modals by overriding the modals service in your app, eg:

import EPMModalsService from 'ember-promise-modals/services/modals';

export default class ModalsService extends EPMModalsService {
    clickOutsideDeactivates(event) {
        let shouldClose = true;

        for (const elem of event.path) {
            if (elem.classList?.contains('ember-basic-dropdown-content')) {
                shouldClose = false;
                break;
            }
        }

        return shouldClose;
    }
}

However, because there's no equivalent option passthrough for allowOutsideClick the clicks get prevented from doing anything at all.

[kevinansfield]

As a proof-of-concept I modified the modal.js file in ember-promise-modals locally so that an allowOutsideClick option is passed through:

didInsertElement() {
    this._super(...arguments);

    let { allowOutsideClick, clickOutsideDeactivates } = this.modals;
    let element = document.getElementById(this.modalElementId);
    let options = {
      allowOutsideClick,
      clickOutsideDeactivates,
      ...

Then in my app's modals service, added the option as a function:

allowOutsideClick(event) {
    let shouldAllow = false;

    for (const elem of event.path) {
        if (elem.classList?.contains('ember-basic-dropdown-content')) {
            shouldAllow = true;
            break;
        }
    }

    return shouldAllow;
}

With that everything works well with dropdowns inside of a modal. I guess the question is whether allowOutsideClick is a valid option to add to the addon service or if there should be some other route to configuring or disabling focus-trap. The previous argument against adding the option was that the addon becomes tied to focus-trap's API but I'd argue the addon is already tied heavily to focus-trap's behaviour so it's not exactly an implementation detail right now.

[pichfl]

@zeppelin @kevinansfield I wouldn't mind having that option at all, a PR for this (including some form of documentation in the README) would be much appreciated.

[zeppelin]

@pichfl @kevinansfield We currently patch EPM in our project:

diff --git a/node_modules/ember-promise-modals/addon/components/modal.js b/node_modules/ember-promise-modals/addon/components/modal.js
index f145b4f..331ed18 100644
--- a/node_modules/ember-promise-modals/addon/components/modal.js
+++ b/node_modules/ember-promise-modals/addon/components/modal.js
@@ -30,7 +30,7 @@ export default Component.extend({
   didInsertElement() {
     this._super(...arguments);
 
-    let { clickOutsideDeactivates } = this.modals;
+    let { clickOutsideDeactivates, _disableFocusTrap } = this.modals;
     let element = document.getElementById(this.modalElementId);
     let options = {
       clickOutsideDeactivates,
@@ -44,8 +44,10 @@ export default Component.extend({
       },
     };
 
-    this.focusTrap = createFocusTrap(element, options);
-    this.focusTrap.activate();
+    if (!_disableFocusTrap) {
+      this.focusTrap = createFocusTrap(element, options);
+      this.focusTrap.activate();
+    }

So right now this pretty much looks like a private/undocumented option. Are you cool with it, or just make it public (and optionally reverse the wording to enableFocusTrap with default true)?

I can update the README for any of the solutions above.

[pichfl]

I would prefer it being public and documented API, I feel like more people might run into this issue. I would also keep disableFocusTrap as the name 👍