Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(extension-link): Click handler opens selected link instead of clicked link #3732

Merged

Conversation

jmtaber129
Copy link
Contributor

Description

Fixes a bug in which under certain conditions where multiple links are present, clicking one link will instead open a different link. Specifically, when the editor state shows one link as selected, clicking a different link will open the "selected" link instead.

See also ParabolInc/parabol#7524 where this issue was initially reported for our web app.

Repro

The specific conditions in which editor state does not reflect the clicked link are:

  • openOnClick is true
  • Content is read-only - editable is false

As such, I recommend reviewers test with the following patch applied:

diff --git a/demos/src/Marks/Link/React/index.jsx b/demos/src/Marks/Link/React/index.jsx
index 53b3134a9..4cfb9b4f3 100644
--- a/demos/src/Marks/Link/React/index.jsx
+++ b/demos/src/Marks/Link/React/index.jsx
@@ -16,7 +16,7 @@ export default () => {
       Text,
       Code,
       Link.configure({
-        openOnClick: false,
+        openOnClick: true,
       }),
     ],
     content: `
@@ -27,6 +27,8 @@ export default () => {
           By default every link will get a <code>rel="noopener noreferrer nofollow"</code> attribute. It’s configurable though.
         </p>
       `,
+
+    editable: false,
   })
 
   const setLink = useCallback(() => {

With this patch applied, you can reproduce locally with the following steps:

After applying the changes in this PR, the Cmd+click on "another one" will open the correct link, https://statamic.com/

@netlify
Copy link

netlify bot commented Feb 9, 2023

Deploy Preview for tiptap-embed ready!

Name Link
🔨 Latest commit 5c6292a
🔍 Latest deploy log https://app.netlify.com/sites/tiptap-embed/deploys/63e4e20bff53d70008f7044e
😎 Deploy Preview https://deploy-preview-3732--tiptap-embed.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jmtaber129
Copy link
Contributor Author

@bdbch mind taking a look at this small bugfix when you have a chance? This is causing a P2 bug over in https://github.com/ParabolInc/parabol (ParabolInc/parabol#7524). Thanks!

@bdbch
Copy link
Contributor

bdbch commented Feb 18, 2023

@jmtaber129 sorry for the wait. Looking into it right now!

Copy link
Contributor

@bdbch bdbch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bdbch bdbch merged commit 6997bca into ueberdosis:main Feb 18, 2023
@jmtaber129 jmtaber129 deleted the jmtaber129/fix-incorrect-link-opening branch February 20, 2023 11:16
aliasliao pushed a commit to aliasliao/tiptap that referenced this pull request May 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants