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 - set selection on update for tiptap #979

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

adriangohjw
Copy link
Contributor

Problem

When users type in tiptap, the cursor will jump. This is caused by the useEffect we have which was necessary to ensure data is repopulated back when user dismisess the "are you sure you want to drop changes" modal

RE: https://opengovproducts.slack.com/archives/C07CWUNUL68/p1736426201036129

Solution

Breaking Changes

  • Yes - this PR contains breaking changes
    • Details ...
  • No - this PR is backwards compatible

Bug Fixes:

  • to note the cursor position and add it back

Tests

  1. on studio, create a Callout component with some text. Save the component
  2. open the component again. Put your cursor in the middle of the text and start typing - your cursor should not jump to the end of the text

@adriangohjw adriangohjw added the bug Something isn't working label Jan 9, 2025
@adriangohjw adriangohjw self-assigned this Jan 9, 2025
@adriangohjw adriangohjw requested a review from a team as a code owner January 9, 2025 17:57
@datadog-opengovsg
Copy link

datadog-opengovsg bot commented Jan 9, 2025

Datadog Report

Branch report: fix-tiptap-editor-selection-jumping
Commit report: 4e01566
Test service: isomer-studio

✅ 0 Failed, 257 Passed, 36 Skipped, 47.49s Total Time
➡️ Test Sessions change in coverage: 1 no change

Copy link
Contributor

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

i'm abit hesistant on this because it seems like we're adding alot of overhead that we must remember as useEffect is not very tightly coupled to the call site - should we try to find another solution?

// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
editor?.commands.setContent(data)
editor.commands.setContent(data, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

image

not sure if we need this second argument?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved in cb8ab4f

@adriangohjw
Copy link
Contributor Author

i'm abit hesistant on this because it seems like we're adding alot of overhead that we must remember as useEffect is not very tightly coupled to the call site - should we try to find another solution?

@seaerchin agree. perhaps i can add a ticket for that since this issue is quite a blocker to the site users since they cannot edit certain components

@adriangohjw adriangohjw merged commit 126c9f5 into main Jan 13, 2025
16 of 17 checks passed
@adriangohjw adriangohjw deleted the fix-tiptap-editor-selection-jumping branch January 13, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants