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

User Feedback widget appears behind modal dialog #13699

Closed
3 tasks done
rodolfoBee opened this issue Sep 16, 2024 · 4 comments
Closed
3 tasks done

User Feedback widget appears behind modal dialog #13699

rodolfoBee opened this issue Sep 16, 2024 · 4 comments
Labels
Package: browser Issues related to the Sentry Browser SDK

Comments

@rodolfoBee
Copy link
Member

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8.30.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

https://github.com/HealperDK/sentry-feedback-form-modal-issue-example

Steps to Reproduce

Steps are in the Read Me

Expected Result

Sentry feedback widget appears "in the foreground" and users can interact with it without closing the modal dialog.

Actual Result

The widget appears behind the modal dialog.

@github-actions github-actions bot added the Package: browser Issues related to the Sentry Browser SDK label Sep 16, 2024
@HealperDK
Copy link

Hi. I was the one who originally submitter the issue to your support. In case you have any questions :)

@ryan953 ryan953 self-assigned this Sep 16, 2024
@ryan953
Copy link
Member

ryan953 commented Sep 16, 2024

@HealperDK thanks for the awesome example repo!

It's clear from that example that the dialog.showModal() method is putting it's content on the very top of everything, and our feedback form can't compete, even by adjusting the z-index to outrageous levels.

I think this stack-overflow question and answers summarize the situation:

What we can do is see if the feedback form can use .showModal() internally so that it can be on top of the dialog in the example repo. What I'd want to check though is;

  • does it actually solve the problem when there are two modals on the page, or does one replace the other?
  • does the 'take screenshot' interaction still work ok, with or without other modals on the page?
  • does it significantly break css or other customizations folks might have applied to the form? will we need a major version bump to land it?

What I would suggest to you in the mean time is to explore mounting the feedback into it's own <dialog> and call .showModal() on that in order to raise it to the toplayer of the page.
Something like this looks like a start, but has some problems still:

commit d39a7b0782833b3514d88fc5a85541e14e5e4b31
Author: Ryan Albrecht <[email protected]>
Date:   Mon Sep 16 08:41:24 2024 -0700

    Exploration to move the feedback dialog into the top-layer of the page

diff --git index.html index.html
index 3a713a0..383b52f 100644
--- index.html
+++ index.html
@@ -15,5 +15,7 @@
 
 <button id="modal-btn" >Open Modal</button>
 
+<dialog id="feedbackDialog"></dialog>
+
 </body>
 </html>
diff --git index.js index.js
index 57d8a47..e439a90 100644
--- index.js
+++ index.js
@@ -12,6 +12,8 @@ const modalButton = document.getElementById('modal-btn');
 const sentryFeedbackButton = document.getElementById('sentry-feedback-btn');
 const modalCloseButton = document.getElementById('modal-close-btn');
 
+const feedbackDialog = document.getElementById('feedbackDialog');
+
 
 function openModal() {
     modalDialog.showModal();
@@ -24,6 +26,7 @@ function closeModal() {
 async function openSentryUserFeedbackForm() {
     const sentryFeedback = Sentry.feedbackIntegration({
         autoInject: false,
+        id: 'sentry-feedback', // this is the default, adding it here for visibility
     });
     // console.log(feedback)
     //
@@ -32,7 +35,10 @@ async function openSentryUserFeedbackForm() {
     // form.open();
     const form = await sentryFeedback.createForm();
     form.appendToDom();
+    const feedbackNode = document.getElementById('sentry-feedback');
+    feedbackDialog.append(feedbackNode);
     form.open();
+    feedbackDialog.showModal();
 
 
 // Create and render the button
@@ -47,4 +53,4 @@ async function openSentryUserFeedbackForm() {
 
 modalButton.addEventListener('click', openModal);
 modalCloseButton.addEventListener('click', closeModal);
-sentryFeedbackButton.addEventListener('click', openSentryUserFeedbackForm);
\ No newline at end of file
+sentryFeedbackButton.addEventListener('click', openSentryUserFeedbackForm);

@ryan953 ryan953 removed their assignment Sep 16, 2024
@HealperDK
Copy link

HealperDK commented Sep 17, 2024

Thanks for the suggestion of mounting the feedback form onto my own dialog. I hadn't thought of that.

I managed to get it working.
If anyone else needs this I will post my solution here. It needed a bit of event handler shenanigans before it worked properly for me.

const form = await sentryFeedback.createForm();
form.appendToDom();

// Create a hidden dialog to hold the feedback form
const invisibleDialog = document.createElement("dialog")
invisibleDialog.classList.add("transparent-backdrop");  // Hide the backdrop
invisibleDialog.style.border = "none";  // Hide the rest of the modal
document.body.appendChild(invisibleDialog);

// Find the node containing the feedback form and add it to the hidden dialog
const feedbackNode = document.getElementById('sentry-feedback')
invisibleDialog.append(feedbackNode);
invisibleDialog.showModal();

// Now we can open our form
form.open();

//For the form to work correctly we need to add certain event listeners
invisibleDialog.addEventListener("close", (event) => {
    // Remove entire dialog from the DOM when it is closed
    invisibleDialog.remove()
});

invisibleDialog.addEventListener("click", (event) => {
    // When dialog (backdrop) is clicked, close the dialog
    invisibleDialog.close();
});


form.el.querySelector('dialog').addEventListener('close', () => {
    // When form is close also clsoe the dialog
    invisibleDialog.close();
});

// Find the cancel button. Its the last of the default buttons. Not ideal but there is no good selector to 
// use here
const defaultButtons = form.el.querySelectorAll('.btn--default');
defaultButtons[defaultButtons.length - 1].addEventListener("click", () => {
    invisibleDialog.close();
});
        

@andreiborza
Copy link
Member

@HealperDK glad you got it to work and thank you for posting about it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: browser Issues related to the Sentry Browser SDK
Projects
Archived in project
Development

No branches or pull requests

4 participants