[Bug]: dialog directly mounted with open=true could not get focus

KagamiChan commented 1 year ago

Library

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

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (32) x64 AMD Ryzen 9 7950X 16-Core Processor
    Memory: 33.54 GB / 63.16 GB
  Browsers:
    Edge: Spartan (44.22621.1105.0), Chromium (110.0.1587.41)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

no

Reproduction

https://codesandbox.io/s/focused-sid-ml3ty9

Bug Description

Actual Behavior

the dialog with open=true is mounted and open, but none of the inputs could be focused

Expected Behavior

When the dialog is mounted, elements in the dialog could be focused

Logs

@fluentui/react-dialog: a Dialog should have at least one focusable element inside DialogSurface.
Please add at least a close button either on `DialogTitle` action slot or inside `DialogActions` 
    in Dialog (created by WithForm)
    in WithForm

Requested priority

Normal

Products/sites affected

No response

Are you willing to submit a PR to fix?

no

Validations

[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] The provided reproduction is a minimal reproducible example of the bug.

micahgodbolt commented 1 year ago

Also note that if you leave "open={true}" the behavior persists. I thought maybe it was a race condition.

One question is, why do you need to conditionally render the Dialog? The DOM is not injected until open is set to true. It's possible that this behavior is by design, but I will check.

KagamiChan commented 1 year ago

Thanks for helping,

In our project we use redux to store the Dialog instance and mount this instance in a same place. So we always set open=true for these <Dialog />s. And we see the same issue when the first <Dialog /> is saved to the store

Another use case I can think of could be immediately opening up a dialog when user navigates to a specific path with browser router or server side

The code worked for version 9.8.0 but we discovered this issue when I tried to upgrade to 9.14.0/19.15.0

bsunderhus commented 1 year ago

Seems like our a11y mechanism from tabster is locking you out of the dialog. Wrap the Button that is opening the Dialog component with DialogTrigger to ensure proper attributes to the Button, otherwise there's not way to know that the navigation you've made is a proper one.

https://codesandbox.io/s/agitated-fog-muketx?file=/example.tsx

There's a session on Dialog's documentation dedicated to show what should you do if you have a trigger outside of a dialog. https://react.fluentui.dev/?path=/docs/components-dialog--default#trigger-outside-dialog

KagamiChan commented 1 year ago

Thanks @bsunderhus you're correct, the button should be wrapped. However I think the button itself is irrelevant to the issue described here.

I'm able to construct a modified example without the button, please check https://codesandbox.io/s/focused-sid-ml3ty9?file=/example.tsx, here I use setTimeout to mount the Dialog which does not require any user interaction

updated the example link in the issue to the new one too

bsunderhus commented 1 year ago

The problem is basically the same @KagamiChan, our a11y mechanism expects the dialog to be opened by a click on an element with the proper attributes, and since you're not doing that you're getting locked out of the dialog.

Definitely there should be a proper alternative here that allow you to use the Dialog component without getting yourself locked out of it by tabster. I'll investigate that! Thanks for opening the issue.

bsunderhus commented 1 year ago

After a deep dive into this, seems like tabster is not being properly initialised. This has been cause on the last update of tabster version 4.

A quick fix for this problem is to manually initialise tabster by ivoking some of the tabster hooks, like this: https://codesandbox.io/s/quizzical-danilo-r7f55e

Another quick solution would be to manually focus on the first element: https://codesandbox.io/s/xenodochial-taussig-x8wzbx?file=/example.tsx

khowling commented 1 year ago

+1 Upgraded project to fluentui9, using dialog for data entry with open attribute (was using Panel in v8), and immediatly hit this issue.

bsunderhus commented 1 year ago

This is under investigation by tabster experts https://github.com/microsoft/tabster/issues/250! I'll keep this updated if anything changes