Skip to content

Warn users before leaving playground with unsaved changes#456

Open
shubhamjrd4559-sudo wants to merge 1 commit into
piyushdotcomm:mainfrom
shubhamjrd4559-sudo:codex/unsaved-changes-warning
Open

Warn users before leaving playground with unsaved changes#456
shubhamjrd4559-sudo wants to merge 1 commit into
piyushdotcomm:mainfrom
shubhamjrd4559-sudo:codex/unsaved-changes-warning

Conversation

@shubhamjrd4559-sudo

@shubhamjrd4559-sudo shubhamjrd4559-sudo commented Jun 3, 2026

Copy link
Copy Markdown

Summary

  • Add an unsaved-changes warning hook for Playground sessions
  • Prompt users before browser refresh/close and guard in-app navigation when open files contain unsaved changes
  • Protect the Dashboard back button with the same confirmation flow
  • Add unit tests covering clean, dirty, cancelled, and confirmed navigation scenarios

Why

Closes #427.

Users can currently leave the Playground with unsaved edits and lose their work without any warning. This change reuses the existing hasUnsavedChanges state from open Playground files to display warnings only when there is actual unsaved editor content.

Validation

  • npm.cmd run lint
  • npm.cmd test
  • npm.cmd run build

Maintainer Notes

This PR is submitted for review and merge into main. I do not intend to merge it directly; please review and merge if it aligns with the project's standards and requirements.

Summary by CodeRabbit

  • New Features

    • Adds unsaved-changes detection in the playground that prompts before navigating away; header's back action now respects this confirmation.
  • Tests

    • Adds tests covering browser unload prevention, in-page navigation interception, and the navigation confirmation helper.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@github-actions github-actions Bot added the enhancement New feature or request label Jun 3, 2026
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

👋 Thanks for opening a PR, @shubhamjrd4559-sudo!

Your PR has entered the 🚦 PR Review Pipeline.

Standard PR detected — your PR will follow the standard review pipeline.


What happens next

Stage Reviewer Checks
Stage 1 — Automated Validation 🤖 Bot DCO · Format · AI/Slop · Duplicate
Stage 2 — Human Review 👥 Maintainer Code + Quality Review
Stage 3 — PA / Maintainer Review 🔑 Project Admin Final Merge Decision

A pipeline status comment will appear below and update automatically as your PR progresses.


While you wait

  • Sign all commits (git commit -s)
  • Link your issue (Closes #123)
  • Use a feature branch (not main)
  • Avoid unrelated changes

This comment is posted only once.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 605457c8-f5c9-4357-b146-5ea6ae8e6628

📥 Commits

Reviewing files that changed from the base of the PR and between 2e6504e and 00927bc.

📒 Files selected for processing (4)
  • app/playground/[id]/page.tsx
  • modules/playground/components/playground-header.tsx
  • modules/playground/hooks/useUnsavedChangesWarning.test.ts
  • modules/playground/hooks/useUnsavedChangesWarning.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • modules/playground/hooks/useUnsavedChangesWarning.ts

Walkthrough

Adds a client-side hook that blocks browser unload and intercepts in-page link clicks when editors have unsaved changes; the hook is used on the playground page and its confirmNavigation callback is passed to the header to guard the “Back to Dashboard” action.

Changes

Unsaved Changes Protection

Layer / File(s) Summary
Hook implementation and tests
modules/playground/hooks/useUnsavedChangesWarning.ts, modules/playground/hooks/useUnsavedChangesWarning.test.ts
useUnsavedChangesWarning(hasUnsavedChanges) returns confirmNavigation. When enabled, it registers a beforeunload handler and captures in-page anchor clicks to prompt via window.confirm. Tests verify unload blocking, click interception, and the imperative helper's return values.
Playground page wiring
app/playground/[id]/page.tsx
Imports the hook, computes hasUnsavedChanges from openFiles, calls the hook, and passes confirmNavigation into PlaygroundHeader.
Header back-button confirmation
modules/playground/components/playground-header.tsx
PlaygroundHeader accepts confirmNavigation?: () => boolean and invokes it before navigating to /dashboard; navigation uses router.push and is cancelled if the callback returns false.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • piyushdotcomm/Editron#219: Refactors PlaygroundHeader props/state; overlaps with this PR's header prop addition and back-button behavior.

Suggested labels

help wanted, mentor:piyushdotcomm, quality:exceptional

Suggested reviewers

  • piyushdotcomm

Poem

🐰 I hop and guard your edits bright,
Unsaved bits I hold till night,
A confirm nudges your wandering feet,
Stay or go — choose safe and sweet,
Hooray for saves and code kept right!

🚥 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 'Warn users before leaving playground with unsaved changes' clearly and concisely summarizes the main feature addition—implementing unsaved-changes protection with appropriate navigation warnings.
Description check ✅ Passed The PR description covers all required template sections: Summary explains what and why changed, Type of change indicates new feature, Related issue references #427, Validation lists executed commands, and Checklist items are addressed.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #427 objectives: implements unsaved-changes detection via the new hook, adds beforeunload handler, intercepts in-app navigation with confirmation, and includes comprehensive unit tests covering clean, dirty, cancelled, and confirmed navigation scenarios.
Out of Scope Changes check ✅ Passed All code changes directly support the stated objective of warning users about unsaved changes. The hook implementation, header integration, and test suite are all focused and necessary for the feature—no unrelated modifications detected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@modules/playground/components/playground-header.tsx`:
- Around line 66-69: The current onClick handler calls confirmNavigation() and
then window.location.href = '/dashboard', which causes the beforeunload handler
from useUnsavedChangesWarning to trigger the native browser dialog and produce a
double prompt; fix this by importing Next's useRouter, call const router =
useRouter() inside the component, and replace window.location.href =
'/dashboard' with router.push('/dashboard') (or router.replace('/dashboard') if
you prefer replacing history) so navigation uses the Next router and avoids the
full unload/native prompt while still honoring confirmNavigation().
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0deda88f-c7c3-4e17-b093-dfa46aadcb6e

📥 Commits

Reviewing files that changed from the base of the PR and between f12b223 and 2e6504e.

📒 Files selected for processing (4)
  • app/playground/[id]/page.tsx
  • modules/playground/components/playground-header.tsx
  • modules/playground/hooks/useUnsavedChangesWarning.test.ts
  • modules/playground/hooks/useUnsavedChangesWarning.ts

Comment thread modules/playground/components/playground-header.tsx
@shubhamjrd4559-sudo shubhamjrd4559-sudo force-pushed the codex/unsaved-changes-warning branch from 2e6504e to 00927bc Compare June 3, 2026 06:59
@shubhamjrd4559-sudo

Copy link
Copy Markdown
Author

Hi @piyushdotcomm,

Just a friendly follow-up on PR #456. Whenever you have time, could you please review the PR and approve the pending workflows if everything looks good?

Thank you for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

# Feature: Warn Users About Unsaved Changes Before Leaving the Playground

1 participant