superlistapp / super_editor

A Flutter toolkit for building document editors and readers
https://superlist.com/SuperEditor/
MIT License
1.67k stars 245 forks source link

[BUG] - Flutter Web (Undo) - CMD+Z not working properly on text-node created after non-text node #2164

Closed rafalplonka closed 2 months ago

rafalplonka commented 3 months ago

Package Version super_editor, GitHub, stable branch

User Info Knips (https://knips.io/) - We are working on replacing our Block Text Editor with the Super Editor (not released yet). The editor will be used to create posts on our platform.

To Reproduce Steps to reproduce the behavior:

  1. Run the example app
  2. Place caret on the image (first block)
  3. Click enter
  4. Start typing
  5. Press CMD+Z to undo
  6. Text gets duplicated (without the removed char)

Minimal Reproduction Code The bug is present in the demo app.

Actual behaviour The text gets duplicated after pressing CMD+Z when creating a text node after a non-text node.

It works correctly when pressing enter after the text node.

The issue is probably connected to the difference between these two scenarios in the common_editor_operations.dart file, method: insertBlockLevelNewline:

   else if (extentNode is ParagraphNode) {
      // Split the paragraph into two. This includes headers, blockquotes, and
      // any other block-level paragraph.
      final currentExtentPosition = composer.selection!.extent.nodePosition as TextNodePosition;
      final endOfParagraph = extentNode.endPosition;

      editorOpsLog.finer("Splitting paragraph in two.");
      editor.execute([
        SplitParagraphRequest(
          nodeId: extentNode.id,
          splitPosition: currentExtentPosition,
          newNodeId: newNodeId,
          replicateExistingMetadata: currentExtentPosition.offset != endOfParagraph.offset,
        ),
        // Place the caret at the beginning of the new node.
        ChangeSelectionRequest(
          DocumentSelection.collapsed(
            position: DocumentPosition(
              nodeId: newNodeId,
              nodePosition: const TextNodePosition(offset: 0),
            ),
          ),
          SelectionChangeType.insertContent,
          SelectionReason.userInteraction,
        ),
      ]);
    } else if (composer.selection!.extent.nodePosition is UpstreamDownstreamNodePosition) {
      final extentPosition = composer.selection!.extent.nodePosition as UpstreamDownstreamNodePosition;
      if (extentPosition.affinity == TextAffinity.downstream) {
        // The caret sits on the downstream edge of block-level content. Insert
        // a new paragraph after this node.
        editorOpsLog.finer("Inserting paragraph after block-level node.");
        editor.execute([
          InsertNodeAfterNodeRequest(
            existingNodeId: extentNode.id,
            newNode: ParagraphNode(
              id: newNodeId,
              text: AttributedText(),
            ),
          ),
          // Place the caret at the beginning of the new node.
          ChangeSelectionRequest(
            DocumentSelection.collapsed(
              position: DocumentPosition(
                nodeId: newNodeId,
                nodePosition: const TextNodePosition(offset: 0),
              ),
            ),
            SelectionChangeType.insertContent,
            SelectionReason.userInteraction,
          ),
        ]);
      }

Expected behavior Regular undo behaviour

Platform Web

Flutter version [3.22.2](https://github.com/flutter/flutter/releases/tag/3.22.

matthew-carroll commented 2 months ago

Thanks for filing this. I'm looking into it.

BTW, unrelated to this issue - we like to promote apps that are using Super Editor. Would you like us to feature your logo at flutterbountyhunters.com when you launch with Super Editor?

thanhhai08sk commented 2 months ago

Thanks for filing this. I'm looking into it.

BTW, unrelated to this issue - we like to promote apps that are using Super Editor. Would you like us to feature your logo at flutterbountyhunters.com when you launch with Super Editor?

Please consider including my app journalit.app too. Thanks πŸ™

matthew-carroll commented 2 months ago

@thanhhai08sk happy to include you. Can you provide a URL where I can find a logo for your company? We display company logos in our website header at flutterbountyhunters.com

matthew-carroll commented 2 months ago

WRT this ticket, upon investigation, the root problem here is the retention of a mutable node. This is essentially the same type of problem that I just solved with regard to paste functionality with undo/redo: https://github.com/superlistapp/super_editor/pull/2190

I'll solve that same problem for the commands related to this ticket, too.

The broader solution is to convert all DocumentNodes to immutable data structures, which is ongoing work here: https://github.com/superlistapp/super_editor/issues/2166

rafalplonka commented 2 months ago

Thanks for filing this. I'm looking into it.

BTW, unrelated to this issue - we like to promote apps that are using Super Editor. Would you like us to feature your logo at flutterbountyhunters.com when you launch with Super Editor?

Confirming that the behaviour works correctly after the fix on the PR. πŸ‘

Thank you for reaching out. We would be happy to have our logo on flutterbountyhunters.com. 😁 We will let you know when we're live with the SuperEditor.

Link to the logo