D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294820 , @rkflx wrote:
  
  > It seems you forgot to merge stable into master, which I now have done with
  
  
  I was actually in the process of trying to figure that out... I got a lengthy 
list of warnings when I tried it and the wiki is a bit vague on what to do in 
that case. I was hunting through the diff history trying to figure out if it 
was necessary in this case. Regardless, thanks for taking care of it.
  
  > Thanks again for the quick update so we could get the string changes in in 
time, and hopefully for the RC in two weeks you'll be able to solve the 
remaining problems with `checkBounds`.
  
  `checkBounds` seemed so simple, yet it frustrated me endlessly. I'll dig into 
it and see what's going on. I have a few suspicions.
  
  ---
  
  If you have time, would you mind dropping me a line at `sc...@spharvey.me`? I 
have some unrelated questions I'd like to get your opinion on. Thanks in 
advance.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

[spectacle/Applications/18.08] /: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

Git commit 48eacd1ba1495a9167dd9f9cd8097e86ae6ff6dd by Scott Harvey.
Committed on 19/07/2018 at 10:38.
Pushed by sharvey into branch 'Applications/18.08'.

Add arrow keys to move and resize selection rectangle

Summary:
Add arrow keys to move and resize selection rectangle.
{key Arrows} alone moves in large increment
{key Shift} + {key Arrows} moves in small (single pixel) increments
{key ALT} + {key Arrows} resize rectangle in large increment
{key ALT} + {key Shift} + {key Arrows} resize rectangle in small increment

BUG: 394947

Test Plan:
- Apply patch
 - Launch Spectacle; begin Rectangular Selection
 - Select a rectangle with the mouse
 - Use {key Arrows} to move rectangle (large increment)
 - Use {key Shift} + {key Arrows} to move in small increment
 - Use {key ALT} + {key Arrows} to resize rectangle (large increment
 - Use {key ALT} + {key Shift} + {key Arrows} to resize rectangle in small 
increment
 - Complete capture with mouse or {key Enter} key
 - Ensure right selection is captured after moving and resizing rectangle

Reviewers: rkflx, ngraham, #spectacle, yurchor

Reviewed By: rkflx, ngraham, #spectacle, yurchor

Subscribers: ltoscano, kde-doc-english, abalaji, #spectacle

Tags: #documentation

Differential Revision: https://phabricator.kde.org/D13450

M  +5-2doc/index.docbook
M  +202  -3src/QuickEditor/EditorRoot.qml

https://commits.kde.org/spectacle/48eacd1ba1495a9167dd9f9cd8097e86ae6ff6dd

diff --git a/doc/index.docbook b/doc/index.docbook
index 4fcfb40..0331d5f 100644
--- a/doc/index.docbook
+++ b/doc/index.docbook
@@ -29,8 +29,8 @@
 
 
 
-2018-03-06
-Applications 18.04
+2018-07-17
+Applications 18.08
 
 
  is a simple application for capturing desktop 
screenshots. It can capture images of the entire desktop, a single monitor, the 
currently active window, the window currently under the mouse, or a rectangular 
region of the screen. The images can then be printed, sent to other 
applications for manipulation, or quickly be saved as-is.
@@ -150,6 +150,9 @@
 The Rectangular Region 
option allows you to select a rectangular region of your desktop with your 
mouse. This region may be spread across different outputs.
 
 This mode does not immediately take a screenshot 
but allows you to draw a rectangle on your screen, which can be moved and 
resized as needed. Once the desired selection rectangle has been drawn, 
double-clicking anywhere on the screen, or pressing the  button on the 
keyboard will capture the screenshot.
+
+You can use the arrow keys to move and adjust 
the rectangle. Pressing the arrow keys will move the rectangle. Holding the 
Shift key while pressing the arrow keys will move the 
rectangle slowly, for fine-tuning your selection. Holding the 
Alt key while pressing the arrow keys will adjust the size of 
the rectangle.
+
 
 
 
diff --git a/src/QuickEditor/EditorRoot.qml b/src/QuickEditor/EditorRoot.qml
index 583bc37..10560ab 100644
--- a/src/QuickEditor/EditorRoot.qml
+++ b/src/QuickEditor/EditorRoot.qml
@@ -38,6 +38,8 @@ Item {
 property int magZoom: 5;
 property int magPixels: 16;
 property int magOffset: 32;
+property double largeChange: 15;
+property double smallChange: 1 / Screen.devicePixelRatio;
 
 SystemPalette {
 id: systemPalette;
@@ -78,11 +80,202 @@ Item {
 }
 
 Keys.onPressed: {
+
+var change;
+
+// shift key alone = magnifier toggle
 if (event.modifiers & Qt.ShiftModifier) {
 toggleMagnifier = true;
-cropDisplayCanvas.requestPaint();
 }
-}
+
+// nested switches for arrow keys based on modifier keys
+switch(event.modifiers) {
+
+   case Qt.NoModifier:
+   switch (event.key) {
+
+   case Qt.Key_Left:
+change = checkBounds(-largeChange, 0.0, "left");
+selection.x += change;
+break;
+case Qt.Key_Right:
+change = checkBounds(largeChange, 0.0, "right");
+selection.x += change;
+break;
+case Qt.Key_Up:
+change = checkBounds(0.0, -largeChange, "up");
+selection.y += change;
+break;
+case Qt.Key_Down:
+change = checkBounds(0.0, largeChange, "down");
+selection.y += change;
+break;
+}
+
+break; // end no modifier (just arrows)
+
+case Qt.ShiftModifier:
+switch (event.key) {
+
+case Qt.Key_Left:
+ change = checkBounds(-smallChange, 0.0, "left");
+ 

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

This revision was automatically updated to reflect the committed changes.
Closed by commit R166:48eacd1ba149: Add arrow keys to move and resize selection 
rectangle (authored by sharvey).

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=38082=38083

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294808 , @rkflx wrote:
  
  > Thanks, looking much better now.
  >
  > Please make sure to commit to the stable branch, i.e. `Applications/18.08` 
(let me know if you are unsure what that means).
  
  
  Following the Arcanist/Phabricator docs on the KDE wiki...

REPOSITORY
  R166 Spectacle

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey updated this revision to Diff 38082.
sharvey added a comment.


  - Minor whitespace cleanup

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=38080=38082

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey marked 2 inline comments as done.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey updated this revision to Diff 38080.
sharvey added a comment.


  - Fix Alt + Down; Undo layout changes for help text; Reorder help text items

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=38014=38080

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294538 , @rkflx wrote:
  
  > Tagging is on Thursday at 23:59 UTC. I will accept the patch tomorrow so 
you can still land it in time (let me know if you are busy and I should commit 
on your behalf).
  
  
  That’ll be 6:59pm in my world. I’ll see what I can accomplish in the next 24 
hours.
  
  > - Revert unnecessary layout changes.
  
  Please clarify... layout for the help text? I don’t want to waste time.
  
  > The rest can be improved upon later.
  
  If it’s not ready (E.g. the edge-touching issue), then it’s not ready. I 
don’t want to see anything released (even as a beta) that’s got bugs. 
Especially known bugs that we just ran out of time to fix. If it has to wait, 
it can wait. I don’t want to be responsible for any bending or breaking of 
rules, and I certainly don’t want to release dodgy software.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294304 , @ngraham wrote:
  
  > Yeah, the magnifier shows up while dragging the selection rect (or not, 
depending on whether or not you've holding down [⇧]), but it currently doesn't 
show up when you're using [Alt] to resize the rect using the keyboard arrow 
keys.
  >
  > IMHO, if you have the magnifier turned on in the settings, it should show 
up when you're in "fine tune" mode (holding down both [⇧] and [Alt] and using 
the arrow keys to resize the rect by single pixels), because conceptually, if 
you're doing this, it's because you want a pixel-perfect rect.
  
  
  Let's get @rkflx 's input on this. I'm not sure how easily it will be to 
implement. There's a deadline hanging over our heads.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294047 , @rkflx wrote:
  
  > I still disagree regarding the default speed selection. We determined by 
looking at other apps that [⇧] is the modifier to use, and I argued (in line 
with what I meant when triaging the bug) that for making the rectangle feature 
useful for keyboard users by default the movement should be fast. BTW, this is 
also what KWin is doing, and I see no reason at all why Spectacle should 
deviate from that standard. (See above for even more arguments.) I don't feel 
comfortable approving the current default, sorry.
  
  
  Fixed. The default movement - [Arrows] alone - is `largeChange`, and [⇧] + 
[Arrows] is a single pixel move.
  
  > To summarize, here's what I propose: If we can reach agreement on the 
default and you are fast with the string changes still required, the patch can 
go in for the Beta. Then you have time to work on the other points until the RC 
(at which point we have to decide whether to ship or to revert again, based on 
your progress)
  
  The help text box has been reformatted to match the prior layout. `Esc` has 
been moved to the bottom of the list. Also, the docbook entry for this patch 
has been rewritten and is hopefully clearer and more consistent.
  
  ---
  
  You suggested multiple changes to the `checkBounds` function, all of which I 
understand and agree with. This function caused me the most heartburn and I 
feel uncomfortable attempting to rework it while under a time crunch. 
Unfortunately, this leaves the `0x0px` resize and the "cannot resize when 
expanded to full screen` bugs unresolved.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294291 , @ngraham wrote:
  
  > I'm noticing that the magnifier doesn't er show up while you're using the 
keyboard to resize the selection rect. Might wanna hook that up.
  
  
  It works for me when holding [⇧] and dragging a new rectangle. I'm a little 
fuzzy on how the magnifier is intended to work.
  
  We've already commandeers [⇧] for small adjustments, so that's likely 
overriding the magnifier.
  
  How do we think this should work?

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey marked 7 inline comments as done.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey edited the summary of this revision.
sharvey edited the test plan for this revision.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey marked 9 inline comments as done.
sharvey added inline comments.

INLINE COMMENTS

> rkflx wrote in index.docbook:154
> "Holding the Shift while": Either remove "the", or add "key".
> 
> "using the arrows": Change to "Using the arrow keys"?

Typos and/error sloppy grammar. Rewritten.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey updated this revision to Diff 38014.
sharvey marked an inline comment as done.
sharvey added a comment.


  - Change default adjustment to `largeChange`, [⇧] uses `smallChange` for fine 
tuning
  - Numerous miscellaneous changes per reviewer comments, including:
- Condensed adjustment calculations to use `+=`
- Reverted help text box to match original layout
- Revised docbook entry for clarity and consistency

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=37965=38014

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added inline comments.

INLINE COMMENTS

> rkflx wrote in EditorRoot.qml:443-507
> Currently I don't understand what all those changes are for and how they 
> relate to the topic of the patch (and I would disagree with some).
> 
> Contrary to @ngraham I'd say this actually is a visual regression.

These are remnants (which will be reverted) from when the help text box was two 
columns wide.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added inline comments.

INLINE COMMENTS

> rkflx wrote in EditorRoot.qml:41
> That does not seem right, because this will result in slower movement on a 
> HiDPI screen compared to a regular screen of the same size. (It's needed for 
> the 1px case, of course.)

This was suggested by @abalaji based on his work on reworking the selection 
rectangle. Should it be removed?

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#294047 , @rkflx wrote:
  
  > - Selecting with the mouse sets a minimum size, however resizing with the 
arrow keys allows to resize to 0x0px.
  
  
  Where should it stop? 1x1?

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  I will attempt to get these completed early tomorrow morning (my time). It's 
already the end of the day here and I know (from unfortunate experience) that I 
don't do my best work "after hours".

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey marked 2 inline comments as done.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#293854 , @ltoscano wrote:
  
  > About the date: that's not the release date, but the date where the 
document was last updated.
  
  
  Thanks. I corrected it to today's date.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey updated this revision to Diff 37964.
sharvey added a comment.


  - Correct revision date in docbook

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=37963=37964

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: ltoscano, kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey added a comment.


  In D13450#293781 , @ngraham wrote:
  
  > There's only one remaining thing to do from my perspective: update the 
docbook to document this nice new feature! It would be great if you could do 
that in this patch. Keep in mind that 18.08 has already been branched and the 
freeze is in two days, so if you'd like to get this in for the next release, 
you'll have to work quickly. :)
  
  
  Arrow keys information added. I checked the `.docbook` file with the doc 
tools and it parsed properly and generated correct HTML files.
  
  I changed the release tag in the docbook file to 18.08 and set the date to 
2018-08-09 based on the `Applications Tagging` date on the Application Release 
Schedule 
 page. 
That was an educated guess. If it should be something else, just let me know 
and I'll change it.

REPOSITORY
  R166 Spectacle

REVISION DETAIL
  https://phabricator.kde.org/D13450

To: sharvey, rkflx, ngraham, #spectacle, yurchor
Cc: kde-doc-english, abalaji, #spectacle, skadinna

D13450: Add arrow keys to move and resize selection rectangle

[Scott Harvey]

sharvey updated this revision to Diff 37963.
sharvey added a comment.
Restricted Application added a project: Documentation.
Restricted Application added a subscriber: kde-doc-english.


  - Edit docbook to include arrow key usage; update release number and date

REPOSITORY
  R166 Spectacle

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13450?vs=37944=37963

BRANCH
  arcpatch-D13450

REVISION DETAIL
  https://phabricator.kde.org/D13450

AFFECTED FILES
  doc/index.docbook
  src/QuickEditor/EditorRoot.qml

To: sharvey, rkflx, ngraham, #spectacle
Cc: kde-doc-english, abalaji, #spectacle, skadinna