Skip to content

Keep the Content sidebar interactive after deletions - #2536

Open
3mdistal wants to merge 1 commit into
mainfrom
codex/content-sidebar-delete-dialog-owner
Open

Keep the Content sidebar interactive after deletions#2536
3mdistal wants to merge 1 commit into
mainfrom
codex/content-sidebar-delete-dialog-owner

Conversation

@3mdistal

Copy link
Copy Markdown
Contributor

Problem

Deleting an active page from Content could leave the entire sidebar unable to receive pointer input until the browser was refreshed. The failure affected ordinary pages, database pages, and nested page subtrees.

The delete command started in a modal dropdown and immediately opened a row-owned confirmation dialog. Radix modal layers could overlap their body pointer locks, and optimistic deletion could then unmount the row that owned the dialog, stranding the page with pointer events disabled.

Approach

Keep confirmation ownership in the stable sidebar root and sequence each modal transition only after the previous body pointer lock has cleared. The existing optimistic cache updates, navigation, and document/database delete actions remain unchanged.

What changed

  • Route ordinary-page and database-page delete requests to one sidebar-owned confirmation dialog.
  • Wait for the dropdown lock to clear before opening confirmation, and wait for confirmation cleanup before starting the existing optimistic deletion.
  • Remove row-local delete dialogs and an unused legacy favorite-row implementation.
  • Add Playwright coverage for ordinary pages, database pages, and nested subtrees, plus a static ownership guard and a Content changelog entry.

Safety and operations

This changes only client-side dialog ownership and timing. It adds no schema, action, API, permission, credential, migration, or data-retention change. Rollback is the single commit; no persisted data needs reversal.

Verification

  • Focused Vitest: 42 tests passed across the sidebar layout and database sidebar suites.
  • Content typecheck passed.
  • Oxfmt passed; Oxlint completed with pre-existing warnings and no errors.
  • Playwright passed all 3 deletion stories together, asserting restored inline/computed body pointer events and a real unforced survivor click.
  • Independent real-Chrome QA passed ordinary-page, true database-page, and nested-subtree deletion on commit d426302, using UI-only setup and interaction with no refresh, force click, or action shortcut. Screenshots and cleanup proof were inspected.
  • The broad local pnpm test command is not reported as green: under Node 26, the existing use-notion test sees window.localStorage as undefined without a localstorage file, and other suites expect a server on localhost:3000. The focused affected suites and browser acceptance above pass.

Review focus

  • Does waiting for body pointer unlock correctly serialize both dropdown-to-dialog and dialog-to-delete transitions without changing destructive-action semantics?
  • Is the stable sidebar the right ownership boundary across ordinary, pinned/workspace database, and nested rows?
  • Do the browser tests prove interaction restoration while keeping fixture creation and permanent cleanup isolated?

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Here's a visual recap of what changed:

Visual recap

Open the full interactive recap

@builder-io-integration builder-io-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Builder reviewed your changes — looks good ✅

Review Details

Code Review Summary

PR #2536 centralizes Content deletion confirmation in the stable DocumentSidebar instead of allowing ordinary, database, and nested row components to own dialogs. The afterBodyPointerUnlock requestAnimationFrame loop serializes dropdown-to-dialog transitions, while the confirmation ref captures the selected document before dialog cleanup and starts the existing optimistic deletion only after Radix releases its body pointer lock. The three new end-to-end scenarios and the layout ownership guard cover the primary regression surface without changing actions, permissions, persistence, or navigation semantics.

Risk: Standard. Three independent balanced reviews found no confirmed bugs in the dialog lifecycle, callback/state handling, destructive flow, or test cleanup. The implementation also preserves the existing database and nested-row callback paths while removing obsolete row-local dialog code.

🧪 Browser testing: Attempted full verification, but all planner executors were blocked by an environment-wide Chrome MCP/browser-tool outage; no UI evidence was produced. The dev server and /content proxy were healthy, so this is an infrastructure limitation rather than a PR failure. The grounded test plan should be rerun when browser tooling is restored.

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