Would it break on my Dvorak keyboard?
Would it break on my Dvorak keyboard?
I just tested your pull request.
After clicking on…
I am not familiar with the issues with other keyboard layouts, but it seems plausible that going back to the old method will introduce a shitstorm of user complaints if the keyboard shortcuts suddenly stop working altogether. So, even if the hacks to the current implementation are not exactly elegant, I would imagine it would prevent some annoyances.
well i think in the end it probably doesn’t matter if we break dijit-native keyboard stuff any further or not, tt-rss broke it left and right for years with global hotkeys and nobody complained. :shrug:
very nice, that’s exactly what I meant. can you resubmit with it included so we would just skip the awkward inline mixins altogether?
i don’t think there’s going to be a shitstorm either way because in my opinion a minority of users is even aware of hotkeys. a subset of those (Germans and others (?) with QWERTZ layout) had them broken for years and nobody complained (much).
anyway, my main problem with this PR was adding inline methods to every toolbar, but with custom toolbar class that wouldn’t be an issue, although i’d ask @suraia to check this:
The new approach will not work with keyboards where the letter on the key does not necessarily correspond to the generated character. For example, pressing “a” on a Cyrillic or Chinese keyboard might generate another character. Everything else should work as is.
Will do. Will probably need a day or two to finish everything up.
This is actually by design because buttons stay focused after closing them (so you can open them again using Space or Enter). I kept this behavior as this is also how other GUIs usually work. I can try to change this, though, if you prefer.
Ah - I’m OK if the only hotkeys I use are a, m and any digit then.
This is just my take on it, but I don’t think any focus other than textarea/input fields justify breaking hotkeys, especially when considering the navigation hotkeys (‘g a’, ‘g s’ etc.). I really think consistency here should have priority. Apart from that, I don’t think anyone clicks on a button and then uses the arrow keys to go through a pop-up menu instigated by that button.
So, I would indeed advocate that tt-rss hotkeys should prevail even after those toolbar buttons have gained focus.
this wouldn’t work with basic html buttons, i think, so we might as well override this for dijit controls
then again i don’t think tt-rss has any hotkeys with space, enter or tab so maybe allow specifically those? not sure about this tbh
I have updated the PR (https://git.tt-rss.org/fox/tt-rss/pulls/114) to include the proposed toolbar changes. It does not include any changes for the buttons yet because I want to test them in more detail first.
diff looks very clean, great job there.
Looks great! I just applied to changes to my tt-rss instance and did a quick test to be sure. As expected, the results are identical to the previous pull request:
I have just rebased the PR to the current master and added a new commit that takes care of most of the buttons. I have not touched a few ones that did not seem necessary (login form, installation form).
Buttons will now not keep focus after their menus are closed, enabling the use of hotkeys. Menus still grab button presses while they are open.
Thanks! I just tested it. I found no issues with the combo/input/dropdown buttons, cool!
It seems that the main menu button, however, is acting up, although it is not specifically due to this pull request: without the PR, it only gets focus after closing the menu (only with a mouse click, because Esc does not work), not when clicking it the first time (which would be the preferrable behavior and consistent with the other buttons). Obviously this PR prevents focus after the second click (which is OK), but there is no way to use the arrow keys to go through the main menu anymore. Perhaps that could be fixed as well within this PR?
Tested on Waterfox 56.
It looks like all DropDownButtons (and ComboBoxes) exhibit this behavior. I will take a look tomorrow.
I have updated the second commit, which fixes the ComboButton and DropDownButton focus issues for me.
all those customized controls, i hope we won’t run into issues with newer dojo versions
overall looks great though
Last commits to those widgets were in 2012 and 2015, so we should hopefully be safe for a while.
well, i think i might as well merge this
Just tested and it’s all working. Wow, that was quite something Thanks @suraia!!
e: Not that anyone really cares but https://github.com/ltGuillaume/FeedMei has been updated to support the changed class names
I really appreciate your work, thank you!
With a german keyboard and Firefox I never questioned the flakyness of the shortcuts