Skip to content

Added warning when leaving automation editor with unsaved edits#28045

Merged
troyciesco merged 1 commit into
mainfrom
evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2
May 21, 2026
Merged

Added warning when leaving automation editor with unsaved edits#28045
troyciesco merged 1 commit into
mainfrom
evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2

Conversation

@EvanHahn
Copy link
Copy Markdown
Contributor

closes https://linear.app/ghost/issue/NY-1297

Screencast.mp4

Before this change, publishers could leave the automations editor (e.g., by closing the browser tab) when their draft had unsaved changes or when an edit request was active.

Now, they get a browser-level warning message. This is the best we can do to keep them on the page.

(Note that we plan to add in-app confirmation dialogs in other places, such as when you click the in-app back button. This is related but separate.)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

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

Run ID: 685df573-f56d-4a9d-92ea-8003a2e88a9d

📥 Commits

Reviewing files that changed from the base of the PR and between 36fe149 and 6f291c3.

📒 Files selected for processing (5)
  • apps/admin-x-framework/src/hooks.ts
  • apps/admin-x-framework/src/hooks/use-confirm-unload.ts
  • apps/admin-x-framework/src/index.ts
  • apps/admin-x-framework/test/unit/hooks/use-confirm-unload.test.ts
  • apps/posts/src/views/Automations/editor.tsx

Walkthrough

Adds a new React hook useConfirmUnload that conditionally registers a window beforeunload listener (calling event.preventDefault() and setting event.returnValue = ''), cleans it up on dependency change or unmount, re-exports the hook from admin-x-framework entrypoints, adds unit tests covering registration, handler behavior, toggling, and unmount cleanup, and integrates the hook into AutomationEditor using isEditRequestActive || hasUnsavedChanges.

Suggested reviewers

  • troyciesco
🚥 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 accurately and concisely summarizes the main change: adding a browser warning when leaving the automation editor with unsaved edits.
Description check ✅ Passed The description clearly explains the change, provides context about the issue being closed, describes the user-facing behavior, and explains the limitation and future plans.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2

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.

@EvanHahn EvanHahn force-pushed the evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2 branch from 000a808 to 36fe149 Compare May 21, 2026 15:55
@EvanHahn EvanHahn added the ok to merge for me You can merge this on my behalf if you want. label May 21, 2026
@EvanHahn

This comment was marked as outdated.

@EvanHahn EvanHahn requested a review from troyciesco May 21, 2026 15:58
closes https://linear.app/ghost/issue/NY-1297

Before this change, publishers could leave the automations editor (e.g.,
by closing the browser tab) when their draft had unsaved changes or when
an edit request was active.

Now, they get a browser-level warning message. This is the best we can
do to keep them on the page.

(Note that we plan to add in-app confirmation dialogs in other places,
such as when you click the in-app back button. This is related but
separate.)
@EvanHahn EvanHahn force-pushed the evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2 branch from 36fe149 to 6f291c3 Compare May 21, 2026 16:16
Comment thread apps/admin-x-framework/src/hooks/use-confirm-unload.ts
@github-actions
Copy link
Copy Markdown
Contributor

E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 26238439252 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

@troyciesco troyciesco enabled auto-merge (squash) May 21, 2026 18:11
@troyciesco troyciesco merged commit c86259d into main May 21, 2026
69 of 81 checks passed
@troyciesco troyciesco deleted the evanhahn-ny-1297-add-beforeunload-handler-when-1-edit-request-is-active-2 branch May 21, 2026 18:23
@github-actions
Copy link
Copy Markdown
Contributor

E2E Tests Failed

To view the Playwright test report locally, run:

REPORT_DIR=$(mktemp -d) && gh run download 26238439252 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR"

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

Labels

ok to merge for me You can merge this on my behalf if you want.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants