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
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/.
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.
packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.test.js
Show resolved
Hide resolved
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".
packages/super-editor/src/extensions/structured-content/structured-content-lock-plugin.js
Show resolved
Hide resolved
…lity - Introduced SdtGroupedHover class to manage hover states for multi-fragment SDT blocks, allowing simultaneous highlighting of all fragments. - Updated shouldRebuildForSdtBoundary function to improve checks for SDT boundary changes, ensuring stale attributes are removed and boundaries are correctly validated. - Adjusted DOM rendering logic to incorporate new hover functionality and boundary checks. - Modified styles for SDT container labels to improve visibility and interaction during hover states.
# Conflicts: # packages/layout-engine/painters/dom/src/renderer.ts
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).Also adds grouped hover for multi-fragment block SDTs, stale fragment rebuild detection, and SDT utility extraction for better code organization.
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 DomPainterGrouped Hover for Multi-Fragment Block SDTs
When a block SDT spans multiple paragraphs, each renders as a separate DOM fragment. Previously, CSS
:hoveronly highlighted the individual fragment under the cursor. Now all fragments of the same SDT highlight simultaneously via JavaScript event delegation:SdtGroupedHoverclass (utils/sdt-hover.ts): Event delegation on mount container usingmouseover/mouseleave. Finds all fragments sharing the samedata-sdt-idand toggles.sdt-hoverCSS class on all of them..sdt-hoverclass.SdtGroupedHover.reapply()is called at the end ofupdateVirtualWindow()to restore the class on all matching fragments.:hoverto.sdt-hoverselector. Inline SDTs remain:hover-based (single element, no grouping needed).Stale Fragment Rebuild Detection
shouldRebuildForSdtBoundary(moved toutils/sdt-helpers.ts): Detects when a fragment's SDT boundary attributes (data-sdt-container-start/end) are stale and need rebuilding. Now correctly handles the case where a fragment leaves an SDT group (attributes present but should be removed).mappingUnreliablecheck: After a full-document paste of identical content, ProseMirror's transaction mapping becomes degenerate — it maps all old positions to the insertion end. This corrupteddata-pm-start/data-pm-endattributes on reused spans, breaking cursor/click navigation. The new check verifiesmapping.map(oldPmStart) === fragment.pmStartand forces a full fragment rebuild when the mapping is unreliable.Code Organization
SdtGroupedHoverclass from renderer.ts intoutils/sdt-hover.tsshouldRebuildForSdtBoundaryfrom renderer.ts intoutils/sdt-helpers.tsTest Coverage
35 tests covering all lock modes and operations:
Files Changed
contractssrc/index.ts— AddStructuredContentLockModetypestyle-enginesrc/index.ts— AddlockModenormalizationpainter-domrenderer.ts— Grouped hover binding, post-render reapply, mapping reliability checkpainter-domstyles.ts— Block SDT hover uses.sdt-hoverclass, updated label stylingpainter-domutils/sdt-helpers.ts— AddshouldRebuildForSdtBoundary, fix stale attribute handlingpainter-domutils/sdt-hover.ts— NewSdtGroupedHoverclass for multi-fragment hoversuper-editorlockModeattribute for inline and block nodessuper-editorsuper-editorstructured-content-lock-plugin.js(169 lines)super-editorstructured-content-lock-plugin.test.js(416 lines, 35 tests)super-editorw:lockelementsuper-editorsuper-editornode-attributes.ts— AddlockModeto type interfaceCommit History
3e5377e7d04f6c6anodesBetween()on changed ranges instead of fulldescendants()786e5b77nodesBetweenrange to valid document bounds9137b9c5isSdtLocked()method andupdateLockStateClasses()(no CSS rules existed)fb4a38c0contentEditable=falseto plugin-only defense for cursor movement UX0646a2f4Test Plan
Related
Demo
CleanShot.2026-02-04.at.15.53.52.mp4