chromium-codereview icon indicating copy to clipboard operation
chromium-codereview copied to clipboard

Published 20 hours ago • verified publisher icon esprehn

Keyboard shortcuts

Open sgraham opened this issue 11 years ago • 11 comments
trafficstars

In particular, I feel sad without j/k.

sgraham avatar Jun 06 '14 23:06 sgraham

What do you expect j/k to do?

esprehn avatar Jun 07 '14 00:06 esprehn

I was thinking next/prev file, but maybe next/prev hunk would make more sense in this layout.

sgraham avatar Jun 07 '14 01:06 sgraham

FWIW, these are the keyboard shortcuts I added to the WebKit code review tool. Personally, I only ever ended up using n/p for navigating comments.

enter add/edit comment for focused item escape accept current comment / close preview and help popups j focus next diff k focus previous diff shift + j focus next line shift + k focus previous line n focus next comment p focus previous comment r focus review select element ctrl + shift + up extend context of the focused comment ctrl + shift + down shrink context of the focused comment

ojanvafai avatar Jun 07 '14 02:06 ojanvafai

I added the <cr-keyboard> widget in https://github.com/esprehn/chromium-codereview/commit/37ce43da35733125e4870d1378966c041eb168b6 so this should just be a matter of hooking up the keys to things.

esprehn avatar Jun 08 '14 23:06 esprehn

I'd also like a keyboard shortcut to bring up the publish dialog. I often find I'm at the bottom of the page when doing a review, and want to publish at that point. It's annoying to have to scroll back up.

ojanvafai avatar Jun 10 '14 21:06 ojanvafai

I'll have to think about how to make that work with the dialog. HTML5 <dialog> is always fixed at the top of the screen so bringing it up would jump your scroll position to the top.

esprehn avatar Jun 10 '14 23:06 esprehn

Are you sure about that? That doesn't sound right to me.

ojanvafai avatar Jun 11 '14 02:06 ojanvafai

ojan@ Ah yeah I see

is vertically centered, I just set top: 100px long ago in the CSS since the center looked kind of silly for some small dialogs. I should remove that.

esprehn avatar Jun 11 '14 05:06 esprehn

How do you expect j/k to work when multiple patchsets are expanded?

esprehn avatar Jun 12 '14 17:06 esprehn

Not sure, probably stay in current PS? And maybe another key or J/K to go harder to the next one.

sgraham avatar Jun 12 '14 18:06 sgraham

Do whichever is easier. I doubt anyone will notice either way.

ojanvafai avatar Jun 12 '14 18:06 ojanvafai