Add option to remember rectangular region until next restart
ClosedPublic

Authored by davidre on Feb 18 2019, 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
Branch
region
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8493
Build 8511: arc lint + arc unit
davidre created this revision.Feb 18 2019, 1:10 PM
Restricted Application added a project: Spectacle. · View Herald TranscriptFeb 18 2019, 1:10 PM
Restricted Application added a subscriber: Spectacle. · View Herald Transcript
davidre requested review of this revision.Feb 18 2019, 1:10 PM
davidre edited the summary of this revision. (Show Details)Feb 18 2019, 1:14 PM
davidre added reviewers: Spectacle, VDG.
davidre edited the summary of this revision. (Show Details)
ngraham requested changes to this revision.Feb 18 2019, 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.Feb 18 2019, 4:36 PM
davidre updated this revision to Diff 51981.Feb 18 2019, 5:31 PM

Requested Changes

davidre added a comment.EditedFeb 18 2019, 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.Feb 18 2019, 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.Feb 18 2019, 6:55 PM

Add vertical spacer item

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

Fantastic! Looks great, works great.

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