theKashey / vue-focus-lock

It is a trap! A lock for a Focus. A11y util for scoping a focus.
MIT License
137 stars 14 forks source link

enhancement : create a class name "data-no-focus-lock" as an alternative way to remove focus management #20

Open tbl0605 opened 4 years ago

tbl0605 commented 4 years ago

Hi, first of all thank you for this great library, it works very well :) My bug report is more about an "enhancement" request, it would be great to also provide a "data-no-focus-lock" CSS class to disable focus management.

In many Vue libraries, you are not allowed to set custom component attributes (they will not be "inherited" by the root element of the component as a normal HTML attribute), but in most cases you can set and use custom CSS classes.

I'll give you a very concrete example: I recently had the problem with some Modal components from bootstrap-vue: https://bootstrap-vue.org/docs/components/modal#modal-message-boxes

bootstrap-vue provides you with a very convenient this.$bvModal.msgBoxConfirm(message, options) function that generates a Confirm style modal message, from anywhere in a Vue app without having to explicitly place a component:

this.$bvModal.msgBoxConfirm('Are you agree?', {
  centered: true,
  noFade: true,
  noCloseOnBackdrop: true,
  noCloseOnEsc: true,
  autoFocusButton: 'cancel'
});

But you have no possibility to directly set special attributes like "data-no-focus-lock" on those auto-generated components, so I had to create following hack to remove focus-lock management once the modal was displayed (you cannot do it "earlier", sadly the other event named "bv::modal::show" can be triggered before the Modal is added to the DOM):

  mounted() {
    this.$root.$on('bv::modal::shown', bvEvent => {
      bvEvent.vueTarget.$refs.dialog.setAttribute('data-no-focus-lock', true);
    });
  }

It's hacky and probably could be racy the first time the Confirm Modal is used (since the "cancel" button could be auto-focused before vue-focus-lock detected the newly-added "data-no-focus-lock" attribute), but surprisingly it works.

It would be so much better if vue-focus-lock could handle this:

this.$bvModal.msgBoxConfirm('Are you agree?', {
  centered: true,
  noFade: true,
  noCloseOnBackdrop: true,
  noCloseOnEsc: true,
  autoFocusButton: 'cancel',
  dialogClass: 'data-no-focus-lock'
});

I know I could ask the bootstrap-vue team to add custom attributes support, but I feel it's the wrong solution (other libraries will probably have same limitations) and the wrong place to enhance things, I think sometimes it's much easier (and portable) to use specific CSS class names than special data attributes :)

theKashey commented 4 years ago

sounds like one line here - https://github.com/theKashey/focus-lock/blob/38901516f49c9d7eca5e155e48bc074a7073fbc6/src/focusIsHidden.js#L5

- document && toArray(document.querySelectorAll(`[${FOCUS_ALLOW}]`)).some(node => node.contains(document.activeElement))
+ document && toArray(document.querySelectorAll(`[${FOCUS_ALLOW}], .${FOCUS_ALLOW}`)).some(node => node.contains(document.activeElement))
tbl0605 commented 1 year ago

Hi @theKashey, would it be still possible to implement the "specific CSS class name" feature in the future? I'm still using same workaround described above, but it would be nice to get ride of it ;)

Thank you!

theKashey commented 1 year ago

if the mountain doesn't come to Muhammad... then it's easier to make adjustments in the focus-lock than in vue-boostrap.

However, there is another question to ask before doing the change - why it is required? Problem can be mitigated by:

tbl0605 commented 1 year ago

Hi @theKashey, I'll have a look in my code again, probably I don't need my hack (anymore), I don't remember what was the initial problem it was supposed to solve (other than the fact you cannot disable focus-lock using css classes).

Thank you for your suggestions.

theKashey commented 1 year ago

That still might be a good addition