Skip to content

Reset unsaved warning after getSampleData() receives data #606

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

Merged
merged 2 commits into from
Apr 9, 2025

Conversation

jdbocarsly
Copy link
Member

Simple fix to prevent the annoying bug where the unsaved warning erroneously appears whenever a sample or cell page is loaded a second time without a refresh.

closes #601

@jdbocarsly jdbocarsly requested a review from ml-evs February 19, 2024 23:32
Copy link

cypress bot commented Feb 19, 2024

datalab    Run #3074

Run Properties:  status check passed Passed #3074  •  git commit 77eea1c6c5 ℹ️: Merge ce8b2151430e9fc11e8958b4fe3d527cf80cd11b into bb5870109c5d353f2ea0b9542cce...
Project datalab
Branch Review jdb/fix_iss601
Run status status check passed Passed #3074
Run duration 07m 30s
Commit git commit 77eea1c6c5 ℹ️: Merge ce8b2151430e9fc11e8958b4fe3d527cf80cd11b into bb5870109c5d353f2ea0b9542cce...
Committer Josh Bocarsly
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 477
View all changes introduced in this branch ↗︎

@jdbocarsly
Copy link
Member Author

@ml-evs , can you deploy this to dev when you get a chance? Would like to test it on a real server with latency, etc.

@ml-evs
Copy link
Member

ml-evs commented Feb 20, 2024

@ml-evs , can you deploy this to dev when you get a chance? Would like to test it on a real server with latency, etc.

Deployed on dev

Copy link

codecov bot commented May 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.24%. Comparing base (cace4f8) to head (a10c3fd).
Report is 5 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #606   +/-   ##
=======================================
  Coverage   67.24%   67.24%           
=======================================
  Files          62       62           
  Lines        3749     3749           
=======================================
  Hits         2521     2521           
  Misses       1228     1228           

@jdbocarsly
Copy link
Member Author

@ml-evs @be-smith

Ok this is ready for review. The approach I took here is just to reset the warning on reload, which is the simplest approach. Note that we still don't clear the store. This may have pluses and minuses that could be considered for other behavior, but the unsaved warning issue seems to be fixed.

Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.28%. Comparing base (bb58701) to head (ce8b215).
Report is 115 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #606   +/-   ##
=======================================
  Coverage   70.28%   70.28%           
=======================================
  Files          63       63           
  Lines        4129     4129           
=======================================
  Hits         2902     2902           
  Misses       1227     1227           
🚀 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.

@jdbocarsly jdbocarsly added bug Something isn't working webapp For issues/PRs pertaining to the web interface and removed on-hold labels Apr 9, 2025
@jdbocarsly jdbocarsly requested a review from be-smith April 9, 2025 21:53
@ml-evs ml-evs changed the title reset unsaved warning after getSampleData() receives data Reset unsaved warning after getSampleData() receives data Apr 9, 2025
@ml-evs ml-evs merged commit 53a985f into main Apr 9, 2025
17 checks passed
@ml-evs ml-evs deleted the jdb/fix_iss601 branch April 9, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working webapp For issues/PRs pertaining to the web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Unsaved changes" warning appears on loading a sample a second time, after using the browser back button
2 participants