Rich Text Editor: Mark as supports read only#22600
Conversation
|
Claude finished @nielslyngsoe's task in 4m 0s —— View job Review Complete
The review has been posted as a PR comment. One Important issue was found: the pre-existing |
There was a problem hiding this comment.
Pull request overview
Marks the TipTap-based Rich Text Editor property editor UI as supporting read-only mode, so the backoffice won’t apply a blocking overlay and users can interact with the content in read-only scenarios.
Changes:
- Add
supportsReadOnly: trueto the TipTap RTE property editor manifest. - Hide the TipTap toolbar when the editor is in
readonlymode.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Umbraco.Web.UI.Client/src/packages/tiptap/property-editors/tiptap-rte/manifests.ts |
Declares TipTap RTE as supporting read-only at the manifest level. |
src/Umbraco.Web.UI.Client/src/packages/tiptap/components/input-tiptap/input-tiptap.element.ts |
Prevents toolbar rendering when readonly is true. |
There was a problem hiding this comment.
PR Review
Target: origin/main · Based on commit: 4ea6463a · Skipped: 0 noise files
Adds supportsReadOnly: true to the TipTap RTE manifest so the Umbraco framework delegates readonly handling to the component, and hides the toolbar when the editor is in readonly mode.
- Other changes: Users in readonly mode will now see the RTE rendered as editable content (via Tiptap's
editable: false) rather than being completely blocked by the framework. The toolbar is no longer rendered in readonly mode.
Important
input-tiptap.element.ts:306-312: The pre-existing CSS:host([readonly]) { pointer-events: none; }becomes active the momentsupportsReadOnly: truecauses the framework to passreadonlyto this component (previously the framework handled readonly at a higher level).pointer-events: noneon the host cascades into the shadow DOM, preventing mouse-initiated text selection — directly contradicting the stated goal of letting users copy content. The fix is to remove host-levelpointer-events: noneand scope it to interactive sub-elements only::host([readonly]) { #editor { background-color: var(--uui-color-surface-alt); pointer-events: auto; /* allow text selection */ } umb-tiptap-statusbar { pointer-events: none; } }
Suggestions
input-tiptap.element.ts:266-278(#renderStatusbar): PR description says "hides the status bar" but the actual diff hides the toolbar (#renderToolbar). The statusbar is still rendered with?readonly=${this.readonly}passed through. Consider also guarding#renderStatusbarwith|| this.readonlyif the intent is to hide it, or update the PR description.
Request Changes
The pointer-events: none on the host element prevents text selection/copy in readonly mode, which is the primary goal of this PR. Please address before merging.
- Remove `pointer-events: none` from `:host([readonly])` so users can select and copy text in read-only mode - Make the editor's editable state reactive to the `readonly` property via `setEditable` - Skip rendering the statusbar in read-only mode (mirrors the toolbar) to avoid the missing border-radius regression - Remove the now-unused `readonly` property from `umb-tiptap-toolbar` and `umb-tiptap-statusbar`
leekelleher
left a comment
There was a problem hiding this comment.
Tested out, works as described. Also addressed the feedback from Claude[bot] and GitHub Co-pilot.
Marks the TipTap Rich Text Editor as supporting read-only, as it does.
This enables users to copy content, which they should be allowed to in read-only mode.
This also hides the status bar, as that helps visually to indicate the read-only mode.
Test notes:
Make your use not able to edit a specific language, open a document with RTE on that Culture variant to see the read only mode.