feat(super-editor): add w:lock support for StructuredContent nodes#1939
feat(super-editor): add w:lock support for StructuredContent nodes#1939
Conversation
|
Based on my review of the code and my knowledge of the ECMA-376 specification, I can provide a detailed review: Status: FAIL The implementation has spec violations in the Issues Found1. Invalid enumeration values (Critical)Files:
Problem: The code uses According to ECMA-376 Part 1 §17.5.2.20 (CT_Lock), the
Spec violation: When an SDT is unlocked, the 2. Incorrect default handling (Major)File: Problem: The code defaults missing const lockMode = validModes.includes(lockValue) ? lockValue : 'unlocked';Correct behavior: When 3. Correct export logic (passes validation)File: Good news: The export logic correctly omits the if (attrs.lockMode && attrs.lockMode !== 'unlocked') resultElements.push(lock);This partially compensates for issue #1, but the internal representation still uses an invalid spec value. Recommendations
ReferenceSee ECMA-376 Part 1 §17.5.2.20 for full specification: https://ooxml.dev/spec?q=w:lock The implementation is close but needs the "unlocked" value removed from the internal data model to fully comply with OOXML. |
Implement ECMA-376 §17.5.2.23 w:lock support for StructuredContent and StructuredContentBlock nodes. This enables template variables to enforce read-only behavior based on lock modes. Lock modes: - unlocked: no restrictions (default) - sdtLocked: SDT wrapper cannot be deleted, content editable - contentLocked: content read-only, SDT can be deleted - sdtContentLocked: fully locked (wrapper and content) Changes: - Add lockMode attribute to StructuredContent/Block extensions - Parse w:lock element on DOCX import - Export w:lock element on DOCX save - Add lock enforcement plugin (prevents deletion of locked SDTs) - Add NodeView methods for content editability - Add visual styling matching Word's appearance (presentation mode) - Add TypeScript types for lock modes - Add unit tests for import, export, and lock behavior
tupizz
force-pushed
the
feat/sdt-lock-support
branch
from
0234ac9 to
3e5377e
Compare
Replace state.doc.descendants() with nodesBetween() to avoid iterating the entire document on every transaction. Now only checks nodes within the affected ranges. Also simplify normalizeLockMode in style-engine since lockMode values are already validated at import time.
…s toggling Remove isSdtLocked() method that was never called - SDT deletion prevention is handled by the lock plugin instead. Remove updateLockStateClasses() and its calls - the CSS classes it toggled had no corresponding CSS rules. Presentation mode uses data-lock-mode attributes with CSS in styles.ts instead.
Change lock enforcement strategy to use plugin-only defense instead of contentEditable='false'. This allows users to: - Move cursor within locked content nodes - Select text for copying - Navigate smoothly through the document The lock plugin now handles all edit blocking through: - handleKeyDown: Block Delete/Backspace/Cut before transaction - handleTextInput: Block typing in content-locked nodes - filterTransaction: Safety net for paste, drag-drop, programmatic changes NodeView now only adds CSS classes for visual feedback without disabling cursor interaction. Also adds comprehensive test suite with 35 tests covering all lock modes and adds research documentation in .tupizz/docs/.
tupizz
force-pushed
the
feat/sdt-lock-support
branch
from
270cf66 to
fb4a38c
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive support for ECMA-376 w:lock functionality to StructuredContent nodes, enabling four lock modes (unlocked, sdtLocked, contentLocked, sdtContentLocked) that control whether the SDT wrapper can be deleted and whether the content can be edited. The implementation uses a three-layer defense strategy in the editor, full round-trip import/export support, and visual styling that differentiates lock modes.
Changes:
- Added
lockModeattribute to StructuredContent and StructuredContentBlock nodes with DOM parsing/rendering - Implemented lock enforcement plugin with handleKeyDown, handleTextInput, and filterTransaction hooks
- Added DOCX import parsing and export generation for w:lock elements
- Implemented visual styling with distinct background colors for each lock mode
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/super-editor/src/extensions/types/node-attributes.ts | Added StructuredContentLockMode type definition |
| packages/super-editor/src/extensions/structured-content/structured-content.js | Added lockMode attribute and registered lock plugin |
| packages/super-editor/src/extensions/structured-content/structured-content-block.js | Added lockMode attribute definition |
| packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js | Implemented three-layer lock enforcement with step relationship analysis |
| packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js | Comprehensive test suite with 35 tests covering all lock modes and scenarios |
| packages/super-editor/src/extensions/structured-content/StructuredContentViewBase.js | Added lock detection and CSS class application methods |
| packages/super-editor/src/extensions/structured-content/StructuredContentInlineView.js | Integrated updateContentEditability in view lifecycle |
| packages/super-editor/src/extensions/structured-content/StructuredContentBlockView.js | Integrated updateContentEditability in view lifecycle |
| packages/super-editor/src/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.js | Added w:lock parsing with validation of lock mode values |
| packages/super-editor/src/core/super-converter/v3/handlers/w/sdt/helpers/handle-structured-content-node.test.js | Tests for parsing all lock mode values and defaults |
| packages/super-editor/src/core/super-converter/v3/handlers/w/sdt/helpers/translate-structured-content.js | Added w:lock export generation and deduplication |
| packages/super-editor/src/core/super-converter/v3/handlers/w/sdt/helpers/translate-structured-content.test.js | Tests for exporting lock modes and preventing duplication |
| packages/layout-engine/style-engine/src/index.ts | Added lockMode to normalized StructuredContentMetadata |
| packages/layout-engine/painters/dom/src/utils/sdt-helpers.ts | Added lockMode data attribute rendering in SDT containers |
| packages/layout-engine/painters/dom/src/renderer.ts | Added lockMode to dataset attributes and rendering |
| packages/layout-engine/painters/dom/src/styles.ts | Added CSS styles for each lock mode with hover effects |
| packages/layout-engine/contracts/src/index.ts | Added StructuredContentLockMode type to contracts |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Lock mode styles for structured content - matches Word appearance exactly */ | ||
| /* Default: background color only, no border. Border appears on hover/focus */ | ||
| /* unlocked: light mint green - fully editable and deletable */ | ||
| .superdoc-structured-content-block[data-lock-mode="unlocked"], | ||
| .superdoc-structured-content-inline[data-lock-mode="unlocked"] { | ||
| background-color: #e6f4ea; | ||
| border: 1px solid transparent; | ||
| } | ||
| /* sdtLocked: golden yellow - SDT cannot be deleted but content can be edited */ | ||
| .superdoc-structured-content-block[data-lock-mode="sdtLocked"], | ||
| .superdoc-structured-content-inline[data-lock-mode="sdtLocked"] { | ||
| background-color: #fff3cd; | ||
| border: 1px solid transparent; | ||
| } | ||
| /* contentLocked: light blue/lavender - content is read-only but SDT can be deleted */ | ||
| .superdoc-structured-content-block[data-lock-mode="contentLocked"], | ||
| .superdoc-structured-content-inline[data-lock-mode="contentLocked"] { | ||
| background-color: #e8f0f8; | ||
| border: 1px solid transparent; | ||
| } | ||
| /* sdtContentLocked: light peach/salmon - fully locked */ | ||
| .superdoc-structured-content-block[data-lock-mode="sdtContentLocked"], | ||
| .superdoc-structured-content-inline[data-lock-mode="sdtContentLocked"] { | ||
| background-color: #ffe8e0; | ||
| border: 1px solid transparent; | ||
| } | ||
| /* Show blue border on hover for all lock modes */ | ||
| .superdoc-structured-content-block[data-lock-mode]:hover, | ||
| .superdoc-structured-content-inline[data-lock-mode]:hover { | ||
| border-color: #629be7; | ||
| } |
There was a problem hiding this comment.
CSS specificity conflict: The new lock mode styles may conflict with existing hover styles. For inline structured content, line 423 sets border: 1px solid #629be7 and line 430-432 sets hover styles with background-color and border-color. The new lock mode styles at lines 459-485 set border: 1px solid transparent which will override the base blue border, and the hover styles at lines 488-491 only set border-color without setting background-color.
This means:
- For inline SDTs without lock mode data attribute, the existing blue border and hover background will work correctly
- For inline SDTs with lock mode data attributes, the transparent border will replace the blue border, and on hover, only the border color changes but the lock mode background color persists
This may not be the intended behavior. Consider whether lock mode styles should only apply to block-level SDTs, or if the inline styles need to be adjusted to work harmoniously with the existing hover behavior. The PR description mentions "matching Word's behavior" but it's unclear if Word shows these lock mode colors for inline SDTs or only for block SDTs.
| // Assert: check if SDT still exists (boundary crossing damages wrapper) | ||
| const sdtStillIntact = sdtNodeExists(newState.doc, 'structuredContent'); |
There was a problem hiding this comment.
Unused variable sdtStillIntact.
| // Assert: check if SDT still exists (boundary crossing damages wrapper) | |
| const sdtStillIntact = sdtNodeExists(newState.doc, 'structuredContent'); | |
| // Assert: check whether the document content changed |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb4a38c0cd
ℹ️ 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".
| const sdtNodes = collectSDTNodes(state.doc); | ||
| if (sdtNodes.length === 0) { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
The filterTransaction guard collects SDT ranges from the pre-transaction document, but each step.from/step.to is expressed in the document after prior steps have applied. In multi-step transactions (e.g., a programmatic command that inserts before an SDT and then deletes inside it), the later step’s coordinates will be shifted while the SDT ranges are not, so checkLockViolation can miss a deletion inside locked content. To keep the safety net accurate, map step positions back to the original doc or map SDT ranges forward for each step (e.g., via tr.mapping).
Useful? React with 👍 / 👎.
1 participant
Summary
Add ECMA-376
w:locksupport forStructuredContent(inline) andStructuredContentBlock(block) nodes, enabling template variables to be read-only per the OOXML specification (§17.5.2.23).Lock Modes
unlockedsdtLockedcontentLockedsdtContentLockedVisual Comparison (Word vs SuperDoc)
Border behavior: Blue border (#629be7) appears on hover only, matching Word's behavior.
Architecture Decisions
1. Plugin-Only Defense (No
contentEditable='false')Decision: Block edits through the lock plugin instead of setting
contentEditable='false'on locked content.Why: Setting
contentEditable='false'completely prevents cursor placement inside the node. This is poor UX because users:Solution: Keep content editable at the DOM level and rely on the plugin to block actual edits. Users can move cursor and select text freely while being prevented from modifying content.
2. Three-Layer Defense Strategy
The lock plugin uses three mechanisms to enforce locks:
handleKeyDownhandleTextInputfilterTransactionWhy
handleKeyDowninstead of justfilterTransaction?ProseMirror's transaction flow is:
When
filterTransactionblocks a transaction, the browser may have already modified DOM selection, causing the cursor to jump unexpectedly. By intercepting inhandleKeyDownand callingevent.preventDefault(), we prevent the browser from processing the event at all.3. Step Relationship Analysis
To detect lock violations, we analyze the geometric relationship between a step's affected range and each SDT node:
4. NodeView Provides Visual Feedback Only
The NodeView adds CSS classes for styling but does NOT set
contentEditable:Changes
Import/Export
w:lockelement fromw:sdtPron DOCX importw:lockelement tow:sdtPron DOCX saveEditor Behavior
lockModeattribute toStructuredContentandStructuredContentBlockextensionsstructuredContentLockPluginwith 3-layer defense:handleKeyDown: Block Delete/Backspace/Cut before transactionhandleTextInput: Block typing in content-locked nodesfilterTransaction: Safety net for programmatic changesVisual Styling (Presentation Mode)
StructuredContentLockModetype to contractslockModetoStructuredContentMetadatain style-enginedata-lock-modeattribute rendering in DomPainterDocumentation
.tupizz/docs/prosemirror-lock-plugin-research.mdTest Coverage
35 tests covering all lock modes and operations:
Files Changed
contractssrc/index.ts- AddStructuredContentLockModetypestyle-enginesrc/index.ts- AddlockModenormalizationpainter-domrenderer.ts,styles.ts,sdt-helpers.ts- Render lock mode stylingsuper-editorTest Plan
Related
Demo
CleanShot.2026-02-04.at.15.53.52.mp4