Skip to content

Create commands to restrict caret traversal inside a textblock#458

Merged
jonathonherbert merged 7 commits intomainfrom
ahe/prevent-caret-boundary-traversal
Nov 3, 2025
Merged

Create commands to restrict caret traversal inside a textblock#458
jonathonherbert merged 7 commits intomainfrom
ahe/prevent-caret-boundary-traversal

Conversation

@andrewHEguardian
Copy link
Contributor

What does this change?

Attempts to address #279 - by preventing the caret traversing outside of the boundaries of text blocks. Previously (in Chrome) navigating with the arrow keys would take the caret outside of the bounds of the current text block and into adjacent blocks.

This is implemented as a set of key mapping commands. I've only applied it to TextFieldView to validate the concept, although it applies to other elements.

Before

Screen.Recording.2025-07-04.at.16.34.21.mov

After

Screen.Recording.2025-07-04.at.16.34.55.mov

How to test

Add a new element, eg Image, and enter the cursor into a plain text block. You should not be able to leave the bounds of that block by using arrow keys, only by tabbing.

@andrewHEguardian andrewHEguardian requested a review from a team as a code owner July 4, 2025 15:55
Copy link
Contributor

@emdash-ie emdash-ie left a comment

Choose a reason for hiding this comment

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

This looks good to me, but I think it should get input from someone with more familiarity with prosemirror-elements.

I’m happy that preventing this behaviour makes sense, as it matches how apps generally work and makes Chrome’s behaviour the same as Firefox’s.

The code looks fine as someone unfamiliar with it.

Using a keymap to achieve this seems like an odd way for prosemirror to work, but that’s beyond my knowledge.

If I understand right, this means the up arrow in some fields where the contents are wrapped over multiple lines will go straight to the start of the field. It’d be nicer for the user if it went up a line (and to the start if it’s the first line) and retained their previous left-right position so that pressing up followed by down always took them back to the previous position. But that might be a lot of work, and I don’t think it’s necessary for it to block this PR. (Though, if what I’ve described is the existing behaviour and this changes it, that might be worth some consideration.)

@andrewHEguardian
Copy link
Contributor Author

I'm also very unfamiliar with prosemirror(-elements) so any input is very appreciated!

If I understand right, this means the up arrow in some fields where the contents are wrapped over multiple lines will go straight to the start of the field. It’d be nicer for the user if it went up a line (and to the start if it’s the first line)

This is the case 👍 video might not have been clear

and retained their previous left-right position so that pressing up followed by down always took them back to the previous position.

Ah, I hadn't even realised that! That's a great spot

@emdash-ie
Copy link
Contributor

If I understand right, this means the up arrow in some fields where the contents are wrapped over multiple lines will go straight to the start of the field. It’d be nicer for the user if it went up a line (and to the start if it’s the first line)

This is the case 👍 video might not have been clear

Ah, fantastic – I missed the initial up press in the video, sorry!

Copy link
Contributor

@jonathonherbert jonathonherbert left a comment

Choose a reason for hiding this comment

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

Love love love this! @emdash-ie has covered the product things. Great job on finding the appropriate parts of the PM API to achieve this.

As @emdash-ie mentions, preserving the state necessary to replicate the existing caret selection memory when moving up against a document boundary and then back down a line will be challenging. Feeling is that we either agree to keep the behaviour in this PR as-is, or keep the previous behaviour. @paperboyo may have thoughts.

A few non-blocking suggestions below. Before we ship, could we add tests for each case (up, down, left, right against boundaries)? We've integration tests that cover selection behaviour @

it("should select the correct range in ProseMirror when the select all shortcut is used within the field", () => {
— be super happy to pair if they're proving tricky to write.

if (view?.endOfTextblock("up", state)) {
// Move the cursor to the start of this block
const tr = state.tr.setSelection(
TextSelection.between(state.doc.resolve(0), state.doc.resolve(0))
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a little clearer as:

Suggested change
TextSelection.between(state.doc.resolve(0), state.doc.resolve(0))
TextSelection.atStart(state.doc)

(there's atEnd for the other case, too)

@paperboyo
Copy link
Contributor

Hi. Thanks all for working on this! Chatted to @emdash-ie a tad and IIUC, this should be fine when fixed as proposed. Up from middle of second line to the middle of first line, then Up again to beginning and then Down to the beginning of second line all sounds correct.

@andrewHEguardian
Copy link
Contributor Author

I've realised this doesn't prevent moving from a paragraph to the text fields (where you can't move back out agian with keyboard). IIUC the paragraph bit would be out of scope for this so we couldn't apply the keymapping there?

Screen.Recording.2025-07-10.at.11.07.57.mov

@jonathonherbert
Copy link
Contributor

Any objections to me merging this, @andrewHEguardian?

@andrewHEguardian
Copy link
Contributor Author

Any objections to me merging this, @andrewHEguardian?

please do!

@jonathonherbert jonathonherbert merged commit cfb9984 into main Nov 3, 2025
6 checks passed
@jonathonherbert jonathonherbert deleted the ahe/prevent-caret-boundary-traversal branch November 3, 2025 11:28
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.

4 participants