[Breaking Change][Fix]: remove default slotted title actions from dialog body #34469
Conversation
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-web-components/Avatar 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Avatar. - Dark Mode.normal.chromium_1.png | 298 | Changed |
vr-tests-web-components/MenuList 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/MenuList. - RTL.2nd selected.chromium.png | 17 | Changed |
| vr-tests-web-components/MenuList. - RTL.normal.chromium_1.png | 39082 | Changed |
vr-tests-web-components/Switch 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/Switch. - Dark Mode.normal.chromium_1.png | 92 | Changed |
There were 1 duplicate changes discarded. Check the build logs for more information.
There was a problem hiding this comment.
The Custom Close story for <fluent-dialog> still causes the default close action to happen after the alert happens in Storybook. Should the clickHandler() function check to see if event.defaultPrevented is false before running? That would allow for custom close action listeners to call event.preventDefault() to override the actual dialog-closing action.
|
So we can openly document it for posterity's sake, can you post about what options you evaluated, what the relative pros and cons were and why we went with this design. |
eljefe223
force-pushed
the
users/jes/dialog-close-button-aria
branch
from
3b716b8 to
faf7217
Compare
| */ | ||
| @attr({ mode: 'boolean', attribute: 'no-title-action' }) | ||
| public noTitleAction: boolean = false; | ||
| public clickHandler(event: MouseEvent): boolean | void { |
There was a problem hiding this comment.
Should this method name be made more descriptive? clickHandler sounds a bit too generic for what it actually does.
| * @returns true if the element is a dialog. | ||
| * @public | ||
| */ | ||
| export function isDialog(element?: Node | null, tagName: string = '-dialog'): element is Dialog { |
There was a problem hiding this comment.
Is there any case where the dialog tag name doesn't end with -dialog? I'm just wondering if the tagName parameter is necessary.
Previous Behavior
The
dialog-bodytemplate had a button defaulted in thetitle-actionsslot. This was preventing the ability to set an accesible label on the button itself.New Behavior
This PR does severalk things to fix the issue.
closeslot and keeps thetitle-actionsslot for sub actions in the titlecloseslot.Related Issue(s)