Rewrite of line boundary movement actions #609
Conversation
peitschie
changed the title
|
Build succeeded. |
| @@ -52,6 +52,18 @@ gui.CaretManager = function CaretManager(sessionController) { | |||
| } | |||
|
|
|||
| /** | |||
| * @return {!number|undefined} | |||
There was a problem hiding this comment.
Not sure if one should just assume the units of the offset to be in pixel, given the method name. But I would prefer to be explicit about that in the API dox.
There was a problem hiding this comment.
Fixed in method name and API dox.
|
Bah, github had the list of commits unordered, so I tried to grasp the second commit as first and made some wrong assumptions of what is already in current master. Sorry for my partial confusion. (And now I also press that green button "Comment" for this comment. The heat!) |
| /**@const*/ MIN_OVERLAP_THRESHOLD = 0.4; | ||
|
|
||
| /** | ||
| * Return the faction of overlap between two rectangles. If there is |
|
Being ready to shift any blame to the heat here, the stuff I grasped made sense to me. Still not sure why the paragraph filter/acceptor/classifier is needed, though. RTL will be some fun to add later... oh if will only would have access to native layout info... but for now this PR seems an improvement over the current code to me :) So optimistic to send it direction harbour in the second review round |
|
Build succeeded. |
|
Build succeeded. |
| } | ||
| // usually the rect is 1px width, so rect.left ~= rect.right. | ||
| // Right is used because during IME composition the caret width includes | ||
| // the chars being composed. The caret is *always* flush against the right side |
There was a problem hiding this comment.
I never realised flush has so many definitions :S.
The one I'm specifically using here is: "level or even with another surface"
There was a problem hiding this comment.
Ah, so it is an adjective here. So I had not yet learned that it can also be that, okay.
|
I run risk failing a test about this PR, but still feel rather happy about the code I got. |
|
Build succeeded. |
|
|
||
|
|
||
| /** | ||
| * @constructor |
There was a problem hiding this comment.
Perhaps it might be still worth it after all to also note here explicitely that this structure is to be considered opaque to anyone not core.StepIterator, so people who come across this structure first get the idea from the beginning.
|
Super. So please 🔨 down to normal commits, then 🐫, eh, ⛵, eh, 🚢, eh, 🚀, no, |
!
When navigating vi the up/down arrow keys, the user often wants to move multiple lines in sequence (e.g., to visually place the caret in one of the following lines). To stop the caret suddenly jumping to the left side of the page when crossing an empty paragraph, the caret's X position will be stored and re-used by subsequent up/down movement for up to 2s after navigating by line. Any other operations performed by the local user (e.g., cursor movement to the left or right), or edit operations by collaborative users will immediately clear the stored X position.
|
Build succeeded. |
Rewrite of line boundary movement actions
|
Darn, forget about that: we also now need to update ChangeLog.md on each user relevant changes/fixes. Could you please add another PR with the respective entry for the ChangeLog.md? |
Rewrite of line boundary detection logic to address numerous cursor movement issues (#555, #405, #227, #225, #224, #185, #124 and #98).
This also removes the last uses of the gui.SelectionMover class, allowing it to be removed in a subsequent PR.