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

Toast messages are hiding under dialog backdrop #617

Closed
mahmudmardini opened this issue Jun 4, 2023 · 4 comments · Fixed by #633
Closed

Toast messages are hiding under dialog backdrop #617

mahmudmardini opened this issue Jun 4, 2023 · 4 comments · Fixed by #633
Labels
bl-dialog Issues related with Dialog component bug Something isn't working released on @beta released

Comments

@mahmudmardini
Copy link
Contributor

mahmudmardini commented Jun 4, 2023

Issue description

When we have a toast message to show while the dialog is open, the messages are hidden behind the dialog backdrop.
This is affecting the UX since the toasts like error or success messages are not enough clear to users.

Baklava Version

v2.2.0-beta.2

Low (slightly annoying)

@mahmudmardini mahmudmardini added bug Something isn't working bl-dialog Issues related with Dialog component labels Jun 4, 2023
@muratcorlu
Copy link
Contributor

This is a known limitation of using native Dialog element because Dialog element renders on top-layer, which you can not override it via z-index values. Technically Popover API is on the way which will fix this issue, but until that time, I think we may check for a workaround for this.

Currently our dialog supports older browsers that don't support dialog element. We make that check automatically and use a polyfilled version instead with that small decision point:

  private get hasHtmlDialogSupport() {
    return !!window.HTMLDialogElement;
  }

Maybe we can consider making this a property that you can manually set outside like:

@property({ type: Boolean })
levelled = !window.HTMLDialogElement;

So, whenever a popover opens within a browser that doesn't support Popover API, dialog that is already visible can be switched to levelled variant which will convert dialog to a version that respects z-index rules. But:

  • We need to be careful about UX effects of this. If this switch causes a rerender on dialog content, that would be awful for use-cases like filling a form inside dialog, or reading a long text, since a new notification will interrupt these badly.
  • If we set this true by default in a project, then we'll miss the opportunities that native dialog element provides us and we can not easily implement in a custom dialog element, like avoiding cropped dialog because of parent style rules and keyboard navigation trap.

Please feel free to share your opinions. @Enes5519

PS: levelled name could be switched with a better option.

@Enes5519
Copy link
Contributor

It is healthiest to do it this way, different ways are not ready yet, so we can pass prop in this way.

@github-actions
Copy link

🎉 This issue has been resolved in version 2.2.0-beta.9 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This issue has been resolved in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bl-dialog Issues related with Dialog component bug Something isn't working released on @beta released
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants