D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

rkflx added a comment. It seems you forgot to merge stable into master, which I now have done with git checkout master git pull git merge -Xours origin/Applications/18.08 git push Thanks again for the quick update so we could get the string changes in in time, and

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. 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). REPOSITORY R166 Spectacle

D13450: Add arrow keys to move and resize selection rectangle

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

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

D13450: Add arrow keys to move and resize selection rectangle

rkflx added a comment. In D13450#294541 , @sharvey wrote: > In D13450#294538 , @rkflx wrote: > > > - Revert unnecessary layout changes. > > > Please clarify... layout for the help text? I don’t

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

rkflx added a comment. 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). It would be great if you could still implement three changes until then: - Move one line

D13450: Add arrow keys to move and resize selection rectangle

rkflx added a comment. Thanks for the updates. Found one more problem, the other changes are fine. In D13450#294047 , @rkflx wrote: > - When the rectangle covers the whole screen, I'm unable to resize, i.e. make the rectangle smaller again.

D13450: Add arrow keys to move and resize selection rectangle

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]

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

ngraham added a comment. 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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

ngraham added a comment. 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. REPOSITORY R166 Spectacle REVISION DETAIL https://phabricator.kde.org/D13450 To: sharvey, rkflx, ngraham, #spectacle,

D13450: Add arrow keys to move and resize selection rectangle

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

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

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.

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. @sharvey Thanks for the updates. As always, first I'd like to iterate to get the patch working properly, and then I'll look at the code in detail. As I'm now put under pressure

D13450: Add arrow keys to move and resize selection rectangle

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Excellent. @rkflx, any last comments or objections, or shall we push this to the 18.08 branch? REPOSITORY R166 Spectacle BRANCH arcpatch-D13450 REVISION DETAIL

D13450: Add arrow keys to move and resize selection rectangle

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

ngraham added inline comments. INLINE COMMENTS > ngraham wrote in index.docbook:154 > Two sentences use the descriptive mood ("Pressing the arrow keys...", > "Holding the Alt key..."), while the middle one uses the imperative mood > ("Hold down the Shift key..."). We should pick one and be

D13450: Add arrow keys to move and resize selection rectangle

ngraham added inline comments. INLINE COMMENTS > index.docbook:154 > + > +You can use the arrow keys to move and adjust > the rectangle. Pressing the arrow keys will slowly move the rectangle. Hold > down the Shift to move in larger increments.

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

ltoscano added a comment. In D13450#293846 , @sharvey wrote: > In D13450#293781 , @ngraham wrote: > > > There's only one remaining thing to do from my perspective: update the docbook to document

D13450: Add arrow keys to move and resize selection rectangle

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

D13450: Add arrow keys to move and resize selection rectangle

yurchor accepted this revision. 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

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