Correct RTL accelerator arrow keys#2797
Conversation
The existing <ctrl>+Left would interfere when editing a filename Closes elementary#2677.
stan-janssen
force-pushed
the
correct-rtl-accelerator-arrow-keys
branch
2 times, most recently
from
4ebfe09 to
3730615
Compare
|
|
||
| uint prev_key; | ||
| uint next_key; | ||
|
|
| @@ -244,10 +244,22 @@ public class Files.IconView : Files.AbstractDirectoryView { | |||
| /* Override native Gtk.IconView cursor handling */ | |||
| protected override bool move_cursor (uint keyval, bool only_shift_pressed, bool control_pressed) { | |||
| Gtk.TreePath? path = get_path_at_cursor (); | |||
There was a problem hiding this comment.
I would move this next to the clause that uses it
| /* Override native Gtk.IconView cursor handling */ | ||
| protected override bool move_cursor (uint keyval, bool only_shift_pressed, bool control_pressed) { | ||
| Gtk.TreePath? path = get_path_at_cursor (); | ||
|
|
| protected override bool on_view_key_press_event (uint keyval, uint keycode, Gdk.ModifierType state) { | ||
| uint up_key; | ||
| uint down_key; | ||
|
|
jeremypw
left a comment
jeremypw
left a comment
There was a problem hiding this comment.
There is some whitespace I would consider superfluous. I try not to leave space between a variable declaration and where it used. Not all existing code may comply!
I have not tested this yet but looks like it does not fully cope with a change in LTR/RTL state "on the fly" as the shortcuts are set up on construction. May be better to have fixed shortcuts (e.g. for action-focus-left and action-focus-right) and deal with the state in the handlers. Not sure how important this is in practice though.
|
Thanks for the review. I'll remove the superfluous whitespace (I'm not completely "fluent" yet in the code style of the project, so thanks for pointing these things out).
The left/right arrow keys for ascending and descending through the hierarchy and for navigating through Icon view do change "on the fly", but the ones for focusing different areas and the ones for Forward / Back do not. I'm not aware of a use case for changing these things on the fly (I think it can only be done using the interactive debugger?) so I don't think it's a big issue myself. |
This corrects accelerators that include a Left or Right arrow key when using a Right-to-Left (RTL) language, where UI elements are draw from right to left. This includes switching focus to the sidebar, which is on the right side in an RTL situation, as well as the 'drill down' and 'drill up' keys when navigating in list or Miller views, and the arrow keys for selecting the previous and next icon in icon view (the icons are also drawn right to left, with the 'next' icon being on the left of the 'previous' icon in an RTL situation. This also matches the directions of the arrow buttons in the tool bar.
stan-janssen
force-pushed
the
correct-rtl-accelerator-arrow-keys
branch
from
3730615 to
8be84dd
Compare
|
Yes, you are probably right that its not a problem in practice. I tried switching to an RTL language and although some widgets (e.g. TextView) start typing RTL, the layout of widgets does not change. I guess you either need to use the interactive debugger or change the system locale in settings (needing a new session). I'll test tomorrow. |
2 participants
This corrects accelerators that include a Left or Right arrow key when using a Right-to-Left (RTL) language, where UI elements are draw from right to left.
This includes:
(This PR includes the commits from PR #2796 )