Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add line selection shortcuts #403

Closed
wants to merge 1 commit into from
Closed

Conversation

lessless
Copy link

This PR is intended to introduce line selection functionality on OS X:

  • when user press CMD+SHIFT+LEFT text is selected from the cursor position till the beginning of the line
  • when user press CMD+SHIFT+RIGHT text is selected from the cursor position till the end of the line

@@ -76,6 +94,13 @@ export const DEFAULT_KEY_COMMANDS = [{
}
}
}, {
str: 'META+SHIFT+LEFT',
run(editor) {
if (Browser.isMac) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker for this PR, but we've really got to figure out a more generic solution for environment-dependent hotkeys. Phew.

@mixonic
Copy link
Contributor

mixonic commented May 18, 2016

@lessless in a multi-line section, wouldn't this highlight to the start of a section and tail of a section from the cursor and not the line?

@bantic
Copy link
Collaborator

bantic commented May 18, 2016

This looks pretty good. The Range constructor takes a third argument for direction, and that is the preferred way to construct new ranges (instead of setting the .direction property on the range). I'll take a closer look at the tests later.

@lessless
Copy link
Author

@mixonic good catch! Can you suggest how to get an end of the line?

@lessless
Copy link
Author

@bantic what are you thinking about:

  • add the second line to the tests
  • change LEFT/RIGHT+META+SHIFT to UP/DOWN+META+SHIFT
  • pretend as it was intended 💃

@mixonic
Copy link
Contributor

mixonic commented May 18, 2016

@lessless we should be wary of introducing cursor controls (or any hotkeys) that aren't compatible with the native experience. Creating our own set of bindings would be fairly trivial from an engineering perspective, but a bit trolling from the perspective of user experience. I'd like to see the implementation of this rule be accurate to what a native text editing surface (like TextEdit on OSX) would do. In general our goal for user interactions should be that they are as unsurprising as possible.

The mobiledoc-kit editing surface has avoided introspection of layout as much as possible. @bantic mentioned we do it for one spot in the code earlier this morning in passing, we can find a link shortly. The algorithm for finding the start/end of the line can probably be fairly boring- identify the cursor's y-position and the x-position of the side of the text area, create a bounding rectangle and see what part of a text node is there, map that text node and char back to a marker.

@lessless
Copy link
Author

lessless commented May 19, 2016

@mixonic are we talking about onmouseomove hook to track pointer position (and pass over it coordinates via editor.cursor.{pageX, pageY})?

@mixonic
Copy link
Contributor

mixonic commented May 21, 2016

@lessless no mouse involved- the aim is to determine what the left-most and right-most characters are in a line. Mobiledoc-kit knows nothing about line wrapping today. To accurately implement the cursor behavior you're building here, we need to have a way to determine the edge positions for a selection.

@lessless
Copy link
Author

Just to keep conversation in one place:

Yevhenii Kurtov @lessless 09:09
and I just have a thought - is it possible to proxy-pass SHIFT+META+LEFT/RIGHT/UP/DOWN key commands to the browser? :\

Cory Forsyth @bantic 20:48
@lessless Yes! Good point. That’s the best way to fix this, and it will make all the other META+ARROW commands work properly. Rather than playing whack-a-mole fixing them all one-by-one, we need to modify the “new selection or update selection” detection code in the EventManager. Right now it updates the selection on keyup of movement keys: https://github.com/bustlelabs/mobiledoc-kit/blob/master/src/js/editor/event-manager.js#L159
But when the key combo is META+ARROW that likely won’t fire because the keydown code is catching it and handling it semantically here: https://github.com/bustlelabs/mobiledoc-kit/blob/master/src/js/editor/event-manager.js#L124
That switch in event-manager probably just needs to ignore arrow keys if meta is also pressed — then the browser will do its native thing and the keyup handler will fire and re-read the selection from the DOM

@bantic
Copy link
Collaborator

bantic commented Aug 3, 2016

@lessless Thank you very much for this PR, and I'm sorry that it has lingered so long! We opted to use selection polling to keep the editor's range in sync with the dom's range, which greatly simplifies the internal code for reacting to keyboard-related movement key combinations. Using the modifier keys + arrow keys to move now "just works" without having to whitelist particular browser key combinations. Unfortunately the movement key combos with delete don't benefit from this, so option- and alt + delete is implemented separately and as a result we still don't have delete-to-end-of-line (meta+delete on Mac, e.g.). I'll add an issue to track that (we'll need to use a positioning heuristic to find the start/end of a given line; section.headPosition does not do what we want here, sadly).

@bantic bantic closed this Aug 3, 2016
@bantic bantic mentioned this pull request Aug 3, 2016
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants