Skip to content

Warn when closing tab if there is an unsaved, non-empty draft #7083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robertknight
Copy link
Member

@robertknight robertknight commented May 16, 2025

Copy the useUnsavedChanges hook from the h repository and use it in AnnotationEditor to enable a warning if the user tries to close the tab while there is a non-empty draft. In the process, fix a bug where returnValue was set to an empty string when according to MDN, it must be set to a truthy value. Without this change, the hook works in Chrome but not Firefox.

This change works in Chrome and Firefox, but not Safari. In Safari, the tab closes even when the beforeunload handler is active.

Part of https://github.com/hypothesis/support/issues/59.


Chrome unsaved changes warning

Testing:

  1. In Chrome or Firefox, go to http://localhost:3000
  2. Close the tab, there should be no warning
  3. Go to http://localhost:3000 again, create a new annotation and enter some text or tags but don't save
  4. Try to close the tab, and you should see a warning like the above

Copy link

codecov bot commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.38%. Comparing base (069eba5) to head (341619b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7083   +/-   ##
=======================================
  Coverage   99.38%   99.38%           
=======================================
  Files         277      278    +1     
  Lines       11273    11291   +18     
  Branches     2719     2722    +3     
=======================================
+ Hits        11204    11222   +18     
  Misses         69       69           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy the `useUnsavedChanges` hook from the h repository and use it in
AnnotationEditor to enable a warning if the user tries to close the tab while
there is a non-empty draft. In the process, fix a bug where `returnValue` was
set to an empty string when according to MDN, it must be set to a truthy value.
Without this change, the hook works in Chrome but not Firefox.

This change works in Chrome and Firefox, but not Safari. In Safari, the tab
closes even when the `beforeunload` handler is active.

Part of hypothesis/support#59.
@robertknight robertknight force-pushed the unsaved-draft-warning branch from 28d0099 to 341619b Compare May 16, 2025 12:00
@@ -0,0 +1,55 @@
import { useEffect } from 'preact/hooks';
Copy link
Member Author

Choose a reason for hiding this comment

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

Next week I will look at upstreaming this into the frontend-shared library.

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.

1 participant