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

Change exit dialog to ask for confirmation to apply previewed background... #33

Merged
merged 2 commits into from
Oct 12, 2014

Conversation

sidequestboy
Copy link
Contributor

Commit 86ea08b addresses #32.
Commit 319421e addresses #34.

This changes the exit dialog from this:
2013-07-20-191705_507x124_scrot
to this:
2013-07-20-231236_456x124_scrot

The Cancel button is redundant with the window delete event, since they both close the dialog and do not exit Nitrogen. I wouldn't be opposed to removing the Cancel button.

The No button continues quitting without applying (resetting background to configuration file setting with Util::restore_saved_bgs()).
The Yes button continues quitting after applying (by calling NWindow::apply_bg()).

This involved decoupling the apply function from the sighandle_click_apply() into a new protected member function apply_bg().

@sidequestboy
Copy link
Contributor Author

On "No" selection, Util::restore_saved_bgs() is now called so that the background settings in the configuration file are applied.

@ghost ghost assigned daf Jul 24, 2013
@daf
Copy link
Member

daf commented Jul 24, 2013

Thanks, this is useful.

I can think of one edge case however: consider when a user has no saved settings and they exit without saving - what happens when you call Util::restore_saved_bgs? Maybe nothing - I think it might just no-op. Been a while since I've been in that portion.

I don't think any code in here affects my large pending refactor (#25), or maybe just trivial merges, so that's alright. I'll merge this soon, especially if you're able to test out the no-saved-bgs scenario.

@sidequestboy
Copy link
Contributor Author

I tested the case:
preview->delete window->"no" to Apply dialog
The previewed image remains on the desktop for the current session, instead of resetting to a blank default or something.

Then a subsequent nitrogen --restore prints Could not get bg groups from config file. to stderr, and does nothing (leaves the previewed image up), or if it's in a startup script on openbox, openbox's default grey background is set.

But, on the initial exit, the previewed background ought to reset itself to some default, and this default should probably be the same as the default from a nitrogen --restore command.

I think the easiest solution would be to have our own background colour defined that we set every time Util::restore_saved_bgs is called and bg-saved.cfg does not exist (Util.cc line 102 on master, or SetBG.cc line 473 on setter_refactor). Light grey would be okay? This would be run typically in a startup script with nitrogen --restore or through the exit dialog.

Though, it would be best if we could actually reset the background to whatever state it was in before launching Nitrogen - this discussion could take place back at #34.

In the meantime, I think that issue is beyond the scope of this feature branch, so I would suggest closing this pull request, and leaving #34 open to fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants