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(Toast): unreachable behind overlays #2650

Merged
merged 3 commits into from
Nov 15, 2024

Conversation

sandros94
Copy link
Collaborator

πŸ”— Linked issue

Resolves #2646

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The fix is composed into two parts:

  • prevent the default's of DialogContent if the overed element is a toaster
  • make sure that the Toast is a valid css target for pointer events

no updates to the vitest's snapshots are required

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@sandros94 sandros94 self-assigned this Nov 15, 2024
@sandros94 sandros94 added the v3 #1289 label Nov 15, 2024
Copy link

pkg-pr-new bot commented Nov 15, 2024

pnpm add https://pkg.pr.new/@nuxt/ui@2650

commit: aed57e6

Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It actually works but one thing I don't understand is there is no data-sonner-toaster in the DOM πŸ€”

@sandros94
Copy link
Collaborator Author

It actually works but one thing I don't understand is there is no data-sonner-toaster in the DOM πŸ€”

Ikr? 🀣

But considering that radix-vue is a port of the react version, that bases its toaster on Sonner, there might be some pollution that works in our favor.

Once we migrate to Reka, and if things do change, we just update the search (even just a div > ol works, but it is kinda generic)

@benjamincanac benjamincanac merged commit 0daac5b into nuxt:v3 Nov 15, 2024
2 checks passed
@benjamincanac
Copy link
Member

@sandros94 I was testing to remove this code now that we migrated to Reka UI and it does seem to work anyway. Would you mind confirming that we can remove this safely?

@sandros94
Copy link
Collaborator Author

Let me double check

@sandros94
Copy link
Collaborator Author

@benjamincanac yes, I double confirm that the preventDefault is no longer needed. Opening PR

Copy link
Member

Thanks for the confirmation, no need to open a PR I'm in the middle of refactoring this already. I'll rename prevent-close to dismissible to match Drawer component as well.

Copy link
Member

We still have the issue with the Toasts when using a Drawer but this might be due to the fact that it's still using radix-vue instead of reka-ui.

@sandros94
Copy link
Collaborator Author

Thanks for the confirmation, no need to open a PR I'm in the middle of refactoring this already.

Ah great! It only needs pointer-events-auto in the toaster's theme

We still have the issue with the Toasts when using a Drawer but this might be due to the fact that it's still using radix-vue instead of reka-ui.

probably πŸ€”

@sandros94 sandros94 deleted the fix-toast-behind-overlays branch December 16, 2024 11:46
Copy link
Member

@sandros94 Check out:

@sandros94
Copy link
Collaborator Author

@sandros94 Check out:

Looks great πŸ‘Œ
Indeed the only issue I can reproduce is the drawer one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 #1289
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v3] Toaster gets rendered behind Slideover's and Modal's overlays
2 participants