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

feat: expose whitelist prop #43

Closed narehart closed 3 years ago

narehart commented 4 years ago

Exposes whiteList prop from react-focus-on for cases where ref shards cannot be used, e.g., another library that renders a dropdown into the body.

theKashey commented 4 years ago

I should be wired to ScrollLock as well, or it will suppress events from "outer" nodes. And right now it does not support such prop.

Would it be required to solve your case?

narehart commented 4 years ago

Not required in my case because there is no scrolling on the dropdowns being rendered. But I could see it being a problem.

theKashey commented 4 years ago

Ok. Let's solve one problem a time.

I am also going to rename this prop as long as it's actually about ignore, however it will also require to "flip" the meaning of result

Like shouldIgnore?: (activeElement: HTMLElement) => boolean; -> true is "ignore", "false" is handle.

- <FocusOn whiteList={node => document.getElementById('root').contains(node)}>
+ <FocusOn shouldIgnore={node => !document.getElementById('root').contains(node)}>

Nowadays usually there are 2 props (in different tools, like Jest for example):

Which way sounds better for you?

narehart commented 4 years ago

I agree shouldIgnore makes more sense

jacobweber commented 2 years ago

BTW, this alternate name isn't documented -- I was trying to use whiteList. And does it actually "flip" the value? Looks like it just passes it on, from what I can tell.