Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Dialog Docs nits #25132

Closed
5 tasks done
micahgodbolt opened this issue Oct 7, 2022 · 3 comments · Fixed by #25170
Closed
5 tasks done

Bug: Dialog Docs nits #25132

micahgodbolt opened this issue Oct 7, 2022 · 3 comments · Fixed by #25170

Comments

@micahgodbolt
Copy link
Member

micahgodbolt commented Oct 7, 2022

Library

React Components / v9 (@fluentui/react-components)

  • With Form says 'Having a formulary', which I assume should be 'Having a form'.
  • Trigger Outside of Dialog It appears that the only thing added by DialogTrigger is the aria-haspopup="dialog". If that's the only thing added, we should note that, and not make the use of DialogTrigger sound so critical. Bug: Dialog Docs nits #25132 (comment)
  • opening the "Open non-modal dialog" in No Focusable Element leaves the user in a state where they have to refresh the page to return to a useable page state. While I understand that this is the lesson we're trying to teach, I feel this is a bit too much for someone that clicked the button out of curiosity. Either provide a way to close the modal, or at least warn what is about to happen. react-dialog: send focus to the dialog root when no other focusables are present #25150
  • We tell people not to open a dialog from a dialog. I'm glad that we have validated that it is technically possible, but we already talk about it in the 'dont' section. There is no need to have an example of it.
  • Does dialog title need it's own Story page, or can customizing the actions be an example on the main dialog page chore(react-dialog): updates stories #25070
@bsunderhus
Copy link
Contributor

Trigger Outside of Dialog It appears that the only thing added by DialogTrigger is the aria-haspopup="dialog". If that's the only thing added, we should note that, and not make the use of DialogTrigger sound so critical.

It actually also includes tabster deloser attributes which will ensure proper focus behaviour.

@bsunderhus
Copy link
Contributor

opening the "Open non-modal dialog" in No Focusable Element leaves the user in a state where they have to refresh the page to return to a useable page state. While I understand that this is the lesson we're trying to teach, I feel this is a bit too much for someone that clicked the button out of curiosity. Either provide a way to close the modal, or at least warn what is about to happen.

This will be addressed by this #25150

@bsunderhus
Copy link
Contributor

We tell people not to open a dialog from a dialog. I'm glad that we have validated that it is technically possible, but we already talk about it in the 'don't' section. There is no need to have an example of it.

This is a valid point. I guess I can keep the validation on a e2e test and omit it from the storybook itself 🤔

@msft-fluent-ui-bot msft-fluent-ui-bot added Status: Fixed Fixed in some PR and removed Status: In PR labels Oct 11, 2022
@microsoft microsoft locked as resolved and limited conversation to collaborators Nov 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants