D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Nathaniel Graham]

ngraham abandoned this revision.
ngraham added a comment.


  Abandoning in favor of D21971 .

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: davidhurka, aacid, okular-devel, johnzh, andisa, siddharthmanthan, 
maguirre, fbampaloukas, joaonetto, kezik, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Albert Astals Cid]

aacid added a comment.


  Personally I don't really about the button order, what i care is about the 
shortcuts i've been using for 10 years not changing

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: davidhurka, aacid, okular-devel, maguirre, fbampaloukas, joaonetto, kezik, 
tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[David Hurka]

davidhurka added a comment.


  In D21635#475541 , @ngraham wrote:
  
  > The current keyboard shortcuts aren't a problem if we don't re-arrange the 
items in the Selection Mode toolbar button.
  
  
  Is it important to have a 3-4-5 order in that button menu? The other 3 
shortcuts are not shown in the toolbar, so to learn the shortcuts it is 
neccessary to look into the menu. And there, the 1-2-3-4-5-6 order is a big aid 
to remember them.
  
  Additionally, a 3-4-5 order does not start at 1, which makes it harder to 
recognize as order.
  
  So, I like D21624 , but can live without 
this.
  F6876365: shortcuts.png 

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: davidhurka, aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Nathaniel Graham]

ngraham added a comment.


  In D21635#475539 , @aacid wrote:
  
  > But we're setting the menu here no?
  >
  >   ac->setDefaultShortcut(d->aMouseTextSelect, Qt::CTRL + Qt::Key_3);
  >   
  >
  > Can it just be still be a 4?
  
  
  The current keyboard shortcuts aren't a problem if we don't re-arrange the 
items in the Selection Mode toolbar button. I only re-arranged them so that 
Text Selection could be the default item shown in the button when you first 
open Okular. If there is an alternative method to do that, then we don't have 
to re-arrange the items, and we don't have to change any shortcuts. Does that 
make sense?

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Albert Astals Cid]

aacid added a comment.


  But we're setting the menu here no?
  
  ac->setDefaultShortcut(d->aMouseTextSelect, Qt::CTRL + Qt::Key_3);
  
  Can it just be still be a 4?

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Nathaniel Graham]

ngraham added a comment.


  In D21635#475496 , @aacid wrote:
  
  > *Do not change the shortcuts*
  
  
  In that case perhaps we should revert D21624 
 and then find another way to make the 
toolbar selection menu button show Text Selection by default, which was the 
original goal. Do you have any ideas about that?

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Albert Astals Cid]

aacid requested changes to this revision.
aacid added a comment.
This revision now requires changes to proceed.


  *Do not change the shortcuts*

REPOSITORY
  R223 Okular

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

To: ngraham, #okular, #vdg, ndavis, aacid
Cc: aacid, okular-devel, joaonetto, tfella, ngraham, darcyshen

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Nathaniel Graham]

ngraham added a comment.


  I am a little hesitant to do this since changing shortcuts is always a bit 
controversial and we'd be resetting people's muscle memory for these tools. :/
  
  But I don't see any way around it unless we revert D21624 
 and find a different way to make the 
toolbar button show the Text Selection tool by default that doesn't involve 
re-arranging its order in the pop-up.

REPOSITORY
  R223 Okular

BRANCH
  re-arrange-shortcuts-and-menu-items-accordingly (branched from master)

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

To: ngraham, #okular, #vdg, ndavis
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Noah Davis]

ndavis accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R223 Okular

BRANCH
  re-arrange-shortcuts-and-menu-items-accordingly (branched from master)

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

To: ngraham, #okular, #vdg, ndavis
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid

D21635: Re-arrange selection tool order and shortcuts to reflect new arrangement

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Okular, VDG.
Herald added a project: Okular.
Herald added a subscriber: okular-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  D21624  re-arranged the ordering of the 
selection tools in the toolbar menu. However
  this made it inconsistent with the order in the menu, and now their keyboard
  shortcuts no longer nicely increase.
  
  This patch fixes that. Note that it does involve and necessitate a shortcut 
switch:
  Text Selection is now [Ctrl] + [3], and Area Selection is now [Ctrl] + [4].

TEST PLAN
  rm `~/.config/okularpartrc`
  
  In toolbar button: F6874944: In pop-up button.png 

  
  In Menu: F6874945: In menu.png 

REPOSITORY
  R223 Okular

BRANCH
  re-arrange-shortcuts-and-menu-items-accordingly (branched from master)

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

AFFECTED FILES
  part.rc
  ui/pageview.cpp

To: ngraham, #okular, #vdg
Cc: okular-devel, joaonetto, tfella, ngraham, darcyshen, aacid