Overlay can't be closed using `esc` key when with interacted property actions

umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.

https://umbraco.com

MIT License

4.49k stars 2.69k forks source link

Overlay can't be closed using `esc` key when with interacted property actions #13601

Open bjarnef opened 1 year ago

bjarnef commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.3.2

Bug summary

When opening an overlay, e.g. infinite editing the overlay can be closed using esc key.

However when interacting with property actions inside the overlay this dropdown can be closed using esc key, but even several hits on esc key it never close the overlay via keyboard.

Specifics

No response

Steps to reproduce

Open an overlay with properties, e.g. listview opened in infinite mode and use keyboard to open property actions on a property like MTNP. Close the property action dropdown using esc key and furthermore try to close the overlay using esc key as well.

It only seems to be an issue as soon the property action dropdown has been opened.

Expected result / actual result

No response

github-actions[bot] commented 1 year ago

Hi there @bjarnef!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
We'll replicate the issue to ensure that the problem is as described.
We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

elit0451 commented 1 year ago

Hey @bjarnef

I can reproduce the problem as it can be seen in the video 🙂 I'll mark it as up for grabs for now so you or someone else from the community can help us improve the keyboard shortcut flow in this particular case 💪

https://user-images.githubusercontent.com/21998037/208639896-ee93d6b1-b418-4077-a6d0-c47a7619f443.mp4

github-actions[bot] commented 1 year ago

Hi @bjarnef,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)

bjarnef commented 1 year ago

Hi @elit0451

Yes, it should unbind esc on close here, but it seems something is still bound to esc key event (or it need to rebind the esc key event on overlay). https://github.com/umbraco/Umbraco-CMS/blob/60765b976c0ac49417c56821035cabae87454b78/src/Umbraco.Web.UI.Client/src/common/directives/components/property/umbpropertyactions.component.js#L52

RyuLindow commented 1 year ago

Looking at it during the Bristol hackathon.

RyuLindow commented 1 year ago

In my understanding, the keyboardservice listens for every single key on every single element which uses this service making it a global service.

I would imagine that rewriting this is the ideal solution, as this would need to be scoped down but I don't have the experience to do it. Also, not sure how much sense it would make given that we'll be releasing a new backoffice.

I've also noticed that the Close button has the hotkey="esc" property but the Remove all items button doesn't.

This is the property actions dropdown - https://github.com/umbraco/Umbraco-CMS/blob/33adbf41fa1f5c5d0759c70a7116114107addf56/src/Umbraco.Web.UI.Client/src/views/components/property/umb-property-actions.html

By commenting out this line I was able to get it to close both the actions and the property editor overlay.

Vid - https://www.dropbox.com/s/ligimbol4vp6em1/brave_5l9D6lgDZT.mp4?dl=0

bjarnef commented 11 months ago

I also noticed this happens in Umbraco 12.3.3 if one opens e.g. Media Picker overlay in infinite editing.

It can be tested with the example dashboard in this PR https://github.com/umbraco/Umbraco-CMS/pull/15339 on on the content type opening add a media picker or Block Grid with an image block.

However I noticed if double hit esc it does seem to close both overlay - but if waiting a bit after closing the media picker overlay, one seems to be trapped in the infinite overlay, in this case content editor overlay.