Remove aria-hidden=true with noIsolation behavior

theKashey / react-focus-on

🎯 Solution for WAI ARIA compatible modal dialogs or full-screen tasks, you were looking for

MIT License

333 stars 14 forks source link

Remove aria-hidden=true with noIsolation behavior #57

Closed omerdemirkan closed 2 years ago

omerdemirkan commented 2 years ago

Current behavior adds aria-hidden attribute throughout the DOM with noIsolation === true by calling hideOthers(refs, document.body, undefined).

This makes introducing the library into a codebase with existing UI testing extremely challenging, as many testing frameworks ignore subtrees with aria-hidden="true", meaning as soon as react-focus-on is used, many UI tests may go red. I made this in hopes of removing that barrier to entry.

theKashey commented 2 years ago

From one point of view hiding everything except currently "usable" area should fix those tests, so this moment is not really makes sense. But there are two other points:

I know how hard it to be "enforced" to follow some best practices as the old code might be just not "ready"
This code was supposed to be like this. I should double check (git) history to understand why it ended this way, but noIsolation means NO ISOLATION. No other reading is possible.

theKashey commented 2 years ago

the intention of noIsolation ? undefined : focusHiddenMarker is to set focusHiddenMarker for "other nodes"
focusHiddenMarker controls pointer-events: none
😅 obviously removing mouse interaction should be syncronized with removing aria control
in other words - proposed change in the only valid way to do that

omerdemirkan commented 2 years ago

Thanks for the context!

Would love to get my hands on an updated version; is there a particular schedule the package follows for releases?

theKashey commented 2 years ago

Trying to handle 4 more problems (starting from #58)