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

Missing Close / Save button in preference window. #5293

Closed
jade-nl opened this issue Jun 3, 2020 · 23 comments
Closed

Missing Close / Save button in preference window. #5293

jade-nl opened this issue Jun 3, 2020 · 23 comments

Comments

@jade-nl
Copy link

jade-nl commented Jun 3, 2020

Describe the bug
In current master (3.1.0+1891~gac212719e) there is no button option to close the window, in either of the available tabs.

dt 3.0.2 has a close button and master had, for a little while, 2 buttons close without saving and save (or textual something similar).

Pressing escape will close the window, as does using the X.

To Reproduce

  1. Open the preference window
  2. Notice the missing button(s)

Expected behavior
Make sure the user knows what is going to be done (save or discard) by offering buttoned choice(s).

Screenshots
Stable 3.0.2 looks like this:
dt302

Master like this:
dt310

Platform

  • darktable : 3.1.0+1891~gac212719e
  • Graphics card : GeForce RTX 2060 SUPER7979 MiB
  • Graphics driver : nvidia (440.82)
  • OS : Linux (kernel 5.4.0-0.bpo.4-amd64)
  • Distro : Debian Buster (rel. 10.4)
  • Xorg : 7.7+19
  • Gtk : 3.24.5-1

Additional context

  • Reproducible with clean config directory: Yes.
  • Self compiled : Yes.
  • Compilers : gcc 8.3.0 - clang 10.0.1 - LLVM 10.0.1
  • Cmake : 3.13.4
  • Gtk : 3.24.5-1
  • Glib : 2.58.3-2
  • Sqlite : 3.27.2
  • Optimization : None
@johnny-bit
Copy link
Member

I remember the discussion was done in one of @elstoc's PRs - basically some settings are saved without needing to click save so "close without saving" would be meaningless for them.

However I got tripped couple times already so I would be FOR having "close" or any other text button somewhere down there... But anyway - there were much discussion in #4747 #4821 #4825 #4851

So ultimatelly - Needs direction :)

@Nilvus
Copy link
Contributor

Nilvus commented Jun 3, 2020

That I've been long discussed on pointed PR and we over with that choice. All settings are saved when changed so no need to any save button. By now, new preferences could be closed in 2 way: cross button of the window manager bar and ESC shortcut, so no need to close button.

It's a change behavior but a good one as we don't need buttons that are not needed. So no buttons missing, it's the expected behavior.

Just an add to that: If you check on many preferences windows in many apps and many OS, you don't have any close or save buttons. See preferences windows of Mac OS system and Windows one: no buttons. See also preferences in Gnome: no buttons. Firefox: no buttons. And more. Of course some have but I don't see any problem here for those reasons.

@jade-nl
Copy link
Author

jade-nl commented Jun 3, 2020

It's a change behavior but a good one as we don't need buttons that are not needed. So no buttons missing, it's the expected behavior.

Read (most of) the other discussion about this and I do agree with the logic of not having any superfluous buttons. Especially seen from a developers point of view. However.....

Doesn't the average user want some certainty that changes are actually saved? I have to admit that I (re)checked my changes after using ESC (and also tried using X) to make sure it was actually done.

Using a specific button will provide this certainty, another option would be a short duration message if an actual change was made and saved.

I realize that this would be nothing more than user reassurance and strictly speaking doesn't add anything functional to the code (it might even complicate it somewhat). Still, this might be something to consider.

Anyway: I'm going to leave this issue open for a little while to give others a chance to comment. If no button is already a done deal let me know and I'll close it.

@GrahamByrnes
Copy link
Contributor

The only issue I see is that if you type a value into a box, it may not be saved until you exit the box, by clicking elsewhere or hitting return.
It can be a small issue where you have a keyboard that doesn't match the language, ie I need to use commas as decimal markers when entering a float, because my keyboard is azerty although my language is English. So this can lead to numbers with the incorrect symbol being ignored. However that is a wider issue (may also relate to OS, like the non-blinking data entry boxes in lighttable).

@Nilvus
Copy link
Contributor

Nilvus commented Jun 3, 2020

Doesn't the average user want some certainty that changes are actually saved? I have to admit that I (re)checked my changes after using ESC (and also tried using X) to make sure it was actually done.

Most users already doesn't have that certainty on most OS or apps used now. Many modern UI doesn't have anymore buttons on settings/preferences. And that doesn't seem to cause problems or disturb users. When I change settings on my pro computer (under Windows 10), I don't have any message or save button to confirm my change. Change is automatically applied. Same before when I was on Mac OS and now on Gnome (or as I said on Firefox, Thunderbird or many other apps). I know most often don't see any button or message to confirm changes.

When changed on an app, of course it can disturb a little but that's just always the case when a habit is changed, nothing else.

Using a specific button will provide this certainty, another option would be a short duration message if an actual change was made and saved.

I realize that this would be nothing more than user reassurance and strictly speaking doesn't add anything functional to the code (it might even complicate it somewhat). Still, this might be something to consider.

Unuseful reassurance I think for reasons I write. But if something would be add, why not a discrete message (or a short popup explain that changes are automatically applied and some needs restart). About some settings needs to be restart, they could be shown only when reading tooltip (not really visible). On recent issue (can't remember which one), that was discussed to maybe improved how to see which settings needs restart. So this idea and this issue could be merged.

Anyway: I'm going to leave this issue open for a little while to give others a chance to comment. If no button is already a done deal let me know and I'll close it.

For me, no button is already a done deal but we can discuss here about a message popup to advertise user (popup that could be deactivate). And so, change your title.

What I would see if something added (to resume):

Added (when launching preferences window) a popup with message explain how settings are saved (short one) and adding an icon (for example the classic yellow triangle with ! inside) beside all settings that need a restart to be applied and in the popup adding what this icon means to complete the already existing tooltip to keep of course). And add a checkbox on the popup message to disable it for next launch. On this popup, we could also alert the user of the small possible issue pointed by @GrahamByrnes.

@elstoc: as preferences window is your work, your view on that is welcomed.

@elstoc
Copy link
Contributor

elstoc commented Jun 3, 2020

I'm happy enough with the way it works now, but if someone wants to change it to pop up a restart notification when changing those settings that require it I'm fine with that.

@jade-nl
Copy link
Author

jade-nl commented Jun 3, 2020

I'm going to shut up about the buttons option, it being more a general users POV thing than something I really need/want. I can live with the current buttonless implementation.

Added (when launching preferences window) a popup with message explain how settings are saved (short one) and adding an icon (for example the classic yellow triangle with ! inside) beside all settings that need a restart [...]. On this popup, we could also alert the user of the small possible issue pointed by @GrahamByrnes.

What is described by @Nilvus is something that I would like to see implemented. I think that these situations are important enough to warrant these warnings/reminders.

@elstoc
Copy link
Contributor

elstoc commented Jun 3, 2020

Added (when launching preferences window) a popup with message explain how settings are saved (short one)

A popup that appears every time you launch preferences, that you have to click through. I can't say I'm a fan of that, or am I misunderstanding?

@johnny-bit
Copy link
Member

Just get a line of text atop preferences in muted colors and smaller font along the lines of "settings are automatically saved"...

hmmm...

I know! How about a stupid as fox revealer/whatever it's called with 1s timeout atop (or wherever) settings apearing on every setting change (something like toast msg) going:
":floppy_disk: saving setting"

and if there's change in restart-required option, just put atop prefs window:
" ⚠️ restart required to apply settings"

it's just an eye candy, but could be reassuring.

@elstoc
Copy link
Contributor

elstoc commented Jun 3, 2020

settings apearing on every setting change

Most settings are not actually saved until you exit so better would be a generic "saving settings" toast when you exit preferences. We would then not need to care whether the user changed anything.

The restart required toast would be better only when changing a given setting (on top of prefs).

@johnny-bit
Copy link
Member

works for me. the problem is with changes in text fields, eg db maintenance % - if you just change value by entering "0" and click "X" in top corner it won't really be saved... dunno what to do about it.

@jade-nl
Copy link
Author

jade-nl commented Jun 3, 2020

Shouldn't "user error" (entering stuff that is just plain wrong) be followed by an error message? I think this is separate from from the way users are informed about stuff being saved.

@elstoc
Copy link
Contributor

elstoc commented Jun 3, 2020

the problem is with changes in text fields, eg db maintenance % - if you just change value by entering "0" and click "X" in top corner it won't really be saved... dunno what to do about it.

I'm not sure what you mean. Could you be more specific.

@johnny-bit
Copy link
Member

Shouldn't "user error" (entering stuff that is just plain wrong) be followed by an error message

web designer 101 - form validation in real time ;)

I'm not sure what you mean. Could you be more specific.

Open preferences window. go to storage. In "database fragmentation ratio threshold" input any number different than one you have now. click "X" in corner to close preferences window. open preferences window again and go to storage - see that value there is old one.

workaround:
Open preferences window. go to storage. In "database fragmentation ratio threshold" input any number different than one you have now. click on "-/+" buttons or other combo box, or move to different preferences page etc.

@elstoc
Copy link
Contributor

elstoc commented Jun 3, 2020

Might be worth raising a separate issue for that one. Works fine on other fields.

@parafin
Copy link
Member

parafin commented Jun 3, 2020

Just get a line of text atop preferences in muted colors and smaller font along the lines of "settings are automatically saved"...

hmmm...

I know! How about a stupid as fox revealer/whatever it's called with 1s timeout atop (or wherever) settings apearing on every setting change (something like toast msg) going:
" saving setting"

and if there's change in restart-required option, just put atop prefs window:
" restart required to apply settings"

it's just an eye candy, but could be reassuring.

Maybe abuse window title for that message? Just an idea.

@GrahamByrnes
Copy link
Contributor

"💾 saving setting"

Noooo ! :-)

I can see where you're coming from though. My reptile brain now understands that a float field needs to blink and add zeros, and then I know it has done what I want.

type enum are already fine, because the machine has reacted... maybe for the others the field could change colour slightly, or some other unobtrusive way of saying "yes, I heard you".

@jade-nl
Copy link
Author

jade-nl commented Jun 15, 2020

There's a discussion going on on Pixls.us that ties in to this topic (@elstoc being one of the participants).

It seems that people like to see this implemented (idea came from pehar):

[...] Alternatively the warning could appear one-time when the preferences dialog is closed and one (ore more) modified setting(s) require a restart.

Thought it might be helpful to add those opinions in this (stalled?) discussion.

@GrahamByrnes
Copy link
Contributor

Do they refer to changes that need a re-start?
Or to the general issue of a save button?

It should be pretty minor to add a little flag to fields requiring a re-start (he says having no idea how to do that). Maybe start a new issue, since the save-button is maybe putting people off?

@jade-nl
Copy link
Author

jade-nl commented Jun 15, 2020

@GrahamByrnes: About stuff that needs a re-start. Showing it in a way that doesn't keep bugging the user but is clear enough to get the message across.

Initially this was about a save/discard button, but that ship has sailed (see the beginning of this thread).

My remark from a few hours ago (the one about the Pixls.us discussion) is in line with what was discussed here earlier and relates to a re-start message.

@GrahamByrnes
Copy link
Contributor

I think the little-yellow-triangle next to the data field should do it...

@github-actions
Copy link

This issue did not get any activity in the past 30 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

@jade-nl
Copy link
Author

jade-nl commented Jul 23, 2020

Initial issue, the close/save button, will not be implemented/fixed as explained.

Closing this one.

@jade-nl jade-nl closed this as completed Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants