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

Undo in Title or Tagline block positions caret at start of field #64724

Open
2 tasks done
ironprogrammer opened this issue Aug 22, 2024 · 10 comments
Open
2 tasks done

Undo in Title or Tagline block positions caret at start of field #64724

ironprogrammer opened this issue Aug 22, 2024 · 10 comments
Labels
[Block] Post Title Affects the Post Title Block [Block] Site Tagline Affects the Site Tagline Block [Type] Bug An existing feature does not function as intended

Comments

@ironprogrammer
Copy link
Contributor

Description

After Undo (Cmd+z) of a paste in the Title or Tagline block, the caret is (visually) positioned at the start of the field. However, subsequently pasted text is inserted at the end of the field, which is not expected based on caret position.

Expected behavior after Undo would be for:

  • The caret to be positioned where it was prior to the last "undid" action, instead of defaulting to the start of the field.
  • Pasted text to appear where the caret is positioned.

In testing a related issue, #64665, @Mamaduka also indicated:

...after digging, it turned out to be a general bug with the RichText component. The same behavior can be reproduced with Tagline block.

Problem

  • RichText only "knows" how to re-adjust selection after undoing for block attribute values.
  • When undoing entity properties, selection will point to the old start and end.

Step-by-step reproduction instructions

  1. Add a new post or page.
  2. Select the Title block and enter a title.
  3. Paste additional text into the end of the field.
  4. Undo the paste with Cmd+z/Ctrl+z (using the editor toolbar Undo button changes focus from the field, so is n/a).
  5. Observe the location of the caret after undo (🐛 caret is moved to start of field).
  6. Without clicking or repositioning the caret, re-paste into the field with Cmd+v/Ctrl+v.
  7. Observe where the pasted text is inserted (🐛 text is inserted at end of field).

Screenshots, screen recording, code snippet

title-paste-caret-front
Demonstration of pasting text and resultant caret position. After Undo of paste caret appears to be at front but pastes at the end of the field.

Environment info

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@ironprogrammer ironprogrammer added [Type] Bug An existing feature does not function as intended [Block] Post Title Affects the Post Title Block [Block] Site Tagline Affects the Site Tagline Block labels Aug 22, 2024
@Kallyan01
Copy link

I would like to work on this issue !

@Mamaduka
Copy link
Member

@Kallyan01, feel free to give it a try.

@Kallyan01
Copy link

I think to solve this issue efficiently and handle all the edge cases , we need to maintain a undo , redo stack where we can store the caret position !

@Mamaduka @ironprogrammer let me know if i am thinking in the right direction !

@Mamaduka
Copy link
Member

Mamaduka commented Dec 5, 2024

@Kallyan01, the editor maintains the undo/redo stack for this data, but for block attributes, it also maintains a selection(start|end), which is later used by the RichText component to determine the correct caret position.

I think we need to store similar selection data for entity properties like Title and Tagline.

@Kallyan01
Copy link

@Mamaduka @ironprogrammer i've raised the PR #68005 , can you have a look !

@Mamaduka
Copy link
Member

@Kallyan01, thank you for your efforts on this!

I appreciate the proposed solution, but I believe exploring a more general resolution to the issue would be beneficial.

#68005 tries to resolve the issue locally for the PostTitle component by mimicking global history and keeping a local copy, which has a couple of downsides:

  • The same logic must be replicated in every component affected by the same bug - Site Title, Post Title, Tagline, etc.
  • Global and local history can diverge when entity property is updated by different means.
  • I see it has keyboard shortcut handlers, but I'm not sure how it handles Undo/Redo actions dispatched from the top toolbar or directly via the store.

cc @ellatrix, @youknowriad, what do you think?

Related #37171.

@ellatrix
Copy link
Member

Site title etc. should store selection similar to how useEntityBlockEditor stores selection in the editEntityRecord edits

@ellatrix
Copy link
Member

ellatrix commented Dec 18, 2024

I'm guessing you'd also want to introduce a onSelectionChange for RichText to override the internal one (that shouldn't be too hard). And allow selection to be passed (instead of RichText trying to get it from the content).

@Kallyan01
Copy link

@Mamaduka Thank you for the feedback! I understand your concerns regarding the replication of logic across multiple components and the potential divergence of global and local histories.

If we want to maintain the caret logic within Gutenberg, keeping the history globally might not offer significant benefits, as those histories are specific to a single component. Thus, encapsulating the logic within each component itself seems to make sense in this context.

However, an alternative approach could be to integrate this logic directly into @wordpress/rich-text via the useRichText() hook. This would centralize the behaviour and potentially reduce duplication across components.

@Kallyan01
Copy link

Kallyan01 commented Dec 21, 2024

@Mamaduka

@Mamaduka Thank you for the feedback! I understand your concerns regarding the replication of logic across multiple components and the potential divergence of global and local histories.

If we want to maintain the caret logic within Gutenberg, keeping the history globally might not offer significant benefits, as those histories are specific to a single component. Thus, encapsulating the logic within each component itself seems to make sense in this context.

However, an alternative approach could be to integrate this logic directly into @wordpress/rich-text via the useRichText() hook. This would centralize the behaviour and potentially reduce duplication across components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Title Affects the Post Title Block [Block] Site Tagline Affects the Site Tagline Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

4 participants