CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

Keyboard shortcut [ don't work when on all keyboards #1207

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Affected Version: 2.2.1-67-g17ca55f

The [  (previous file) does not work on OS X (Safari/Firefox). Presumably this 
has
something to do with the fact that on a swedish keyboard the alt key is required
to type [.

Shorcuts like / and ?, that only require Shift, do work and for ] there is an 
alternative shortcut.

Original issue reported on code.google.com by robin.ro...@gmail.com on 3 Dec 2011 at 11:33

GoogleCodeExporter commented 9 years ago
This looks like the same thing:
Both [ and ] (previous/next file) are broken on Finnish keyboard layout (Win7, 
Chrome 17.0, gerrit 2.2.2.1). And yes, typing those characters requires the 
"Alt Gr" key.

What's the alternative shortcut for 'next file' ?

Original comment by moigoo...@nic.fi on 13 Mar 2012 at 5:43

GoogleCodeExporter commented 9 years ago
Same problem here with german keyboard layout on OS X. Is there an alternative 
shortcut?

Original comment by bruno.bo...@gmail.com on 1 Mar 2013 at 10:51

GoogleCodeExporter commented 9 years ago
Here with win 7 / gerrit 2.7-rc2-529-ge348f21 it doesn't works either on chrome 
27.0.1453.116 BUT it works with firefox 22.0.

(French keyboard, but i'm not sure it matters since it works fine with firefox)

Original comment by maxime.g...@gmail.com on 5 Jul 2013 at 1:13

GoogleCodeExporter commented 9 years ago
Some shortcuts does work, like "?" (shift +)
Some shortcuts does not work, like "]" (altgr 9)

OS: Windows 7, 
Browser: Chrome 33.0.1750.117 
Gerrit: 2.8.1
Keyboard: Finnish layout

Original comment by lauri.ja...@gmail.com on 4 Mar 2014 at 10:23

GoogleCodeExporter commented 9 years ago
On the Spanish keyboard you have to type Ctrl+Alt+KEY to get [ and ]. KEY being 
the keys where [ and ] are located on the US keyboard.

On the OLD Change Screen this used to work to change between files while 
reviewing the diff. On the NEW Change Screen it doesn't work anymore, except 
when first clicking outside of the 'editor' (i.e. somewhere along the border of 
the website).

OS: Ubuntu 13.10
Browser: Firefox 27
Gerrit: 2.8
Keyboard: Spanish layout

Original comment by omark...@gmail.com on 13 Mar 2014 at 7:56

GoogleCodeExporter commented 9 years ago
On the German Keyboard you've to press Altgr + KEY.

While in Chrome (Version 34.0.1847.132 (265804)) it does work Firefox (29) 
fails on it.

OS: Archlinux
WindowManager: XMonad
Browser: Firefox 29
Gerrit: 2.8.5
Keyboard: German layout

Original comment by anpie...@gmail.com on 15 May 2014 at 8:40

GoogleCodeExporter commented 9 years ago
Same here with a Swedish keyboard. On old screen it works with AltGr+9 but on 
new screen it doesn't.

OS: Linux Mint 14
Browser: Firefox 27.0
Gerrit: 2.8.5
Keyboard: Swedish

Original comment by andreas....@gmail.com on 8 Jul 2014 at 11:12

GoogleCodeExporter commented 9 years ago
To add more weirdness to this the bindings are inconsistent on different 
screens.
We recently upgraded to 2.9 at work and the new file view is quite hard to use 
with a swedish keyboard.

In the patchset view [] works correctly but not in the file diff view
In the patchset view ? works correctly, but in the file diff view it starts a 
search. Despite the instructions saying it should show help. '/' Also starts 
the same search.

In the file view 'å' will take you to the next file, but there is no combo I 
found that will take you back. Using 'å' in the patchset view does not do 
anything.

While the new diff view seems to be aimed at being even more keyboard driven 
which is fantastic I think that it currently lacks a lot of support for non 
english keyboards. This really needs to be fixed. Especially since the bindings 
are different on different pages.

Since these problems seem to have been around for quite a long time I really 
think at least a response from the developers would be appropriate.

Cheers

Original comment by o...@skyshaper.net on 5 Nov 2014 at 4:37

GoogleCodeExporter commented 9 years ago
I've noticed that if you click the top of the review in diff-view (e.g. above 
the diff) - as to give another element focus. Then [ and ] works.

On the next page 'that element' looses focus again so you'd have to re-click 
the top. Impractical.

Original comment by noseg...@gmail.com on 14 Nov 2014 at 9:14

GoogleCodeExporter commented 9 years ago
On a german keyboard pressing ? to access help, opens the backwards search when 
viewing a diff. Thus I cannot look up any shortcuts there.

Original comment by andreas....@freiheit.com on 24 Nov 2014 at 3:56

GoogleCodeExporter commented 9 years ago
> On a german keyboard pressing ? to access help, opens the backwards search 
when 
> viewing a diff. Thus I cannot look up any shortcuts there.
This is not related to the key language but to where the focus is. If the focus 
is on the code mirror editor pressing ? opens the backwards search, if the 
focus is somewhere else you get the help on pressing ?.

Original comment by edwin.ke...@gmail.com on 26 Nov 2014 at 7:57

GoogleCodeExporter commented 9 years ago
>> On a german keyboard pressing ? to access help, opens the backwards search 
when 
>> viewing a diff. Thus I cannot look up any shortcuts there.
> This is not related to the key language but to where the focus is. If the 
focus is 
> on the code mirror editor pressing ? opens the backwards search, if the focus 
is somewhere else you get the help on pressing ?.

Indeed, the keybindings seem to work if I manually use the mouse to select the 
top or bottom area of the diff pages.

However, it's very hard to know what area of the page you are in since there is 
no visual indication, and there is no easy way of moving to the area where the 
keybindings work without using the mouse.

But I still think something is fishy with the key language. From what I can 
tell, most key bindings work except []. 

Original comment by o...@skyshaper.net on 26 Nov 2014 at 8:15

GoogleCodeExporter commented 9 years ago
My comment was only related to pressing ?, the issue with [] may depend on the 
key language.

Original comment by edwin.ke...@gmail.com on 26 Nov 2014 at 8:18

GoogleCodeExporter commented 9 years ago
This issue is preventing many of our gerrit users to use the new change screen 
as [ and ] shortcuts do not work on the diff view

We are mainly using finnish keyboard layout where you get [ with altgr+8 and ] 
with altgr+9

Original comment by janne.ro...@vincit.fi on 13 Feb 2015 at 6:28

GoogleCodeExporter commented 9 years ago
Verified.

Just for a quick analysis of the problem: I'd reckon the event.altKey isn't 
being checked to handle brackets correctly. Just for the record, the event 
recorded that should be registering keyCodes 18+56 as [ and 18+57 as ] on the 
Finnish layouts with event.altKey being true. Problems may of course arise 
because we have no way of guessing if the user has Swedish or Finnish layout if 
his browser isn't set to one of these languages.

Original comment by aleksi.h...@vincit.fi on 13 Feb 2015 at 8:31

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Is anything being done with this issue?

The problem blocks the next file function using the keyboard. The key can be 
used if focus is manually moved out of the diff view, but that defeats the 
purpose of the hotkey to begin with!

Even if this problem doesn't affect every user of gerrit, for those that are 
affected it blocks major functionality. (Also not like the affected user amount 
is minor either. This issue affects multiple whole countries). The next file 
function in diff view is the single most important hotkey when doing code 
review. The priority should not be minor.

Original comment by tapani.k...@arcusys.fi on 25 May 2015 at 7:29

GoogleCodeExporter commented 9 years ago
Agreed! This is Major.

Original comment by moigoo...@nic.fi on 26 May 2015 at 9:10