Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feat]: Allow overlays to decide if they can be closed or not #1853

Closed
1 task done
sorinsarca opened this issue Oct 11, 2021 · 3 comments · Fixed by #3456
Closed
1 task done

[Feat]: Allow overlays to decide if they can be closed or not #1853

sorinsarca opened this issue Oct 11, 2021 · 3 comments · Fixed by #3456
Labels
Component: Overlay enhancement New feature or request Needs discussion Proposed UX or spec changes Overlay v2 Confirmed Post overlay API Review after Overlay V2 Ships

Comments

@sorinsarca
Copy link
Contributor

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

overlay

Description of the requested feature

The main problem starts here https://github.com/adobe/spectrum-web-components/blob/main/packages/overlay/src/overlay-stack.ts#L495-L501

Pressing Escape will close the overlay no matter what, and this is problematic for some modals.

Imagine that we have an upload dialog, once the user clicks the "Upload" button the process begins, and a progress bar is
updated from 0% to 100% indicating the upload status. In this particular case the process is not cancellable, the user must wait until the upload is complete, otherwise the app will be (lets say) in some undefined state. Now, if the user accidentally hits Escape the dialog will close, and lots of errors will appear (because the progress bar no longer exists, but the async process tries to update it, or some other errors).

Currently the only option is to monkey-patch and set handleKeyUp method to nop, which completely disables the Escape close mode.

Mockups or screenshots

No response

Implementation notes or ideas

Maybe some isClosable property?

@sorinsarca sorinsarca added enhancement New feature or request triage An issue needing triage labels Oct 11, 2021
@hunterloftis hunterloftis added Needs discussion Proposed UX or spec changes and removed triage An issue needing triage labels Oct 13, 2021
@Cirras
Copy link

Cirras commented Feb 25, 2022

I've run into this problem with a component nested within a modal overlay.

When focused, this nested component should completely swallow Escape keyboard events. As noted here, the overlay stack will see the Escape keyup event and close the entire modal.

The perfect solution for me would be some way to prevent the Escape keyup event from propagating to the overlay stack.
My hack solution has similarly been to monkey-patch the handleKeyUp method.

@Westbrook
Copy link
Contributor

There are two types of overlays that may fall into this category as we look to revisit the capability that we're offering here:

  • popup=manual as outlined by OpenUI, this content is non-blocking, but can only be dismissed by direct action with the content in question or via a timer
  • "page" overlay, this is synthetic to an overlay system but outlines content that acts as a new page to the application in question regardless of whether other content has been removed, it is blocking and not dismissed by user interaction unless specific UI is provided

@najikahalsema
Copy link
Collaborator

That second suggestion would make sense to implement for use cases like an interrupted connection during an in-progress upload, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Overlay enhancement New feature or request Needs discussion Proposed UX or spec changes Overlay v2 Confirmed Post overlay API Review after Overlay V2 Ships
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants