Skip to content

bugfix: enables Escape to dismiss alert Dialog#28276

Merged
bsunderhus merged 1 commit into
microsoft:masterfrom
bsunderhus:bsunderhus/issue28010
Jun 23, 2023
Merged

bugfix: enables Escape to dismiss alert Dialog#28276
bsunderhus merged 1 commit into
microsoft:masterfrom
bsunderhus:bsunderhus/issue28010

Conversation

@bsunderhus

Copy link
Copy Markdown
Contributor

Previous Behavior

  1. an alert dialog would not close on Escape key press

New Behavior

  1. an alert dialog now closes on Escape key press
  2. change wrongly named file useDialogTrigger.styles.ts

Related Issue(s)

@fabricteam

Copy link
Copy Markdown
Collaborator

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
InfoButton mount 6 15 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 599 581 5000
Button mount 284 290 5000
Field mount 1061 1069 5000
FluentProvider mount 632 664 5000
FluentProviderWithTheme mount 81 86 10
FluentProviderWithTheme virtual-rerender 73 66 10
FluentProviderWithTheme virtual-rerender-with-unmount 72 68 10
InfoButton mount 6 15 5000 Possible regression
MakeStyles mount 847 868 50000
Persona mount 1609 1578 5000
SpinButton mount 1275 1296 5000

@size-auditor

size-auditor Bot commented Jun 21, 2023

Copy link
Copy Markdown

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 5809e04461b488016393f68d0c809d5b43eb93db (build)

@codesandbox-ci

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 20e9131:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
65.162 kB
17.952 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
210.365 kB
58.697 kB
react-components
react-components: FluentProvider & webLightTheme
36.395 kB
11.996 kB
react-dialog
Dialog (including children components)
92.072 kB
27.46 kB
react-portal-compat
PortalCompatProvider
6.473 kB
2.196 kB
🤖 This report was generated against 5809e04461b488016393f68d0c809d5b43eb93db

@fabricteam

Copy link
Copy Markdown
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

@bsunderhus
bsunderhus marked this pull request as ready for review June 22, 2023 08:38
@bsunderhus
bsunderhus merged commit 5819305 into microsoft:master Jun 23, 2023
@bsunderhus
bsunderhus deleted the bsunderhus/issue28010 branch June 23, 2023 11:43
@msft-fluent-ui-bot

Copy link
Copy Markdown
Collaborator

🎉@fluentui/react-dialog@v9.5.11 has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot

Copy link
Copy Markdown
Collaborator

🎉@fluentui/react-components@v9.22.1 has been released which incorporates this pull request.:tada:

Handy links:

@goamn

goamn commented Sep 20, 2023

Copy link
Copy Markdown

@bsunderhus @ling1726 Why is the Escape key allowed to close the Dialog with modalType="alert"? This doesn't make sense to me. Now there is no way to create a Dialog that can not be closed. Alert was the only modalType to prevent the Dialog from closing and forcing a user to take an action. Both Modal or non-Modal allow the user to close the Dialog. Also now the documentary is invalid, please see below:
image

@bsunderhus

Copy link
Copy Markdown
Contributor Author

Also now the documentary is invalid

Hey there @goamn, you're completely right, I'll see to create an issue to update the documentation!

Why is the Escape key allowed to close the Dialog with modalType="alert"? This doesn't make sense to me.

This change is due to WAI-ARIA alertdialog pattern compliance, an alert dialog has no such feature! you should always be able to close a dialog by pressing the Escape button.

Now there is no way to create a Dialog that can not be closed. Alert was the only modalType to prevent the Dialog from closing and forcing a user to take an action. Both Modal or non-Modal allow the user to close the Dialog.

You can still control the opening state and avoid opening on Escape by using the controlled version of a dialog:

https://codesandbox.io/s/sad-star-c7ytcs?file=/example.tsx

@goamn

goamn commented Sep 21, 2023

Copy link
Copy Markdown

Hello, ok thank you, at least there is a second way :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Enable Escape to dismiss alert Dialog

6 participants