SD-1743: Make link in viewing mode non-editable (incl. tests)#1951
SD-1743: Make link in viewing mode non-editable (incl. tests)#1951johanneswilm wants to merge 3 commits intosuperdoc-dev:mainfrom
Conversation
![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: ea1237a20d
ℹ️ 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".
| placeholder="Type or paste a link" | ||
| :class="{ error: urlError }" | ||
| v-model="rawUrl" | ||
| :readonly="isViewingMode" | ||
| @keydown.enter.stop.prevent="handleSubmit" |
There was a problem hiding this comment.
In viewing mode the inputs are marked readonly, but the @keydown.enter handlers still call handleSubmit. Because the popover focuses an input on mount, a user pressing Enter can still invoke toggleLink/unsetLink and dispatch a transaction, which defeats the “non-editable in viewing mode” behavior and can mutate the document. This happens whenever the popover is opened in viewing mode and Enter is pressed, even without changing the fields. Consider disabling the Enter handler or short-circuiting handleSubmit when isViewingMode is true.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Implemented as suggested, even though it doesn't seem that it should be possible to change anything when in readonly mode anyway
There was a problem hiding this comment.
Thanks @johanneswilm ! Actually it seems the bigger issue here is in LinkInput.vue:225-225
When NOT in viewing mode, we used to be able to type a url in the popover and hit 'enter' -> this is now broken in this branch. Mind fixing that behavior?
Some more possibly helpful context: _ The new keydown binding uses !isViewingMode && handleSubmit, which in Vue inline handlers only evaluates to a function reference and does not call it. As a result, pressing Enter in the text or URL field no longer submits link changes in
editing/suggesting modes, regressing keyboard-based link editing behavior. This should call the handler (for example !isViewingMode && handleSubmit()) while still guarding viewing mode._
There was a problem hiding this comment.
@harbournick Thanks for catching that! I tried adding parenthesis and a few other things, but it would not work. But given that we already cehck in the handleSubmit function itself, there isn't really a need to also check in the vue inline event handler.
There was a problem hiding this comment.
Pull request overview
This PR implements read-only behavior for link inputs when the editor is in viewing mode, preventing users from editing link properties while still allowing them to view link details and open links.
Changes:
- Added
isViewingModecomputed property to detect when the editor is in viewing mode - Made text and URL inputs readonly in viewing mode with appropriate styling
- Updated the title to show "Link details" instead of "Edit link" in viewing mode
- Hidden Apply and Remove buttons in viewing mode
- Added comprehensive test coverage for viewing mode behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/super-editor/src/components/toolbar/LinkInput.vue | Implements viewing mode detection, applies readonly attribute to inputs, conditionally hides edit buttons, updates title text, and adds CSS styling for readonly state |
| packages/super-editor/src/components/toolbar/LinkInput.test.js | Adds comprehensive test suite covering viewing mode detection, UI changes (titles, readonly inputs, hidden buttons), and verification that viewing mode doesn't affect editing mode behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@harbournick Fixed |
2 participants
No description provided.