Skip to content
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

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

Open
jdbocarsly opened this issue Feb 19, 2024 · 2 comments · May be fixed by #606
Assignees
Labels
bug Something isn't working webapp For issues/PRs pertaining to the web interface

Comments

@jdbocarsly
Copy link
Member

The "unsaved warning" button and "are you sure you want to leave?" confirmation dialogue should only appear when something has actually been changed. However, in some cases, it shows up before any changes have been made.

Steps to reproduce:

  1. Start on the sample table page
  2. Load a sample or cell
  3. Press the browser "back" button
  4. Load the same sample/cell again.
    -> "Unsaved changes" warning appears.
  5. Try to leave this page
    -> "are you sure you want to leave?" dialogue comes up.

Previous work on this behavior: #223 #257

@jdbocarsly jdbocarsly added bug Something isn't working webapp For issues/PRs pertaining to the web interface labels Feb 19, 2024
@jdbocarsly jdbocarsly self-assigned this Feb 19, 2024
@jdbocarsly
Copy link
Member Author

Looking more into this, the issue seems to be with the watcher in SynthesisInformation.vue.

Normally, the "unsaved changes" warning is automatically set by the computed setters for each field, as defined in field_utils.js. However, this only works for primitive types. The constituent tables use Arrays/Objects, and the computed setters don't properly work for these as they can't detect deep changes (at least as far as I've been able to figure out).

So, we use a deep watcher on constituents to detect any deep changes and set the unsaved warning. In most cases this works fine, but in the above case, a bug arises. The original data is still in the store, and getSampleData() is called again.
The constituents array gets regenerated and gets a new js reference, even though the actual data stays the same. Therefore, the watcher triggers even though the data didn't really change.

@jdbocarsly
Copy link
Member Author

#606 gives a fix that will hopefully fix this by removing the warning right after a page load, but I want to put some other possible solutions here in case this comes up in the future.

  1. In the watcher, explicitely do a deep check that the oldConstituents and newConstituents are actually different in content, not just reference. Downside to this is that the code is a little tricky to write, and it could potentially be slow if the constituent table was ever very large (though if that is the case, the deep watch is probably already slow).
  2. Clear an item's data from the store whenever the page is unloaded.

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 a pull request may close this issue.

1 participant