SD-2524 - feat: implement context menu for lists#2918
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
aba9ab9 to
58bffc3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b170c88c1b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@artem-harbour can you please prioritize reviewing this one today? thank you! |
|
@caio-pizzol, please check your comments and let us know if this is ready for merge. |
…atch Both commands set preventDispatch on the captured tr, expecting handleNumberingInvalidation to dispatch via editor.view?.dispatch. With no view that handler is a silent no-op and the captured tr is suppressed too, so no transaction flows and listRendering stays stale on the next read. These tests fail today and will pass once the dispatch path no longer skips when there is no view.
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @chittolinag! thanks for addressing last round.
a few things to land before merge, left them inline. also worth a pnpm test:layout run before merge.
| type MinimalWordLayout, | ||
| type ResolvedListMarkerGeometry, | ||
| } from '@superdoc/common/list-marker-utils'; | ||
| import { applySourceAnchorDataset } from '../renderer'; |
There was a problem hiding this comment.
this file and renderer.ts import each other (helper pulls applySourceAnchorDataset from renderer; renderer pulls createListMarkerElement from here). works now, fragile. move applySourceAnchorDataset into its own small file. while you're there, import type { SourceAnchor } since it's only used as a type.
| if (markerLayout?.run?.letterSpacing != null) { | ||
| markerEl.style.letterSpacing = `${markerLayout.run.letterSpacing}px`; | ||
| } | ||
| const markerContainer = createListMarkerElement(doc, markerLayout?.markerText ?? '', markerLayout?.run ?? {}); |
There was a problem hiding this comment.
flow markers carry source anchors via the helper now, but this call passes 3 args so table markers don't. snapshot consumers walking markers will skip them. any reason not to pass markerLayout?.sourceAnchor here, like the renderer.ts call at 3231?
| markerContainer.style.wordSpacing = '0px'; | ||
|
|
||
| const markerEl = doc.createElement('span'); | ||
| markerEl.classList.add('superdoc-paragraph-marker'); |
There was a problem hiding this comment.
since this PR is moving list-marker classes into DOM_CLASS_NAMES, superdoc-paragraph-marker is the obvious next one. fine as a follow-up.
…context-menu-on-list-markers-for
…re-support-right-click-context-menu-on-list-markers-for
…re-support-right-click-context-menu-on-list-markers-for
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.63 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.105 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.79 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.107 |
|
🎉 This PR is included in superdoc v1.30.0-next.61 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.61 |
|
🎉 This PR is included in superdoc-cli v0.9.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.32.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/mcp v0.4.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.3.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.4.0 |
Issue
When a user right-clicks a numbered list marker (e.g., "1.") in the document, the context menu shows only generic editor actions. There is no way to perform list-specific operations — restart numbering, continue numbering, or change indentation — from the marker itself. Left-clicking a marker is also not distinguished from clicking paragraph text, so no "marker selected" visual feedback is given.
Proposed solution
getEditorContext()to detect when a right-click lands on a.superdoc-list-markerelement and exposeisOnListMarkerin the context.list-markersection to the context menu with restart, continue, decrease-indent, and increase-indent actions, visible only whenisOnListMarkeris true.