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

show dialog windows in predictable places - at mouse cursor or center #5124

Merged
merged 6 commits into from
Aug 14, 2020

Conversation

johnny-bit
Copy link
Member

we want to show dialog windows in very predictable manner, so in case of multi-screen action or multi-window action, dialogs blocking input won't cause harm to users.

with this change dialogs are shown at mouse cursor if dt gui isn't yet initialised (or is already closed) or at dt window center on top

additionally based on @parafin suggestion - the main window isn't minimalised on closing. I invite anybody to come up with nice closing message though...

This improves solution from #5116 and should be generally better fir for #5110

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor editing. Looks good otherwise, not tested yet.

src/gui/gtk.c Outdated Show resolved Hide resolved
src/gui/gtk.c Outdated Show resolved Hide resolved
src/gui/gtk.c Outdated Show resolved Hide resolved
src/gui/gtk.c Outdated Show resolved Hide resolved
@TurboGit TurboGit added feature: enhancement current features to improve scope: codebase making darktable source code easier to manage scope: UI user interface and interactions labels May 19, 2020
@TurboGit TurboGit added this to the 3.2 milestone May 19, 2020
@TurboGit
Copy link
Member

I invite anybody to come up with nice closing message though...

Yes, this is really needed now. When you ask to quit (Ctrl+Q) there is no visual indication that the shutdown is going on. We may want indeed to have a message or maybe a very dull CSS version to indicate the shutdown ? @Nilvus : what you think on this?

@johnny-bit
Copy link
Member Author

The added class is dt_gui_quit... Hope this allows for nice "fix" ;)

@Nilvus
Copy link
Contributor

Nilvus commented May 20, 2020

We may want indeed to have a message or maybe a very dull CSS version to indicate the shutdown ? @Nilvus : what you think on this?

Why not but is good for me without dialog close window.
But if we add that, we just have to remain it short and simple. I've test this PR but seems the closing dialog is not created by now.

For the text, that could be something like "See you soon with new photos. Bye" for example. Or "Hungry for photos. Waiting for new ones. Bye."

@TurboGit
Copy link
Member

There is no closing dialog at the moment. My idea was to wash out the dt GUI while closing by using the new class added in this PR. Just to show that the GUI is going to be closed.

@johnny-bit
Copy link
Member Author

Honestly I have no idea how to best show closing state without being... hmmm...

Anyway - I thought if it's possible via css to mute colours of interface as for it to look "disabled"... And if there's a way - blur whole interface? However muted interface would be IMO quite enough...

Otherwise, having message would mean... hmmm... having hidden custom view such as knight and on it have closing message? If so - I could create separate PR for that but honestly "closing messages" would be very hard design-wise for me ;)

@Nilvus
Copy link
Contributor

Nilvus commented May 20, 2020

@johnny-bit: the best way for me would be: nothing (as it is actually).

@johnny-bit
Copy link
Member Author

I've been wondering about this PR and further steps or whether to close it off completely.

as @parafin said in #5110 minimizing window on close isn't the best way to handle closing state of application, that's why this PR does away with minimizing window. Having window open on close allows for popping up dialog windows on top of main window that look VERY good. However if we don't have dialog open, having window open for period of closing and cleaning up is a bit... weird. Most aplications I use that have long closing time do either nothing (simply all input/clicking on stuff is ignored) or do the fancy. Fancy things possible:

  1. since I added class to parent window on closing, muting colors is possible, but it requires writting bunch of CSS IMHO
  2. I could create custom view that could be switched to on closing and have things there like closing messages etc. Interesting thing to pursue, however I wonder how long usually darktable closes to be considered "long" since messages there could be tips&trick or other fun stuff
  3. ??? any ideas?

Or simply drop it and "#5116 is good enough"?

@johnny-bit johnny-bit requested a review from TurboGit May 25, 2020 11:12
@Nilvus
Copy link
Contributor

Nilvus commented May 25, 2020

I've been wondering about this PR and further steps or whether to close it off completely.

as @parafin said in #5110 minimizing window on close isn't the best way to handle closing state of application, that's why this PR does away with minimizing window. Having window open on close allows for popping up dialog windows on top of main window that look VERY good. However if we don't have dialog open, having window open for period of closing and cleaning up is a bit... weird. Most aplications I use that have long closing time do either nothing (simply all input/clicking on stuff is ignored) or do the fancy. Fancy things possible:

1. since I added class to parent window on closing, muting colors is possible, but it requires writting bunch of CSS IMHO

Bad idea for me. And which colors? And for what? Just few seconds...

Or simply drop it and "#5116 is good enough"?

For me just the best way. And for 2 simple reasons:

  • As yourself said: "Most aplications I use that have long closing time do either nothing (simply all input/clicking on stuff is ignored)". And I agree with that. On apps I use, I know none of them having fancy things when closing, except just that indeed input/clicking is ignored (and that's the only good way).
  • It's most of the times, just few seconds to close. So really no need to spent more time on that. There's a lot of better things to do on darktable.

@johnny-bit
Copy link
Member Author

OK, so just to be in the clear here. 2 options, nothing fancy:

  1. Don't minimize window on close, just disable/ignore clicks (this would allow to popup exit dialogs on closing dt at center of dt window making them very predictable)
  2. Close this PR and let dialogs open at the cursor location. No window lingering.

@Nilvus
Copy link
Contributor

Nilvus commented May 26, 2020

OK, so just to be in the clear here. 2 options, nothing fancy:

1. Don't minimize window on close, just disable/ignore clicks (this would allow to popup exit dialogs on closing dt at center of dt window making them very predictable)

2. Close this PR and let dialogs open at the cursor location. No window lingering.

Not sure to understand that part but if that PR improve your first merged PR about places of dialog windows, why close it? For me, the only thing is that there's no need to add something when closing darktable (as about all apps, it's good as it is) but taking care of dialog windows places is a good thing. And if it's about maintenance dialog windows, if displayed when opening/closing, just (when needed) be sure that no click on the UI is possible this time.

@johnny-bit
Copy link
Member Author

Hey @TurboGit - If you don't mind, this PR can be merged as-is (or closed).

Currently it works in very nice way (imo):

  1. if there's no UI window, the dialog appears at the mouse location.
  2. if there's ui window, dialog appears in the middle of window on same screen as window.
  3. if darktable is closing, all clicks and events are disabled regardless whether dialog appears or not. this covers @Nilvus's worry that user could click something wrong.

I tried playing a bit with possibilities and upon quit action there's not much possible to do, I couldn't even get switching to my custom empty view happen on quit. With added class it should possible to mute colors, I tried playing with css and either I don't know much or there's something funny going on and having rule

.dt_gui_quit * {
color: @disabled_fg_color;
}

doesn't work... Also gtk inspector worked weird for me since I haven't seen class change on quit despite it working. maybe on quit disables changes to windows? dunno. So ultimatelly either merge or close, nothing to add here ;)

@Nilvus
Copy link
Contributor

Nilvus commented May 31, 2020

@johnny-bit: with CSS you can change colors but not really "mute" them. Or to be more precise, changing color to disabled color would only set all colors to disabled default color but a user could always click on it. Anyway, I do not really worry about user try to click, it's just a possibility. As I said, I find how darktable close by now good.

@hgustafsson
Copy link

The way darktable closes now on OSX (by minimizing the window to the Dock) is very non-standard for mac apps (and the minimization animation is quite obtrusive). See also issue #4218 and PR #5262

@Nilvus
Copy link
Contributor

Nilvus commented Jun 1, 2020

On any OS, minimizing is not a good way.

@johnny-bit
Copy link
Member Author

Oh my... I tried and hide behaviour is waaay nicer than iconify. I tested it in most recent commit and it is VERY nice indeed.

So - one more question: let window 'linger' for a bit, so some dialogs could be shown or hide immediately? I'd rather let it linger for a bit.

@parafin
Copy link
Member

parafin commented Jun 3, 2020

I think there should be some way to tell when darktable finished exiting (meaning all data has been committed to disk and also that darktable can be restarted at that point). Not hiding a window is IMHO the easiest way to achieve this.

@hgustafsson
Copy link

@parafin - On mac this is usually visualized by the icon disappearing from the dock (or the dot beneath the app icon disappears if the app is pinned to the dock). It is standard behaviour that a mac app can still be running while not having any windows open and this is shown by keeping it on the dock.

When closing a window I would like to clear up that screen real estate as soon as possible so that I can continue working. The expected behaviour of clicking a window close button is for the window to disappear, and going against that expectation just makes it very disruptive for the user.

If you feel some visual cue is needed for when committing data to disk upon quitting, a smaller, unobtrusive window with a progress / loading indication or animation might be a good compromise.

@parafin
Copy link
Member

parafin commented Jun 3, 2020

On Mac yeah, and I think it's the same with GNOME, but other systems might not have such indication.

@johnny-bit
Copy link
Member Author

OK, I think I struct here some middle ground and it's probably least intrusive:

  1. window title is changed to "closing darktable..." (needs translators to translate that ;) )
  2. all clicking/input events for main window are blocked
  3. window hiding is postponed till most events that may ask for user input are finished (so dialogs can popup in middle of screen)
  4. window is hiden since it's BEAUTIFUL and quick transition on most uis (minification is meh on gnome and really obtrusive on mac)
  5. cleanup continues till darktable is finally closed.

This allows for best of all worlds - main window lingers for a bit, but title cleary indicates that it's closing and then it's closed nicelly :)

What do you all think?

@hgustafsson
Copy link

Experience on OSX for commit d5083ab:

  • When clicking the close window button (the red x in title bar) it stays pressed until clean up is finished (with window still open and no window title change). Then both the window and the app on the dock closes simultaneously.
    To summarize, it looks like darktable hangs for a while before the window and app closes.

  • When pressing Cmd + Q (i.e. quitting the app without closing the window first), the window title does change to "closing darktable..." before both window and app closes simultaneously after clean up.

Is there some way I can help in creating a small (dialog) window showing the closing message instead while hiding the main window? (I'm new to the darktable code.)

@johnny-bit
Copy link
Member Author

Damn OSX and it's showstopper features ;( I'll try to cope up with yet another way...

@TurboGit
Copy link
Member

I'll reschedule this for 3.4.

@TurboGit TurboGit modified the milestones: 3.2, 3.4 Jun 13, 2020
@github-actions
Copy link

This pull request did not get any activity in the past 30 days and will be closed in 365 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work.

we want to show dialog windows in very predictable manner, so in
case of multi-screen action or multi-window action, dialogs blocking
input won't cause harm to users.

with this change dialogs are shown at mouse cursor if dt gui isn't
yet initialised (or is already closed) or at dt window center on top

additionally based on @parafin suggestion - the main window isn't
minimalised on closing. I invite anybody to come up with nice closing
message though...
also - hide main window after most user input
@johnny-bit
Copy link
Member Author

There's no good way to have nice closing experience on macosx & have window up for long AND not introduce splashscreen (IMO). I opened up FR #5782 for it.

In limited scope of this PR, the current functionality is as good as I can get it to be. nothing more to add/remove IMO, so I'm open to further comments :) and/or merging it :)

@aurelienpierre aurelienpierre self-requested a review August 14, 2020 20:05
Copy link
Member

@aurelienpierre aurelienpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't test on a multi-screen setup, but it doesn't break anything on a single one and the code looks nice.

@aurelienpierre aurelienpierre merged commit 7d14e5f into darktable-org:master Aug 14, 2020
@johnny-bit johnny-bit deleted the dialog_fun branch December 16, 2020 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: enhancement current features to improve no-pr-activity scope: codebase making darktable source code easier to manage scope: UI user interface and interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants