fix(editor): add list numbering watcher#14414
fix(editor): add list numbering watcher#14414asehee wants to merge 5 commits intotoeverything:canaryfrom
Conversation
📝 WalkthroughWalkthroughNew Store extension Changes
Sequence DiagramsequenceDiagram
actor Store as ListStore
participant Ext as ListNumberingWatcherExtension
participant Cache as Cache
participant Blocks as affine:list Blocks
participant Txn as Store Transaction
Store->>Ext: loaded()
Ext->>Blocks: scan existing affine:list blocks
Ext->>Cache: cache next-sibling & type
alt Numbered List Added
Blocks->>Ext: blockAdded
Ext->>Cache: add cache entries
Ext->>Blocks: subscribe to propsUpdated
end
alt Type Changed (Numbered → Other)
Blocks->>Ext: propsUpdated (type changed)
Ext->>Cache: find next numbered sibling
Ext->>Txn: begin transaction
Txn->>Blocks: renumber subsequent numbered items
Txn->>Ext: commit
end
alt Numbered List Deleted
Blocks->>Ext: blockDeleted
Ext->>Cache: lookup next numbered item
Ext->>Txn: begin transaction
Txn->>Blocks: renumber from next item onward
Txn->>Ext: commit
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@blocksuite/affine/blocks/list/src/list-numbering-watcher.ts`:
- Around line 54-111: The propsUpdated handler inside _subscribeToModelChanges
duplicates the numbered→non-numbered renumbering logic already handled by the
blockUpdated 'update' handler (which uses correctNumberedListsOrderToPrev),
causing inconsistent behavior; remove the renumbering block in
_subscribeToModelChanges (the code that reads previousType, nextSiblingId,
fetches nextSibling, calls _getPrevNumberedSibling and wraps manual transact
renumbering) and instead only update caches (_previousTypeCache and
_nextSiblingCache) there, leaving all actual renumbering to the blockUpdated
'update' path (ensure _previousTypeCache is still set and _nextSiblingCache
updated when currentType === 'numbered').
- Around line 113-129: The _getPrevNumberedSibling function currently walks past
non-numbered siblings and can cross list-group boundaries; either remove this
unused method if duplicate-handler consolidation eliminated its callers, or
change its logic (inside _getPrevNumberedSibling using this.store.getPrev and
matchModels/ListBlockModel checks) to stop at the first sibling that is not a
numbered ListBlockModel (i.e., if a previous sibling exists but is not
matchModels(... && props.type === 'numbered'), return null instead of continuing
to the next previous sibling) so numbering never crosses group boundaries.
- Around line 223-265: The delete handler in ListNumberingWatcher leaves stale
entries in _nextSiblingCache for the previous sibling when a middle numbered
item is deleted; update the cache so previous sibling points to the deleted
item's next sibling (or null if none). In the delete branch that checks
payload.model, after determining the deletedIndex via
this.store.getParent(payload.model), find the previous sibling
(parent.children[deletedIndex - 1]) and set
this._nextSiblingCache.set(prevSibling, nextSiblingId ?? null); also handle the
case where a cache hit yields a missing block (getBlock(nextSiblingId) returns
undefined) by recomputing nextSiblingId from the parent and updating
_nextSiblingCache before deciding whether to call
correctNumberedListsOrderToPrev(this.store, nextSibling).
🧹 Nitpick comments (2)
blocksuite/affine/blocks/list/src/list-numbering-watcher.ts (2)
36-52: Non-numbered list items are not cached in_previousTypeCacheduring initialization.During
_initializeCache, onlynumbereditems have their type stored in_previousTypeCache(line 46). All other list types (bulleted, todo, toggle) are subscribed topropsUpdated(line 50) but their initial type isn't cached. This means_previousTypeCache.get(model)returnsundefinedfor these items until their first type change.Currently this is safe because the only check is
previousType === 'numbered', but it's fragile — a future change adding logic for other transitions would silently fail.Suggested fix
if (model.props.type === 'numbered') { const nextSibling = this.store.getNext(model); this._nextSiblingCache.set(model, nextSibling?.id ?? null); - this._previousTypeCache.set(model, 'numbered'); } + // Always cache the type so transitions from any type are tracked + this._previousTypeCache.set(model, model.props.type); // Subscribe to props changes for each list block this._subscribeToModelChanges(model);
7-20: Doc comment is incomplete — mentions only deletion, but the extension also handles type changes.The JSDoc (lines 7–20) describes only the deletion scenario, but the extension also renumbers when a list item's type changes from
numberedto a non-numbered type. Consider updating the comment to reflect both cases.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## canary #14414 +/- ##
==========================================
- Coverage 57.51% 56.98% -0.53%
==========================================
Files 2859 2860 +1
Lines 154468 154576 +108
Branches 23336 23253 -83
==========================================
- Hits 88845 88090 -755
- Misses 62720 63493 +773
- Partials 2903 2993 +90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
This PR adds a watcher extension to keep numbered list ordering consistent in list blocks.
Changes
ListNumberingWatcherExtensionatblocksuite/affine/blocks/list/src/list-numbering-watcher.ts.blocksuite/affine/blocks/list/src/store.ts.Behavior
The watcher updates numbering for affected continuous numbered siblings when:
numberedto a non-numbered typeImplementation Notes
affine:listevents only.Validation
Summary by CodeRabbit
New Features
Stability