Update inner selection when outer selection updates selection within a text field #400
Conversation
rhystmills
force-pushed
the
rm/update-inner-selection-from-outer-editor
branch
from
e7007f9
to
6a8a328
Compare
Co-authored-by: Simon Byford <simon.byford@guardian.co.uk>
| @@ -288,18 +289,23 @@ describe("createPlugin", () => { | |||
| // By inserting content before the element, we enable the content to move | |||
| // upward, changing the command output. | |||
| const positionThatEnablesUpCommand = 0; | |||
| const tr = view.state.tr.replaceWith( | |||
| const tr = view.state.tr.replace( | |||
There was a problem hiding this comment.
These tests have changed – do we have a test for the case when content changes but the selection does not?
| if (overlap > 0) { | ||
| endOfOuterDiff += overlap; | ||
| endOfInnerDiff += overlap; | ||
| if (shouldDispatchTransaction) { |
There was a problem hiding this comment.
More a note to self: shame we can't ask the transaction itself if it's changed! (There's a this.updated field, but it's private.) We might be able to avoid this state with ('transaction selection has changed' && 'transaction.steps.length > 0), but it's a bit of a weird one as it relies on the dev knowing that .setSelection mutates the selection, and .replace adds steps. So this is fine.
| // nestedElement FieldViews are always updated as a workaround for a bug | ||
| // with merging text elements in the zipRoot plugin after an intermediate element is | ||
| // deleted. | ||
| if ( | ||
| fieldValuesChanged || | ||
| innerDecosChanged || | ||
| anyDescendantFieldIsNestedElementField(newNode) | ||
| anyDescendantFieldIsNestedElementField(newNode) || |
There was a problem hiding this comment.
Do we have any tests for this logic?
|
(hah – too slow!) |
|
Whoops, apologies! |
|
(There was already an approval, I was just sticking my oar in ⛵ 👍) |
I'll try and address these in another PR. |
Background
In prosemirror-elements, elements are a collection of fields representing user-editable data. Most logic managing these fields is in the relevant
FieldView. Text fields are all based onProseMirrorFieldView, via implementations that extend it, e.g.TextFieldView. These contain instances of ProseMirror ('inner editors'), and the 'outer editor' (the editor that contains the element) delegates control of certain actions to the 'inner editor' managed by the FieldView.Currently, two kinds of changes from the outer editor can cause changes in the field view:
Decorationsthat affect the range covered by the field view.However, if the selection is updated by the outer editor, the selection within a relevant field is not updated.
This hasn't been obvious in the past, because usually the selection is updated by the field's 'inner' editor. With notes being supported in nested element fields, when a user hits 'F10' in Composer with text selected inside the nested element, the selection is updated by the outer editor (to which the menu belongs), and should be moved to the end of the note as users expect. Because the selection is not passed to the inner editor, the note content remains selected, so when the user types, they overwrite the content of the note.
What does this change?
This PR updates field views when the selection changes, and the text fields (extensions of
ProsemirrorFieldView) update their inner editor selection to match the incoming selection. This means that the usual F10 behaviour for notes also works inside our test fields.This happens as follows:
inplugin.ts, a function at the boundary of the plugin, receives state changes from the outer editor, which included the updatedSelection`.FieldViewsvia some intermediate functions.FieldViewthat needs to handle the selection change is the abstractProseMirrorFieldView, which all textFieldViewsextend.FieldViewchecks if the selection covers its own range. If it does, it updates its internal selection.How to test
yalc.n.b. - due to an unrelated bug with menu clicks causing the editor to lose focus, this only works with the F10 shortcut, not the notes menu button
I have updated existing tests to match the new update behaviour and added a test to reflect the selection behaviour itself, making sure that the inner selection is properly updated when there is a relevant incoming selection.