Use F11 as the shortcut to toggle the aside preview
ClosedPublic

Authored by ngraham on Feb 5 2018, 2:01 AM.

Details

Summary

FEATURE: 389880
FIXED-IN: 5.44

Test Plan

Shortcut works and toggles the aside preview.

Diff Detail

Repository
R241 KIO
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham created this revision.Feb 5 2018, 2:01 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 5 2018, 2:01 AM
ngraham requested review of this revision.Feb 5 2018, 2:01 AM
ngraham edited the test plan for this revision. (Show Details)Feb 5 2018, 2:02 AM
markg added a subscriber: markg.Feb 5 2018, 12:32 PM

+1 for the idea, just a question about the key.
Windows uses ALT+P: https://lifehacker.com/5811238/quickly-show-the-preview-pane-in-windows-7-with-alt-%252B-p
I don't know what Finder uses, you can probably tell ;)

As it stands now, i'd go for ALT+P. It also feels more logical due to the "P" in there for "Preview", which is exactly what it's going to show when pressed.

I use F11 to be consistent with Dolphin, where F11 toggles the information panel. It's not exactly the same thing, but I figured re-using existing KDE muscle memory was worthwhile. Alt-P could work too though, and would probably conflict less. Easier to type on a laptop with shared function keys, too.

Any ideas on the "ambiguous shortcut" issue?

markg accepted this revision.EditedFeb 5 2018, 2:20 PM

I use F11 to be consistent with Dolphin, where F11 toggles the information panel. It's not exactly the same thing, but I figured re-using existing KDE muscle memory was worthwhile. Alt-P could work too though, and would probably conflict less. Easier to type on a laptop with shared function keys, too.

Any ideas on the "ambiguous shortcut" issue?

Ah right!
Oke, i guess now is the moment to choose.

  1. Consistency with Dolphin (go for F11)
  2. Consistency with Windows Explorer (go for ALT + P and change the dolphin one default to ALT + P)

I'm fine with both options as long as there is consistency.
Accepting this one just in case you wont to go for option 1 :)

I have no clue for the ambiguous shortcut issue.

This revision is now accepted and ready to land.Feb 5 2018, 2:20 PM
ngraham updated this revision to Diff 26584.Feb 5 2018, 2:20 PM
ngraham removed a reviewer: markg.

Fix "ambiguous shortcut" issue by using plain old QKey instead of QKeySequence

This revision now requires review to proceed.Feb 5 2018, 2:20 PM
markg accepted this revision.Feb 5 2018, 2:21 PM
This revision is now accepted and ready to land.Feb 5 2018, 2:21 PM

Fixed it!

Changing Dolphin to Alt-P would open a can of worms since the other panels are triggered with Function keys, so I think F11 makes sense here.

markg added a comment.Feb 5 2018, 2:23 PM

Fixed it!

Changing Dolphin to Alt-P would open a can of worms since the other panels are triggered with Function keys, so I think F11 makes sense here.

Nice!

That could indeed be a "small issue" ;)
Go for it. Commit (F11).

ngraham closed this revision.Feb 5 2018, 3:07 PM
ltoscano reopened this revision.Feb 5 2018, 3:13 PM
ltoscano added a subscriber: ltoscano.

Let phabricator close it...

This revision is now accepted and ready to land.Feb 5 2018, 3:13 PM
ltoscano closed this revision.Feb 5 2018, 3:16 PM

Too late.

Hmm, I thought I did. I just did a standard arc land.

Interesting, it seems that phabricator updated the link to the commit, but did not close the bug. Let's ignore it, unless it happens again.

I'm a little bit late to the party, but F11is kind of the standard key for fullscreen.

ngraham added a comment.EditedFeb 5 2018, 4:57 PM

Yeah, and Dolphin already overloads that by using F11 to show and hide the Information panel. I wouldn't object to using a different keyboard shortcut, but then we'd want to do it for all of for Dolphin's panels, and change the shortcuts here, too, which currently match Dolphin's.

Yeah, and Dolphin already overloads that by using F11 to show and hide the Information panel. I wouldn't object to using a different keyboard shortcut, but then we'd want to but we'd want to do it for all of for Dolphin's panels, and change the shortcuts here, too, which currently match Dolphin's.

my suggestion would be changing Dolphin's shortcut. If you use F11 it's unexpected if the window does not go to fullscreen. Even for dolphin a fullscreen shortcut might make sense. And in the case of fullscreen: Firefox defined that as the default. We have to accept that ;-)

ngraham edited the test plan for this revision. (Show Details)Feb 5 2018, 9:53 PM