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

Improved link editor UX #4026

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

ChronicLynx
Copy link
Contributor

@ChronicLynx ChronicLynx commented Mar 3, 2023

Summary of Changes:

  • Toggling editMode results in less jarring transition by attempting to reconcile height differences between elements.
  • Reduced link-input width to allow for new buttons controlling submission and cancellation (in addition to keyboard commands).
  • Escape key now exits without saving the link changes, Enter continues to submit.
  • Disabled the ability to open the link when in the editor. Prior to this you could accidentally open the link when trying to move your cursor on the text by clicking. You can still click the link from the link-editor itself to test it. The link will still become clickable if you try to export it elsewhere because this only applies to things nested under ContentEditable__root.
  • link-editor is flex.
  • Split the state of linkUrl to not immediately register the changes until a submit event.
  • Created new functions to handle key presses and submission.
  • Removed background highlighting for initial view. This implied it was a clickable area, but usually resulted in you accidentally opening the link.
  • New app settings to disable LinkPreview by default

LinkPreview

Because LinkPreview is not completely configured in the Playground and uses a Suspense it causes weird visual glitches as the component changes dimensions for a brief moment until deciding preview is null and it should not display anything. The setting to disable LinkPreview from attempting to render when it will 404 (because there's no endpoint) should avoid this issue.


Demo:

Screen.Recording.2023-03-03.at.3.57.35.PM.mov

Issues:

This problem is prior to my current changes. Removing this code fixes it, but might reintroduce the original problem.

#4013: You cannot click and drag to highlight text anymore @LuciNyan

useEffect(() => {
    function mouseMoveListener(e: MouseEvent) {
      if (editorRef?.current && (e.buttons === 1 || e.buttons === 3)) {
        editorRef.current.style.pointerEvents = 'none';
      }
    }
    function mouseUpListener(e: MouseEvent) {
      if (editorRef?.current) {
        editorRef.current.style.pointerEvents = 'auto';
      }
    }

    if (editorRef?.current) {
      document.addEventListener('mousemove', mouseMoveListener);
      document.addEventListener('mouseup', mouseUpListener);

      return () => {
        document.removeEventListener('mousemove', mouseMoveListener);
        document.removeEventListener('mouseup', mouseUpListener);
      };
    }
  }, [editorRef]);

@vercel
Copy link

vercel bot commented Mar 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
lexical ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 10:51PM (UTC)
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 3, 2023 at 10:51PM (UTC)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 3, 2023
Copy link
Contributor

@acywatson acywatson left a comment

Choose a reason for hiding this comment

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

Love it. Thanks!

@thegreatercurve
Copy link
Contributor

I think these changes are okay. Removing the ability to drag and drop the Lexical Nodes by anything other than the handle seems like a sensible change, as it's causing a related issue here (#4028) when combined with the additional mouse listener introduced in #4013.

I've tried to reproduce the original glitching issues from #4028 though by checking out this PR, and the old bug does not seem to be present, so I think this is fine to merge.

Closes #4028

@thegreatercurve thegreatercurve merged commit f161f67 into facebook:main Mar 6, 2023
@acywatson acywatson mentioned this pull request Mar 8, 2023
@zurfyx zurfyx mentioned this pull request Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants