search

NAVADMC / ADSM

A simulation of disease spread in livestock populations. Includes detection and containment simulation.
Other
10 stars 5 forks source link

Incorrectly Prompting To Press Apply or No Save message #994

Closed BryanHurst closed 4 years ago

BryanHurst commented 4 years ago

When I was modifying the scenario description, there was no prompt saying "Current form is not saved, press "Apply" to save changes."

However, AFTER pressing "Apply", that message appeared.
Since the Apply button is grayed out at this point, the message is confusing.

Note that after my first press of "Apply", the changes were correctly saved.

BryanHurst commented 4 years ago

@ConradSelig I'm assigning this one to you since I think you were working on this code recently.

If you could try and get this done before the 2020-03-03 meeting, that would be great.

missyschoenbaum commented 4 years ago

I am seeing this in the exact same way @BryanHurst described above. What is supposed to happen? Just no Save message after apply?

I bet this one never gets "dirty" since you don't usually leave the field, since it is one parameter only.

ConradSelig commented 4 years ago

Alight I see three ways going forward:

  1. Fast track; unsaved change messages are not shown when results exist because looking at the scenario creator after running ADSM is intended to act as a review for what settings you had. Unsaved change messages should not be shown because we don't expect users to continue to make changes. Therefore, everything is already working as intended.
  2. Fix the "Bug"; unsaved changes are unsaved changes. When the user makes changes in the scenario creator regardless of if results exist or not a message should be shown.
  3. Make an enhancement; Display a different message when there are unsaved changes in the scenario creator but also when results exist. Something like "Making further changes will delete existing results".

@missyschoenbaum Thoughts? I've done some digging into each option and while option 1 is obviously the easiest, option 2 will otherwise require the least amount of time.

Personal Note: Looks like an "unsaved" class is not being applied to a div with the class "scenario-status" that is preventing both top right messages from being drawn.

ConradSelig commented 4 years ago

Missy's choice on this ticket should also be applied similarly to #954.

ConradSelig commented 4 years ago

@missyschoenbaum Have you made a decision on how you would like to approach this problem?

missyschoenbaum commented 4 years ago

@ConradSelig I think option 1 is acceptable. Thanks for reminding me.

ConradSelig commented 4 years ago

It would likely be a good idea to note this down as a "known bug" before closing.

missyschoenbaum commented 4 years ago

Known issue done. Works fine.