Skip to content

fix: exclude sonner toast elements from sheet outside-click detection - #3805

Merged
akshaydeo merged 13 commits into
devfrom
05-27-fix_prevent_sheets_closing_on_toast_dismiss
May 27, 2026
Merged

fix: exclude sonner toast elements from sheet outside-click detection#3805
akshaydeo merged 13 commits into
devfrom
05-27-fix_prevent_sheets_closing_on_toast_dismiss

Conversation

@impoiler

@impoiler impoiler commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes an issue where clicking on Sonner toast notifications while a Sheet component is open would incorrectly trigger the sheet's outside-click detection, causing unintended sheet dismissal.

Changes

  • Added [data-sonner-toast] and [data-sonner-toaster] to the list of portaled elements that the SheetContent outside-click handler ignores, preventing toasts from being treated as clicks outside the sheet.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

  1. Open a Sheet component in the UI.
  2. Trigger a Sonner toast notification while the sheet is open.
  3. Click on the toast notification.
  4. Verify the sheet remains open and is not dismissed by the toast click.
cd ui
pnpm i || npm i
pnpm build || npm run build

Screenshots/Recordings

N/A

Breaking changes

  • Yes
  • No

Related issues

N/A

Security considerations

None.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@impoiler, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 4 minutes and 18 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d9e4bef8-510f-4865-be5f-44c73fb67907

📥 Commits

Reviewing files that changed from the base of the PR and between 61c6161 and 7652ab8.

📒 Files selected for processing (1)
  • ui/components/ui/sheet.tsx
📝 Walkthrough

Walkthrough

The Sheet component's outside-interaction suppression logic is expanded to recognize additional portal-like elements (Radix Popper content and Sonner toast elements) as portaled targets, preventing outside handlers from executing when interactions occur within these elements.

Changes

Sheet portal element detection

Layer / File(s) Summary
Portal element detection expansion
ui/components/ui/sheet.tsx
isPortaledElement helper now includes additional target.closest() checks for Radix Popper content wrapper and Sonner toast elements, expanding which outside interactions are suppressed in handlePointerDownOutside and handleInteractOutside.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • maximhq/bifrost#3674: Addresses Sonner toast interaction during overlay usage by ensuring toasts remain clickable via CSS while this PR prevents Sheet outside-interaction handlers from firing on toast elements.
  • maximhq/bifrost#3598: Changes to the Sonner Toaster positioning and close button interact with this PR's expansion of Sonner toast element detection in Sheet's outside-interaction logic.

Suggested reviewers

  • akshaydeo

Poem

🐰 A Sheet learned to see through the Sonner toast,
And past Radix poppers that hover the most,
No handlers shall fire when portals appear,
The cleanest of clicks, both far and near!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: excluding Sonner toast elements from the sheet's outside-click detection logic.
Description check ✅ Passed The description includes all critical sections: a clear summary of the problem, specific changes made, type of change, affected areas, testing steps, and confirmation of no breaking changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-27-fix_prevent_sheets_closing_on_toast_dismiss

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the change is a two-line selector addition inside an existing exclusion list with no impact on other code paths.

The change is narrowly scoped to adding two well-understood CSS attribute selectors that match Sonner's documented DOM attributes. It follows the same pattern already used for react-select and Radix poppers, and does not touch any logic outside the isPortaledElement helper.

No files require special attention.

Important Files Changed

Filename Overview
ui/components/ui/sheet.tsx Adds [data-sonner-toast] and [data-sonner-toaster] to the portaled-element exclusion list in isPortaledElement, preventing Sonner toast clicks from dismissing open sheets.

Reviews (5): Last reviewed commit: "fix: prevent sheets closing on toast dis..." | Re-trigger Greptile

@impoiler
impoiler force-pushed the 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet branch from 0b58704 to f7660f7 Compare May 27, 2026 12:06
@impoiler
impoiler force-pushed the 05-27-fix_prevent_sheets_closing_on_toast_dismiss branch from 5a9d042 to 61c6161 Compare May 27, 2026 12:19
@impoiler
impoiler force-pushed the 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet branch from f7660f7 to 0fc25a8 Compare May 27, 2026 12:19
@coderabbitai
coderabbitai Bot requested a review from akshaydeo May 27, 2026 12:21
coderabbitai[bot]
coderabbitai Bot previously approved these changes May 27, 2026
@impoiler
impoiler force-pushed the 05-27-fix_prevent_sheets_closing_on_toast_dismiss branch from 61c6161 to fe5bb9f Compare May 27, 2026 12:30
@impoiler
impoiler force-pushed the 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet branch from 0fc25a8 to 236bdfa Compare May 27, 2026 12:30
@impoiler
impoiler force-pushed the 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet branch from fa7e8a2 to c620789 Compare May 27, 2026 13:16
@impoiler
impoiler force-pushed the 05-27-fix_prevent_sheets_closing_on_toast_dismiss branch from 5c786d6 to 7652ab8 Compare May 27, 2026 13:16

akshaydeo commented May 27, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 27, 1:44 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 27, 2:03 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 05-27-refactor_use_sheetnavigation_hook_in_mcp_detail_sheet to graphite-base/3805 May 27, 2026 14:00
@akshaydeo
akshaydeo changed the base branch from graphite-base/3805 to dev May 27, 2026 14:03
@akshaydeo
akshaydeo dismissed coderabbitai[bot]’s stale review May 27, 2026 14:03

The base branch was changed.

@akshaydeo
akshaydeo merged commit d8b35f1 into dev May 27, 2026
10 checks passed
@akshaydeo
akshaydeo deleted the 05-27-fix_prevent_sheets_closing_on_toast_dismiss branch May 27, 2026 14:03
akshaydeo pushed a commit that referenced this pull request May 29, 2026
…#3805)

## Summary

Fixes an issue where clicking on Sonner toast notifications while a Sheet component is open would incorrectly trigger the sheet's outside-click detection, causing unintended sheet dismissal.

## Changes

- Added `[data-sonner-toast]` and `[data-sonner-toaster]` to the list of portaled elements that the `SheetContent` outside-click handler ignores, preventing toasts from being treated as clicks outside the sheet.

## Type of change

- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (React)
- [ ] Docs

## How to test

1. Open a Sheet component in the UI.
2. Trigger a Sonner toast notification while the sheet is open.
3. Click on the toast notification.
4. Verify the sheet remains open and is not dismissed by the toast click.

```sh
cd ui
pnpm i || npm i
pnpm build || npm run build
```

## Screenshots/Recordings

N/A

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

N/A

## Security considerations

None.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
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.

2 participants