RTC: Fix inline inserter reset on update sync#76980
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +50 B (0%) Total Size: 7.74 MB 📦 View Changed
ℹ️ View Unchanged
|
| /** | ||
| * Checks whether two records represent the same user-visible state | ||
| * (same text content and cursor position). | ||
| */ | ||
| function recordValuesMatch( | ||
| a: UseAutocompleteProps[ 'record' ], | ||
| b: UseAutocompleteProps[ 'record' ] | ||
| ) { | ||
| return a.text === b.text && a.start === b.start && a.end === b.end; | ||
| } |
There was a problem hiding this comment.
I guess this technically can produce a false positive: same text, different formats. Though probably not important for this case.
P.S. Seems like the rich-text component should have similar utility. Basically, shallow equality for RichText records.
There was a problem hiding this comment.
You're right that this misses some of the text record attributes. I considered adding them all, but formats and replacements are arrays. These are newly created from RESET_BLOCKS, so we'd also need deep equality checking in order to fix the bug. This isn't awful but is a lot more expensive than the previous object ref comparison.
Comparing text + start/end will let us know if the last .text changed meaningfully and the popover should change. Otherwise, as you said, the formats aren't important for this behavior or related tests.
|
Flaky tests detected in de9c87d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24045669688
|
Mamaduka
left a comment
There was a problem hiding this comment.
The focus fix for this particular bug is good ✅
I think the general problem with REST_BLOCKS might need more attention, as it will affect any consumer code that keeps track of record and triggers side effects based on it.
Can we keep the same reference for blocks that aren't affected by RTC? Will this bookkeeping be expensive?
cc @jsnajdr, @youknowriad
| /** | ||
| * Tracks the last record whose value differed from the current one. | ||
| * Used to determine whether the user has actually typed something | ||
| * (as opposed to a re-render producing a new record object with | ||
| * identical content, e.g. during a block reset). | ||
| * | ||
| * Maintains a sliding window of 2 records, similar to the original | ||
| * Set-based approach but using value comparison instead of reference | ||
| * equality so that new objects with the same content (text, start, end) | ||
| * are treated as duplicates. | ||
| */ |
There was a problem hiding this comment.
I think we can simplify the JSDoc block here. There's no need to mention previous methods or anything else. Just what it does currently.
The Tt's used for two purposes:
Technically these two cases are almost the same ( And the second use case, updating the existing entity, although it has always been present, started to be seriously used only now, when RTC was implemented. Because the "updates from external sources" didn't really happen before. This is why One of the typical recent fixes is #76025. Here The original version of #76025 added an |
Mamaduka
left a comment
There was a problem hiding this comment.
Let's add a changelog entry, and I think this is good to merge.
* Modify useLastDifferentValue to use direct text comparison * Add tests for useLastDifferentValue * Fix tests, also compare start and end location. Use Set() instead of two references * Simplify useLastDifferentValue doc block * Add CHANGELOG entry Co-authored-by: alecgeatches <alecgeatches@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org>
|
|
||
| ### Bug Fixes | ||
|
|
||
| <<<<<<< fix/rtc-inline-inserter |
There was a problem hiding this comment.
The changelog here has git conflict markers. I'm removing them at #77127
There was a problem hiding this comment.
@oandregal Thank you for fixing this, I'm not sure how this got by me.
What?
Fixes #76977.
Currently, the inline block inserter disappears during RTC sessions when another user is actively editing:
block-inserter-disappears.mov
Why?
The root cause is
useLastDifferentValuein the autocomplete component. This hook tracks whether the user has changed something by keeping a history of the last 2 rich-text record objects in aSet. SinceSetuses object reference equality, whenRESET_BLOCKSfires during RTC sync and rebuilds the block tree from scratch, the resulting new record objects (same text, different references) append to the history rather than it staying the same. The "previous" record ends up having the same.textas the current one, sodidUserInputevaluates tofalseand the popover is removed.How?
This is a
RESET_BLOCKS-related bug like #76025, a class of bugs we've seen where the editor will reset local editor state on sync in a way that doesn't make sense in a collaborative model.This PR changes
useLastDifferentValueto compare by.textvalue instead of object reference. The new implementation uses two refs: one for the previous record and one for the last record that had a different.text. When a re-render produces a new record object with the same text content (as happens duringRESET_BLOCKSin an RTC sync), the function correctly recognizes that no actual text change occurred and preserves the earlier record that had different text. This keepsdidUserInputastrueand the popover stays visible.Testing Instructions
Automated
Manual
Enable RTC via Settings -> Writing -> "Enable real-time collaboration" checkbox.
Open a post collaboratively in two tabs.
In one session, begin continuously typing in a paragraph block. Here's a JS one-liner that can do that:
In the other session, click into a separate paragraph block and type
/.Verify the inline block inserter popover stays open while the other user is typing:
block-inserter-fix.mov
Select an option from the inserter and verify it inserts correctly:
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Used for: Root cause analysis and implementation