quicktext icon indicating copy to clipboard operation
quicktext copied to clipboard

Feature request: Prompt to save when clicking "close" in settings

Open markwmuller opened this issue 4 years ago • 5 comments

You will lose a new template that you're adding to Quicktext (Tools > Addon options > Quicktext > add template, add text, etc) if you hit "close" without first hitting "save". This is not intuitive, and leads to much frustration (for me, at least, I've lost a bunch of changes and confused myself :) ).

Suggested alternative solutions:

  1. Add a dialogue that appears when clicking "close" if there is unsaved settings edit to confirm user intent
  2. Replace "close" button with "Discard and close" and "save" button with "Save and close" to remove ambiguity.

markwmuller avatar Mar 01 '21 19:03 markwmuller

I support this feature request, and have a mild vote for markwmullers's option #2, but #1 would be fine. You may also want to move the "save [and close]" button from the far left to be next to the "[discard and] close" button. so that the fact that there is a choice is more clear. It is currently easy to overlook the "save", making the user more likely to consider "close" as the only option, like a shorthand for "save and close".

xenotropic avatar Apr 09 '21 00:04 xenotropic

Good Idea. AFAIK there is currently no "something has changed" value. So every change needs to be tracked.

I like to have a "save without close" button as we currently have. (instead of a "save and close" button) So IMO there should be a "save", "save and close" and "discard and close" button.

You may also want to move the "save [and close]" button from the far left to be next to the "[discard and] close" button.

Here the "save" button is on the right and next to the "close" button.

SamuelPlentz avatar Apr 24 '21 09:04 SamuelPlentz

Hey there! Any timeline for this feature request? I can't count how often I clicked "close" instead of "save" an lost my templates (changes) this way... I should have learned by now - but unfortunately that is not the case.

As a supplement to @markwmuller's suggestions, I would like to propose a third variant:

The "Close" button could be omitted without replacement; the "close" button of all common window managers (operating system) already offers this function.

TalusUnheil avatar Sep 13 '22 12:09 TalusUnheil

It is all in the code ready to be used. But apparently there is a linking problem.

The "Your changes has not been saved. Do you want to save them?"-messagebox is implemented here:

https://github.com/jobisoft/quicktext/blob/29627538f6603675bd5cac3ca63b2ac1078a3ae9/chrome/content/settings.js#L76-L115

There are 4 ways to close the window. I had some debugging sessions how they behave (I assume changes were made each time):

  1. In the menu bar click on File > Close:
  • A messagebox Your changes has not been saved. Do you want to save them? pops up and is working as expected.

https://github.com/jobisoft/quicktext/blob/29627538f6603675bd5cac3ca63b2ac1078a3ae9/chrome/content/settings.xhtml#L59

  1. If you press the Escape-key:
  • line 76: it opens the close-funktion
  • line 81: it steps into the block if (this.mChangesMade)
  • line 87: it doesn't show the messagebox itself
  • line 98: the result is 1 in the case
  • line 99: exit the function

https://github.com/jobisoft/quicktext/blob/29627538f6603675bd5cac3ca63b2ac1078a3ae9/chrome/content/settings.xhtml#L34

  1. If you click on the Close-button in the form.
  2. If you click on the X-button in the windows system menu.
  • line 76: it doesn't open the close-funktion

https://github.com/jobisoft/quicktext/blob/29627538f6603675bd5cac3ca63b2ac1078a3ae9/chrome/content/settings.xhtml#L16

@jobisoft Can you have a look?

SamuelPlentz avatar Sep 17 '22 15:09 SamuelPlentz

The "Close" button could be omitted without replacement; the "close" button of all common window managers (operating system) already offers this function.

This should be as easy as changing this: https://github.com/jobisoft/quicktext/blob/29627538f6603675bd5cac3ca63b2ac1078a3ae9/chrome/content/settings.xhtml#L18-L21

To that:

 buttons="extra1" 
 defaultButton="extra1" 
 buttonlabelextra1="&quicktext.save.label;" 

SamuelPlentz avatar Sep 17 '22 16:09 SamuelPlentz