Add option to remember rectangular region until next restart
ClosedPublic

Authored by davidre on Mon, Feb 18, 1:10 PM.

Details

Summary

This adds an option to rememeber the last rectangular region until the next restart. If the user previously had selected the old remember region
checkbox the "remember across restarts will be selected" to preserve the old behavior. Also reorganised the settings page a bit. Something to
think about is changing the "General" settings page to one specific to rectangular region as it contains only options for it.
Old:

New:

BUG: 391299

Diff Detail

Repository
R166 Spectacle
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Mon, Feb 18, 1:10 PM
Restricted Application added a project: Spectacle. · View Herald TranscriptMon, Feb 18, 1:10 PM
Restricted Application added a subscriber: Spectacle. · View Herald Transcript
davidre requested review of this revision.Mon, Feb 18, 1:10 PM
davidre edited the summary of this revision. (Show Details)Mon, Feb 18, 1:14 PM
davidre added reviewers: Spectacle, VDG.
davidre edited the summary of this revision. (Show Details)
ngraham requested changes to this revision.Mon, Feb 18, 4:36 PM
ngraham added a subscriber: ngraham.

Very nice feature! It works well in my testing. Code looks good too. I have only UI suggestions: in addition to the inline comments, I think the layout on the settings page needs work. Let's not abandon the FormLayout style here. I think your addition of a title on the page is good, but then let's keep the FormLayout style for the checkboxes and just change their left label to be "General".

src/Gui/SettingsDialog/GeneralOptionsPage.cpp
59

This string should be "Always"

(and rename the new variables accordingly, too)

60

This string should be "Until Spectacle is closed"

(and rename the new variables accordingly, too)

This revision now requires changes to proceed.Mon, Feb 18, 4:36 PM
davidre updated this revision to Diff 51981.Mon, Feb 18, 5:31 PM

Requested Changes

davidre added a comment.EditedMon, Feb 18, 5:36 PM

Very nice feature! It works well in my testing. Code looks good too. I have only UI suggestions: in addition to the inline comments, I think the layout on the settings page needs work. Let's not abandon the FormLayout style here. I think your addition of a title on the page is good, but then let's keep the FormLayout style for the checkboxes and just change their left label to be "General".

Did you have something like this in mind?

davidre marked 2 inline comments as done.Mon, Feb 18, 5:37 PM

Exactly!

One final request: add an 18px tall spacer between the checkboxes and the comboboxes, like how Dolphin does it: https://cgit.kde.org/dolphin.git/tree/src/settings/general/behaviorsettingspage.cpp#n61

davidre updated this revision to Diff 51992.Mon, Feb 18, 6:55 PM

Add vertical spacer item

ngraham accepted this revision.Mon, Feb 18, 6:58 PM

Fantastic! Looks great, works great.

This revision is now accepted and ready to land.Mon, Feb 18, 6:58 PM
This revision was automatically updated to reflect the committed changes.