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

fix(dialog): fix scrollbar pointer-down click prevent dialog closing #625

Merged
merged 6 commits into from
Feb 10, 2024

Conversation

romanhrynevych
Copy link
Contributor

this PR closes #624

@romanhrynevych romanhrynevych marked this pull request as draft January 18, 2024 21:32
@romanhrynevych
Copy link
Contributor Author

This PR don't fix Toast issue, will try to fix this a little bit later 🙂

@zernonia
Copy link
Member

Thanks for the PR @romanhrynevych . we can prevent dismissing the dialog when clicking on the Toast. So we dont need explicit fix for this.

Your fixes for clicking on scrollbar looks good to me 😁

@romanhrynevych
Copy link
Contributor Author

romanhrynevych commented Feb 5, 2024

@zernonia thank you for your great job on this package ❤️

How can we prevent closing Dialog by dismissing it? I added video asset to clarify bug that I'm cannot fix for now)

Export-1707161704046

@zernonia
Copy link
Member

zernonia commented Feb 6, 2024

@zernonia thank you for your great job on this package ❤️

How can we prevent closing Dialog by dismissing it? I added video asset to clarify bug that I'm cannot fix for now)

@romanhrynevych We can prevent default following this comment here.

The above shows Toast component, but it's technically the same for sonner component

@romanhrynevych
Copy link
Contributor Author

Think that it will be helpful to point this inside docs, so I add some demo for this type of usage, maybe this will clarify things for more devs 🙂

dialog-toast-demo.mp4

@sadeghbarati hi 👋 What you think about this?)

@romanhrynevych romanhrynevych marked this pull request as ready for review February 8, 2024 21:47
docs/package.json Outdated Show resolved Hide resolved
@sadeghbarati
Copy link
Collaborator

Great Job @romanhrynevych ❤️

@romanhrynevych
Copy link
Contributor Author

romanhrynevych commented Feb 9, 2024

I just noticed one more issue, need to resolve it too.

So, when I add Toast component, because of it position in DOM (on top of our Dialog) we never will click on toast because DialogOverlay overlaps it, did you know how to Teleport Toast to the end of body? I search inside vue docs, but nothing 😞

dialog-toast-z-index.mp4

Also, I don't see any need to @interact-outside call on DialogContent, because it will never called when clicking on another elements 🤔

@romanhrynevych
Copy link
Contributor Author

Previously I was teleporting to html but it places Toast after </body>, don't think that it is like it should work 😂

@zernonia
Copy link
Member

zernonia commented Feb 10, 2024

Thanks @romanhrynevych ! After setting pointer-events-auto to <ToastViewport>, we can click on the Toast without closing the Dialog. I will revert the changes to use sonner as I believe other dev could be using other Toast package, and it demonstrates a good example of @interactOutside!

@zernonia zernonia merged commit ecf993e into radix-vue:main Feb 10, 2024
1 check failed
@zernonia
Copy link
Member

Thanks for the PR @romanhrynevych ! Well done! 💚

@romanhrynevych romanhrynevych deleted the fix/dialog-scrollbar-click branch February 11, 2024 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Prevent dismissing Dialog when interacting with Toasts (Overlay Components) or native scrollbar
3 participants