theKashey / react-focus-lock

It is a trap! A lock for a Focus. 🔓
MIT License
1.27k stars 67 forks source link

Can we have a dialog example in the docs? #195

Closed christopher-francisco closed 1 year ago

christopher-francisco commented 2 years ago

I'm trying to sort out whether the author intetion is for the component to be used like this:

<FocusLock>
  <ModalRoot  // styled-component
    aria-labelledby={ariaLabelledBy}
    aria-modal="true"
    aria-expanded="true"
    role="dialog"
  >
    {children}
  </ModalRoot>
</FocusLock>

or the other way around

<ModalRoot  // styled-component
  aria-labelledby={ariaLabelledBy}
  aria-modal="true"
  aria-expanded="true"
  role="dialog"
>
  <FocusLock>
    {children}
  </FocusLock>
</ModalRoot>

Or maybe in a completely different way:

const ModalRoot = styled(FocusLock)`
  // css 
`
<ModalRoot  // styled-component
  aria-labelledby={ariaLabelledBy}
  aria-modal="true"
  aria-expanded="true"
  role="dialog"
>
  {children}
</ModalRoot>

The reason I'm bringing this up is because of the extra HTML elements that get added to the DOM around the main one. Not sure if those should be wrabbed by the [role=dialog] element or whether it even matters.

I'm more than happy to submit a PR to add the intended usage example to the docs. Just need come clarification. Thanks in advance!

theKashey commented 2 years ago

Long story short - I would recommend the first variant. Here is an example passing all requirements for "good modal dialog" - https://codesandbox.io/s/good-react-example-modal-dialog-forked-k4imc?file=/src/App.js

christopher-francisco commented 2 years ago

Thank you so much @theKashey. Would you recommend going for focus-on library instead for a Modal component? Or perhaps it depends, in which case, any resources you could recommend I read in order to make the call?

theKashey commented 2 years ago

FocusLock/FocusOn are behavior-only components, even if they leave some DOM-nodes behind.

You are more than welcome to put something, like dialog inside, and abstract the end combination as a Modal.

stale[bot] commented 1 year ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.