[Bug]: Escape button closes all panels

[drweb86]

Library

React / v8 (@fluentui/react)

System Info

System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
    Memory: 12.23 GB / 31.66 GB
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (105.0.1343.33)

Are you reporting Accessibility issue?

no

Reproduction

https://codepen.io/drweb86/pen/RwyVJeX (panel 1 is opened and panel 2 is opened above it)

Steps

• click with mouse on panel 2 This will set document.activeElement to body.
• press Escape Escape will be processed by onDismiss code of panel 1 instead of panel 2.

This scenario can seem dumb, but it is minimal. Production scenario I was investigated. 3 nested panels are opened. User closed panel 3 by X button. Then he pressed escape which caused all panels to close because escape was handled by panel 1. And it is handled by Panel 1 on dismiss code and user lost all his data.

BTW elementToFocusOnDismiss and similar options doesn't handle those scenarios.

Bug Description

Versions

fluent 7 last fluent 8 last fluent 9 - no panel component was found.

Actual Behavior

Escape pressed on Panel 2 closes panel 1 and panel 2

Expected Behavior

Escape pressed on Panel 2 closes Panel 2

Logs

No response

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]

can you recreate this issue in a codepen for easier testing? aka.ms/fluentpen

[drweb86]

can you recreate this issue in a codepen for easier testing? aka.ms/fluentpen

Hi @micahgodbolt . I've recreated it on CodePen and updated description of bug

[tomi-msft]

@khmakoto , can you confirm whether this is a bug?

[khmakoto]

@tomi-msft this does feel like something we should at least have a workaround for

[micahgodbolt]

This appears to be working correctly now.

[drweb86]

Hi @micahgodbolt ,

I rechecked it on my codepen (am i correct that it fetches latest v8 version?) with disabled cache. It is still there.

1. First, clock with mouse on Edge browser on label

2. Press Escape

You will observe this (panel 1 and panel 2 are closed both).

Expected result: See the panel 1 underneath panel 2 which is available on closing panel 2 with X button

[msft-fluent-ui-bot]

Gentle ping that this issue needs attention.

[micahgodbolt]

@drweb86 okay, I now understand the issue. Unfortunately, it did not repro very consistently, but once I did, it made sense.

When you click on the panel, you are not moving focus to the panel (because you aren't putting focus onto a focusable element). So when you hit escape, focus is on the body, and there is no way for the logic to know which panel to close.

I'm not sure if this is something we'd be able to solve in our system (and is certainly not as much of a bug as it just a missing feature).

My suggestion would be to either force focus onto something in the panel when you click on it, or create a close queue and only dismiss the panel who is at the top of the queue.

[tudorpopams]

@drweb86 this is going to be challenging to be implemented in a general way. Is @micahgodbolt 's answer help you fix the issue?

[drweb86]

Hi, @tudorpopams. @micahgodbolt answer did not solve the issue. It's a wide issue with Escape.

I was investigating the issue that in grid in panel there were items, customer adds another panel for creating entity and presses escape in a hope to see confirmation dialog with save etc. But he lost his data and since he was using Rich Text Editor - he had lost working day of work.

After investigation I found out issue with closing panels and popups - about mentioned active element.

And found out that its everywhere across app. Customers can lose their data if we have situation with 2 panels.

Fluent-ui is not aware that from one panel can be opened another panel, because for it each panel is callout and Escape is something like a core inside it.

I haven't found way in fluent source code to disable Escape at all because loosing input is major issue for people. Also, I understand your resistance to implement it, because it ruins simple design - every popup is callout. Callouts know nothing about each other. And I remember I did something like this for tracking what above each other in former angular project and I hated to support it after a while.

[JustSlone]

Agreed, adding parent/child logic between panels is out of scope here. The fix we would consider is adding the ability to disable or handle the close on Escape behavior, allowing applications to handle that and prevent loss of work. This would need to be done in a low risk non-breaking way, but could be added to v8. We don't have anyone available to work on adding this at this time, but would consider a contribution from the community of this feature.

We will also consider adding this to v9 Drawer (Panel) which is underdevelopment now. CC: @marcosmoura

[marcosmoura]

Hi @drweb86. We are currently working on the Drawer component for v9 that will ship with a feature to prevent accidentally closing by clicking outside (on the background scrim) and pressing the Esc key. This will come as an optional property, and it'll be soon available for preview.

Please not that multiple instances of Drawer will still not be aware of each other, so it'll require some logic on the Application side to decide which Drawer can/cannot be closed.

As @JustSlone mentioned, we are currently on full focus on the v9 component, so a contribution here would be more than welcome.

There was also a thorough research made by the designers to provide with a guidance on how to create more than one level of Drawer. This will also be available soon.

[drweb86]

@marcosmoura Thank you for letting me know. I appreciate your efforts about that.

There was also a thorough research made by the designers to provide with a guidance on how to create more than one level of Drawer. This will also be available soon.

Thanks, it would be really nice to have a guideline about that. Probably this guideline will cover scenario with one Drawer is above another Drawer and Escape will be handled by most child drawer without Escape disabling so the most child drawer will process OnClose event.

[microsoft-github-policy-service[bot]]

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.

[microsoft-github-policy-service[bot]]

Because this issue has not had activity for over 180 days, we're automatically closing it for house-keeping purposes.

Still require assistance? Please, create a new issue with up-to date details.