-
Notifications
You must be signed in to change notification settings - Fork 152
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
[SPIKE] Add delete to start of line for MacOS #636
base: master
Are you sure you want to change the base?
Conversation
refs bustle#445 - add `Position.atStartOfLine` method that takes an existing position from which to find the start of it's line - add `line` unit for use in `editor.performDelete` - update delete key event handler to pass `line` unit when <kbd>Meta+Backspace</kbd> is pressed
Unfortunately removing the delete key handler causes problems as soon as cards are involved - Cmd+Backspace can remove part of the card's DOM leaving the editor in a broken state - so it looks like a solution similar to this PR is needed at least in the short term. Looking at other editor implementations I saw the following behaviour:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an excellent direction to take this @kevinansfield!
@@ -101,6 +135,24 @@ Position = class Position { | |||
return Position.fromNode(_renderTree, node, offset); | |||
} | |||
|
|||
static atStartOfLine(position, editor) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this method should be able to become an instance method or getter.
editor
being passed to atPoint
is perhaps in error. It seems like the only thing read off from the editor is renderTree
and element
, and the root element can be read off the renderTree
just as well. Several other methods are coupled to the renderTree
so we should likely stop from using editor
if we can get by with just the render tree and references to the AST (section
).
@@ -62,6 +62,40 @@ function findOffsetInSection(section, node, offset) { | |||
} | |||
} | |||
|
|||
// TODO: expose as utility function, update cursor._findNodeForPosition | |||
function findNodeForPosition(position) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to have been copied from cursor.js
, the code should probably be de-duplicated?
refs #445
Position.atStartOfLine
method that takes an existing position from which to find the start of it's lineline
unit for use ineditor.performDelete
line
unit when Meta+Backspace is pressedI had a quick stab at re-implementing the default MacOS Cmd+Backspace behaviour but it's highlighted a problem. macOS allows you to place the caret visually at the end of the line when it's actual position is at the start of the next line (either through clicking at the end of the line or using Cmd+Right). You can see a demonstration here:
When using both mobiledoc-kit's internal
position
and the browser-provideddocument.getSelection().getRangeAt(0).getBoundingClientRect()
the only position exposed to JS is the beginning of the next line. The reason this is a problem is because to faithfully replicate macOS' default behaviour the delete needs to happen from the visual caret position rather than the logical caret position.I disabled the delete key event handling in
event-manager.js
to demonstrate the native macOS behaviour where you can see the delete happening from the visual rather than logical position: