Added support to CMD+Left/Right for Mac navigation #380
Conversation
|
@atonse Thanks for this PR! I think this was addressed by #378 . That code hasn't been released yet, though. Cursor movement via key presses is one of the few places in mobiledoc-kit that we still allow some of the native browser behavior. This is mostly done because the heuristics involved to position correctly from up/down keys are challenging (where your cursor moves when you hit up-arrow is dependent on current cursor position, font metrics, and also where the cursor was previously on that line) so we let the native browser cursor movement happen and then read the window's That said, explicitly handling home/end movement seems like a good idea, rather than trusting that the browser will put the cursor in the place we expect it to. Adding key commands for cmd-up and cmd-down also seems good. These would focus on |
|
Looking again now, I see that #378 deals with |
|
An acceptance test like this one would be the ideal sort of test for these commands. |
|
@bantic the other navigational element is Alt/Option+left and right (go back and forward by a word). It was easy to add this PR since I just had to copy CTRL A/E behavior. But if you could point me to the right direction (like what you normally do to walk back until you find the next whitespace), I could add these other two. Lastly, Shift + all of these should continue adding to the selection. I don't envy you guys – this long tail of subtle behaviors can be endless. |
Agreed. 😢 It seems that no
We have a If you could add an acceptance test for the command-left and command-right I'd be happy to merge this PR as is and address the other movement combos separately. And I'd be very happy to help out on the follow-up here on github or through the gitter chat. |
|
Ok thanks @bantic, i'll work on the test and get back to you. |
|
Weird – I tested this locally using phantom ( |
|
From my local testem: |
|
Just realized this is MacOS only and the travis tests are failing for windows. I also added Firefox to my testem and have all local browsers passing. let's see how we do on windows. |
|
All out of ideas – any suggestions welcome :) |
| if (Browser.isMac()) { | ||
| assert.equal(changedCursorPosition.offset, expectedCursorPosition, 'cursor moved to the end of the line on MacOS'); | ||
| } else { | ||
| assert.equal(changedCursorPosition.offset, originalCursorPosition.offset, 'cursor not moved to the end of the line (non-MacOS)'); |
There was a problem hiding this comment.
I think for windows this should actually assert that the cursor moves one character, not that it doesn't move at all.
There was a problem hiding this comment.
So I'm wondering about that because for me, Command on windows is the Windows Key, and Windows Key Left/Right actually are OS-level commands that rearrange your windows.
|
@atonse I'll give this a run-through in windows quickly and let you know what I see. :) |
| let changedCursorPosition = Helpers.dom.getCursorPosition(); | ||
| let expectedCursorPosition = 0; // beginning of text | ||
|
|
||
| if (Browser.isMac()) { |
There was a problem hiding this comment.
ah ha! isMac is not a function, it's a property. 😝
There was a problem hiding this comment.
Oops! Sorry about that. fixed and pushed.
|
@atonse looks great! Can you please squash the commits? |
In MacOS, in addition to CTRL+A and E for "Home" and "End", another commonly used combination is CMD+Left for Home, and CMD+Right for End. This commit adds those shortcuts, and pulls out the "getStartOfLine" and "getEndOfLine" functions out so they can respond to multiple command keys. Also fixed the triggerKeyCommand test method to accept non-alpha codes
|
@bantic squashed the commits. |
|
@atonse thank you! |
|
released in v0.9.6 |
In MacOS, in addition to CTRL+A and E for "Home" and "End", another commonly used combination is CMD+Left for Home, and CMD+Right for End.
This commit adds those shortcuts, and pulls out the "getStartOfLine" and "getEndOfLine" functions out so they can respond to multiple command keys.
Regarding tests: I didn't find any tests for the existing Home/End key commands, so I didn't add anymore. But I have run the test suite and have noted no regressions.