#501 - Prompt user to save/discard/cancel unsaved changes when switching from one talk to another in Talk Editor

[benbuckley]

Ultimately, I would like to have it set up so that, when the user has unsaved changes in a talk and clicks on a different talk from the tableview, a window comes up asking them if they want to save their changes, with the options "Save" (save changes and select the new talk), "Discard" (don't save changes, just select the new talk) and "Cancel" (don't save changes but don't select the new talk either).

I have not made any huge changes yet -- a lot of my time has been spent just understanding Qt and how all the methods interact with each other. I expect to make more progress this week. However, to start with, I have added a method called "click_talk" which mediates between tableView and talk_selected. I know it doesn't look very useful right now, but I believe that it will make things easier in the long run to have a method that acts only when the user clicks on the talk -- as it is, talk_selected is called both when the user clicks on a talk, and in the select_talk method.

ToDo:

• [x] Add QMessageBox with save/discard/cancel options
• [x] Add functionality to this QMessageBox (Will attach screenshot soon)
• [x] Ensure that NEW talk is selected after user selects "Save"
• [x] Change options in QMessageBox to "Yes", "No" and "Cancel"
• [x] Enable user to save changes to current talk before creating new talk (Perhaps this should be made a separate issue -- thoughts?
• [x] Prompt user to Continue Editing, Discard Changes, or Save Changes when closing Talk Window as well.

Screenshot of QMessageBox:

Closes Issue #501 when done.

[benbuckley]

The edits just barely do as promised -- when the user selects a new talk when the current one has unsaved changes, a window comes up with the options "save/discard/cancel" where each option essentially does what it says.

However, I am not done yet. For one thing, I would like to see if I can make my code a bit prettier. For another thing, I would like to see if I can make it so that, when the user creates a new talk (which by default becomes highlighted on the list) they have the option of saving the details on their current talk.

In addition: currently, if you select "Save" from the save/discard/cancel window, your current talk remains selected and highlighted. I would like for the NEW talk to be selected and highlighted after the changes are made.

Also, by default, the "Discard" button says "Close without saving" which is a little odd. Maybe I should change it to yes/no/cancel instead of save/discard/cancel?

[dideler]

To make it easier for us to see what you did and what you plan to do, please create a task list in the PR description. Keep the items in the list concise, so they're easy for others to read.

At the end of your PR description, please reference the related issue (e.g. Closes #...).

Since this PR deals with UI changes, please show us screenshots of the UI whenever you make significant changes. See the screenshot below for how to add images.

I'll take a look at the code and play around with the changes later today, so I can give more feedback then.

[benbuckley]

This looks fine to me. Are there any further changes I should make? Also, in general, should I squash it all into one big commit?

[zxiiro]

Sorry it's not clear to me what the difference between No and Cancel are?

Edit: To clarify my confusion, what's the purpose of selecting a new talk if we are discarding it vs not selecting it if we cancel?

[benbuckley]

zxiiro: Suppose you have made changes to a talk, and then click on another talk from the list.

If you select "No", then your unsaved changes are lost, and the TalkDetailsWidget is repopulated with the details of the new talk. It's a way for the user to confirm "That's right, I don't want to keep the changes I made."

If you select Cancel, then your unsaved changes stay in the TalkDetailsWidget, but they are not saved -- it's as though you never clicked on the new talk to begin with. It's a way for the user to say "On second thought, I'm not sure if I want to save or discard my changes yet, and I need to think about this some more."

mtomwing, I have noted your comments and will address them in due time.

[zxiiro]

@benbuckley makes sense. thanks for the clarification.

[benbuckley]

I've resorted the import statements, and put the QMessageBox stuff on one line. How does it look now?

[mtomwing]

@dideler or @zxiiro, take a look at this when you have a chance since you're more familiar with the talk editor. Should be good to merge if you don't have any objections.

[dideler]

@benbuckley not sure why you changed the button text? See https://github.com/Freeseer/freeseer/pull/610#issuecomment-58211749

Regarding the three options, I think this overcomplicates the prompt. It should just be "Save changes" (default option) and "Discard changes".

After the action is completed in both cases, the user would be brought to the other talk they selected—that was their intention after all.

If they misclicked (the odds are low, but possible), then they still don't need a cancel option. Why? Because they can choose the save option and go back to the previously selected talk.

[benbuckley]

Re: Avoiding "Yes" and "No" -- I originally used QMessageBox.Save and QMessageBox.Discard. When I ran it this way, the Discard button would default to the text "Close Without Saving", which seemed misleading, since it doesn't close the whole program, it just switches from one talk to another. I figured maybe it showed up differently on different computers, and I reasoned that if I just went with "Yes" and "No", it would be more consistent from one machine to another. However, I'm not married to the idea, and I can change it back to Discard and Save.

I would like to defend the "Cancel" option, though.

We have accepted the premise that it's possible for a user to misclick -- there are a variety of reasons why this might happen (maybe their elbow slipped, maybe they're just very indecisive). Suppose we only have "Save" and "Discard" options. Then if the user innocently misclicks, we're forcing them to either lose all their changes, or commit to all their changes.

I can think of plenty of times I've made big changes to a file and been unsure whether I wanted to keep them that way or not. Being forced to either Save or Discard them would put me in a bind. If I Save, I can't go back to how the file was before I made my changes if my changes turn out to be bad. If I Discard, then I lose all my changes and HAVE to start over. With the "Cancel" button, the user still has the option of saving or discarding their changes later if they wish.

[dideler]

I'm not sure I understand your argument for keeping the cancel option.

We have accepted the premise that it's possible for a user to misclick

That's right, but the odds are low and the mistake is easy to recover from with save and discard options. The extra complexity from having more options actually complicates the decision the user has to make. Not to mention that "Cancel" and "No/Discard" are similar enough (both can be thought of as negatives) to make the process even more confusing.

If I Save, I can't go back to how the file was before I made my changes if my changes turn out to be bad.

I don't follow. Why can't you go back? If you weren't done editing the talk but you accidentally clicked another talk, then you save changes and go back to the talk and continue from where you left off.

If I Discard, then I lose all my changes and HAVE to start over.

Start over what process exactly? Either you want the changes you made (or some variation of them) or you don't want them at all. In the former case, you save and go back to the talk. In the latter case you discard changes and focus on the new talk you selected.

[dideler]

FWIW, I just performed a guerilla user test using this sketch:

I described the top window (i.e. talk editor UI - it looks nothing like Freeseer's, but it doesn't need to) and described the situation: You're editing a talk and then select a new talk before saving your changes. You're then presented with a prompt.

Both prompts were initially hidden, I then revealed the first prompt and asked for their thoughts.

The response was something along the lines of "If I say yes, is it going to save to the new talk? If I say no, is it going to give me another option to save to the old talk or do I lose everything? And if I press cancel, is it going to not save anything or go back to the old talk?"

So there was some confusion. (Note that I didn't give any hints or mention my own opinion.)

I then revealed the second prompt and again asked for their opinion.

The response was something like "It's a clear question about what I forgot to do and clear where it will save the changes to."

I then asked about the situation where you accidentally misclicked a talk. The response was something like "I would rarely do that. But if I did, I would save changes and go back. But with the other one it's unclear what I have to click to go back. Do I click no or cancel?"

Now this was just a sample size of one, which is statistically insignificant, but you get the idea.

[benbuckley]

My problem is that saving is not a neutral decision. Saving a set of bad changes is as bad as discarding a set of good changes, and I wouldn't want to do either one of them by accident, or by force.

Suppose the options are Save and Discard. If I'm in the middle of doing some major rewrites on a description, I might not know yet whether I want to keep my changes or not. If I already knew with complete certainty that I wanted to keep my changes, I would have clicked "Save Talk" already. If I already knew that I wanted to get rid of the changes, I would have exited the talk editor or discarded the changes some other way. When I'm still in the process of editing, I'm in a state of uncertainty.

If you want to reduce the number of buttons, how about something like this? Feel free to suggest other ways to word the query:

In the likely situation that the person intended to click on the other talk, then it's obvious which button to press if they want to discard the changes to the current talk. If they clicked by accident, or if they just forgot to save their current talk, then they can cancel and continue editing their current talk or save it if they wish. Admittedly the word "Cancel" is still a tad ambiguous, but it's at least obvious that it's not the same thing as "Discard".

[zxiiro]

I like Dennis' suggestion because his window informs the user what is happening "The talk you were editing has unsaved changes" and then provides 2 possible suggestions on what can be done. I feel its' not ambiguous and is in a language natural to an end user.

Simply stating "Discard changes?" is a bit ambiguous to me, what changes are being discarded? ... Settings? Talk Data?

[dideler]

The "Discard changes?" is nudging users towards discarding changes, which might be improbable assuming they want their changes saved most of the time. This is why I think it's best to have a neutral message and have the buttons contain the actions (which also has the benefit of reduced mistakes since users sometimes skip the message and only read the buttons).

After thinking about it some more, I think this is a fair compromise

+-----------------------------------------------------+
| The talk you were editing has unsaved changes.      |
|                                                     |
| [Continue editing] [Discard changes] [Save changes] |
+-----------------------------------------------------+

It has all three actions. We assume saving is the most common action, so it's primary. Cancelling navigation is the least likely (only happens on misclicks) so it's on the far left.

Other wording choices for "Continue editing"

• "Cancel navigation"
  • Note that just "Cancel" would not be sufficient since it would lead to confusion. The message is about unsaved changes, but the Cancel function cancels navigation.
• "Go back" or "Go back to editing talk"
• "Stay" or "Stay on talk"

P.S. I think autosave would be best from a UX perspective, but I think we experimented with that before and there was some issue, can't remember what.

[benbuckley]

I like it. I'll implement it as soon as possible, which will probably mean after Thanksgiving.

It is a little tricky to customize the button text (up until now, I've just been using the defaults, i.e. "Save", "Yes", "No", "Cancel", "Discard"), but I think I have an idea of how to do it.

[dideler]

@benbuckley there was a PR by @farazs recently that had customized text, maybe that'll help. Search for "reset" https://github.com/Freeseer/freeseer/pull/610/files

[dideler]

@benbuckley please ping us on the PR when you respond to feedback / push new commits. Just say something like "fixed" or "please review" so we get a notification. Otherwise the PR will be ignored since we don't know that there's been activity.

[dideler]

Found a bug. If you try to exit the talk editor with unsaved changes and select "Discard changes", it actually saves your changes.

[benbuckley]

I have made most of the changes you suggested. However, for some reason, using setModal(True) and show() seems to give strange results for me -- the buttons in the message box don't always do the things they're suppose to do.

When you say "ping", does that mean leaving a comment (possibly including a tag like @dideler)?

[dideler]

I have made most of the changes you suggested. However, for some reason, using setModal(True) and show() seems to give strange results for me -- the buttons in the message box don't always do the things they're suppose to do.

Okay thanks. Let's keep it like how you had it originally.

When you say "ping", does that mean leaving a comment (possibly including a tag like @dideler)?

Yup. That way we get notified and can take another look at the PR.

If you squash your commits into a single commit, we can get this merged.

After that, will you be working on #532, #541, or tests for #605?

[benbuckley]

@dideler I've squashed the commits. I think moving on to #532 would be a good idea, since I have TalkEditor.py fresh in my mind.

EDIT: Actually, now that you mention it, I've been thinking I should learn a bit about how unit tests work, and this seems like a good opportunity to learn how. I'm interested in doing tests for #605

[dideler]

@benbuckley I noticed the commit message contains outdated information. Can you update it?

[benbuckley]

@dideler The old commit message still said the options were "Yes", "No" and "Cancel". I have changed it to reflect the new buttons, "Save Changes", "Discard Changes" and "Continue Editing". Is that what you were talking about?

[dideler]

Looks good :+1:

Thanks for your contribution!