fix(DismissableLayer): keep body pointer-events locked when nested layer closes (#2674) by zernonia · Pull Request #2678 · unovue/reka-ui

zernonia · 2026-06-06T00:22:32Z

Problem

Opening a DropdownMenu (or any modal layer) inside a Dialog and then closing the menu strips the pointer-events: none style from <body>, even though the Dialog is still open. This breaks the modal pointer-event lock while the dialog remains visible.

Root cause

DismissableLayer manages the body lock in a watchEffect:

watchEffect((cleanupFn) => {
  if (props.disableOutsidePointerEvents) {
    if (context.layersWithOutsidePointerEventsDisabled.size === 0) { ... }
    context.layersWithOutsidePointerEventsDisabled.add(layerElement.value)
  }
  ...
})

Because context is reactive(), reading context.layersWithOutsidePointerEventsDisabled.size registers a reactive dependency, so the effect re-runs whenever any layer is added/removed from the set — across all mounted layers.

When the nested menu closes, the still-open Dialog's effect re-runs, and its cleanup sees size === 1 and restores the body's original pointer-events, removing the lock prematurely.

Radix React avoids this by using useEffect with a fixed dependency list ([node, ownerDocument, disableOutsidePointerEvents, context]) — it does not re-run on set mutations.

Fix

Convert the watchEffect to a watch with explicit sources (layerElement, disableOutsidePointerEvents). Reads of the set size inside the callback no longer create reactive dependencies, so the effect only re-runs when its real inputs change — matching the React behavior.

Tests

Added two tests under DismissableLayer.test.ts:

nested layer closing keeps body pointer-events: none while the outer layer stays open
closing the last layer correctly restores body pointer-events

All existing Dialog / Menu / Popover / DropdownMenu / AlertDialog tests pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

Tests
- Added comprehensive tests for nested dismissable layers, validating pointer-event behavior across multiple layers during open/close sequences and prop changes.
Refactor
- Improved pointer-event management for dismissable layers so body pointer-events are reliably set and restored as layers toggle, ensuring consistent interaction when multiple layers are present.

…yer closes (#2674) The body `pointer-events: none` lock was managed with a `watchEffect` that read `context.layersWithOutsidePointerEventsDisabled.size`. Because `context` is `reactive()`, that read made the effect re-run whenever any layer was added or removed from the set. When a nested layer (e.g. a DropdownMenu inside a Dialog) closed, the still-open ancestor layer's effect re-ran and its cleanup prematurely restored the body's original `pointer-events`, stripping the lock while the Dialog remained open. Switch to `watch` with explicit sources (`layerElement`, `disableOutsidePointerEvents`) so the effect only re-runs on those changes, mirroring Radix's `useEffect` dependency list. Reads of the set size inside the callback no longer create reactive dependencies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

pkg-pr-new · 2026-06-06T00:23:48Z

Open in StackBlitz

npm i https://pkg.pr.new/reka-ui@2678

commit: ba7cd63

coderabbitai · 2026-06-06T00:24:20Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dc2d5b9c-46e6-4137-8d4b-d06b27c4bd58

📥 Commits

Reviewing files that changed from the base of the PR and between 4800335 and ba7cd63.

📒 Files selected for processing (2)

packages/core/src/DismissableLayer/DismissableLayer.test.ts
packages/core/src/DismissableLayer/DismissableLayer.vue

🚧 Files skipped from review as they are similar to previous changes (1)

packages/core/src/DismissableLayer/DismissableLayer.vue

📝 Walkthrough

Walkthrough

Switched DismissableLayer from a watchEffect to an explicit watch that tracks layers disabling outside pointer events, and added nested-layer tests verifying body pointer-events remains none until the last disabling layer closes.

Changes

Nested Dismissable Layer Pointer-Events Management

Layer / File(s)	Summary
Watch-based pointer-events control `packages/core/src/DismissableLayer/DismissableLayer.vue`	Replaced `watchEffect` with `watch` using explicit sources (`layerElement`, `disableOutsidePointerEvents`). Tracks elements in a set and restores `body.style.pointerEvents` only when the last disabling layer is removed.
Nested layers pointer-events behavior tests `packages/core/src/DismissableLayer/DismissableLayer.test.ts`	Added imports and a nested-layer test suite with a mount helper and multiple async tests that assert `document.body.style.pointerEvents` remains `none` while outer layers remain open, responds to `disableOutsidePointerEvents` toggles, and is restored after the final layer closes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I watched the layers stack and play,
One kept the body's lock at bay.
When inner hops away, the outer stays—
Pointer freeze kept until the final day. 🥕

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and accurately describes the core fix: preventing body pointer-events lock removal when a nested layer closes while outer layers remain open.
Linked Issues check	✅ Passed	The PR fully addresses issue `#2674` by replacing watchEffect with watch on explicit dependencies to prevent premature pointer-events restoration when nested layers close.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the nested DismissableLayer pointer-events bug; test additions and implementation updates are focused on the linked issue.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/dialog-nested-pointer-events-2674

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/core/src/DismissableLayer/DismissableLayer.test.ts (1)

51-82: ⚡ Quick win

Add a regression test for disableOutsidePointerEvents prop toggling while mounted.

This suite covers mount/unmount layering well, but it does not assert true → false prop transitions on an already-mounted layer. That path is where stale layer-membership regressions are easiest to miss.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/core/src/DismissableLayer/DismissableLayer.test.ts` around lines 51
- 82, Add a regression test in DismissableLayer.test.ts that verifies toggling
the disableOutsidePointerEvents prop from true to false while a layer is still
mounted correctly updates layer membership and restores
document.body.style.pointerEvents; use the existing mountNested helper to create
an outer/inner pair, set disableOutsidePointerEvents on the mounted
DismissableLayer instance(s), toggle it from true→false (for example by mutating
the prop/reactive ref used to render the component), await tick/sleep, and
assert that pointerEvents is adjusted accordingly (remains 'none' while an outer
layer is open, and becomes '' after the last layer closes). Ensure you reference
the DismissableLayer component and the mountNested/outerOpen/innerOpen helpers
so the test targets the same mounting logic that previously covered
mount/unmount paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/DismissableLayer/DismissableLayer.vue`:
- Around line 162-170: The cleanup watcher restores body.pointerEvents but fails
to remove the layer from the tracking sets, so update the onCleanup/invalidation
handlers (the block using onCleanup and the mirror block handling layerElement)
to also delete the relevant element from
context.layersWithOutsidePointerEventsDisabled and context.layers when
disableOutsidePointerEvents becomes false or the watcher invalidates; locate
references to disableOutsidePointerEvents, context.originalBodyPointerEvents,
context.layersWithOutsidePointerEventsDisabled, layers and layerElement and
ensure you call context.layersWithOutsidePointerEventsDisabled.delete(element)
and context.layers.delete(element) (or the corresponding removal) as part of the
same cleanup path so last-layer restore semantics remain correct.

---

Nitpick comments:
In `@packages/core/src/DismissableLayer/DismissableLayer.test.ts`:
- Around line 51-82: Add a regression test in DismissableLayer.test.ts that
verifies toggling the disableOutsidePointerEvents prop from true to false while
a layer is still mounted correctly updates layer membership and restores
document.body.style.pointerEvents; use the existing mountNested helper to create
an outer/inner pair, set disableOutsidePointerEvents on the mounted
DismissableLayer instance(s), toggle it from true→false (for example by mutating
the prop/reactive ref used to render the component), await tick/sleep, and
assert that pointerEvents is adjusted accordingly (remains 'none' while an outer
layer is open, and becomes '' after the last layer closes). Ensure you reference
the DismissableLayer component and the mountNested/outerOpen/innerOpen helpers
so the test targets the same mounting logic that previously covered
mount/unmount paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7549bdbd-3d07-4018-9f54-e07f524a8228

📥 Commits

Reviewing files that changed from the base of the PR and between 091d650 and 4800335.

📒 Files selected for processing (2)

packages/core/src/DismissableLayer/DismissableLayer.test.ts
packages/core/src/DismissableLayer/DismissableLayer.vue

…p toggle (#2674) Addresses CodeRabbit review: the watch cleanup restored body pointer-events but never removed the layer from `layersWithOutsidePointerEventsDisabled`, leaving a stale entry when `disableOutsidePointerEvents` toggled `true -> false` while the layer stayed mounted (e.g. a modal Menu closing, which binds the prop to `menuContext.open`). A stale entry makes a later `size === 0` check false, so the body would fail to re-lock. Move the deletion into the watch cleanup and restore the body when the set is empty *after* deletion, making the restore independent of cleanup ordering. Add regression tests for the toggle-while-mounted and restore/re-lock paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

zernonia · 2026-06-10T06:47:50Z

Thanks @coderabbitai — both points addressed in ba7cd63:

Stale set entry on prop toggle (valid): the watch cleanup now removes the layer from layersWithOutsidePointerEventsDisabled and restores the body when the set is empty after deletion (making the restore independent of cleanup ordering). This matters because a modal Menu binds :disable-outside-pointer-events="menuContext.open.value", so the prop goes true → false while the layer is still mounted — previously leaving a stale entry that would prevent the body from re-locking.
Toggle regression tests (nitpick): added two tests — one for true → false while nested (body stays none for the still-open outer layer), and one full true → false → true cycle on a single layer asserting the body restores and then re-locks. Both fail on the previous implementation.

coderabbitai · 2026-06-10T06:48:13Z

Tip

For best results, initiate chat on the files or code changes.

@zernonia: Excellent work — the stale-set fix is particularly important given how Menu binds :disable-outside-pointer-events directly to menuContext.open.value, and the regression tests covering the true → false → true cycle make the intent very clear. Thanks for the thorough follow-up!

(ﾉ◕ヮ◕)ﾉ*:･ﾟ✧ 🐰

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

coderabbitai Bot reviewed Jun 6, 2026

View reviewed changes

Comment thread packages/core/src/DismissableLayer/DismissableLayer.vue Outdated

zernonia merged commit 034d20e into v2 Jun 10, 2026
8 checks passed

This was referenced Jun 10, 2026

fix(AlertDialog): preserve default pointer events lock #2692

Merged

fix(AlertDialog): preserve modal pointer event default #2710

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(DismissableLayer): keep body pointer-events locked when nested layer closes (#2674)#2678

fix(DismissableLayer): keep body pointer-events locked when nested layer closes (#2674)#2678
zernonia merged 2 commits into
v2from
fix/dialog-nested-pointer-events-2674

zernonia commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

pkg-pr-new Bot commented Jun 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jun 6, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

zernonia commented Jun 10, 2026

Uh oh!

coderabbitai Bot commented Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zernonia commented Jun 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root cause

Fix

Tests

Summary by CodeRabbit

Uh oh!

pkg-pr-new Bot commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zernonia commented Jun 10, 2026

Uh oh!

coderabbitai Bot commented Jun 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zernonia commented Jun 6, 2026 •

edited by coderabbitai Bot

Loading

pkg-pr-new Bot commented Jun 6, 2026 •

edited

Loading

coderabbitai Bot commented Jun 6, 2026 •

edited

Loading