feat: delete key functionality #272

MayurSMahajan · 2023-07-01T08:24:41Z

Solves: #335 and partially solves: #2888

Added functionalities:

Delete the right character in the text.
Delete a selected node.
At the end of the line, pressing delete merges the next line into the current line.
At the end of the line, pressing delete merges the next list node or checkbox node.

codecov · 2023-07-01T08:30:53Z

Codecov Report

Merging #272 (0e53201) into main (46158d9) will increase coverage by 0.06%.
The diff coverage is 86.95%.

@@            Coverage Diff             @@
##             main     #272      +/-   ##
==========================================
+ Coverage   70.30%   70.36%   +0.06%     
==========================================
  Files         238      239       +1     
  Lines        9893     9935      +42     
==========================================
+ Hits         6955     6991      +36     
- Misses       2938     2944       +6

Impacted Files	Coverage Δ
lib/src/core/document/text_delta.dart	`95.10% <ø> (ø)`
lib/src/extensions/node_extensions.dart	`90.00% <66.66%> (-6.08%)`	⬇️
...rtcuts/command_shortcut_events/delete_command.dart	`93.75% <93.75%> (ø)`
...tor/block_component/standard_block_components.dart	`100.00% <100.00%> (ø)`
...ts/command_shortcut_events/select_all_command.dart	`93.75% <100.00%> (ø)`

LucasXu0 · 2023-07-03T02:30:37Z

lib/src/editor/editor_component/service/shortcuts/command_shortcut_events/delete_command.dart

+  }
+
+  final position = selection.start;
+  final node = editorState.getNodeAtPath(position.path);


final delta = node?.delta; if (node == null || delta == null) { return } // use `delta` here instead of `node.delta!`

LucasXu0 · 2023-07-03T02:34:30Z

lib/src/editor/editor_component/service/shortcuts/command_shortcut_events/delete_command.dart

+      return KeyEventResult.ignored;
+    }
+    //TODO(Lucas): add logic for merging a bulletList or numberedList item.
+    if (nextNode.next == null && nextNode.children.isEmpty) {


Why did you use nextNode.next here?

Can you list all the cases so that I can implement it more effectively?

I wrote this because in the backspace_command.dart we check the children and next node of a node when we want to append that node to the previous node. I guess it is a bit confusing. I tried to basically duplicate the behavior of the backspace_command.dart.

In backspace handler, when we are at the start of a line and we press backspace, we want the current line to merge with the previous line.
Eg:
Welcome to AppFlowy
|Welcome to AppFlowy
->
Welcome to AppFlowy | Welcome to AppFlowy

In delete handler, when we are at the end of a line and we press a delete, we want the next line to merge with the current line.

Eg:
Welcome to AppFlowy|
Welcome to AppFlowy
->
Welcome to AppFlowy | Welcome to AppFlowy

LucasXu0 · 2023-07-03T02:36:56Z

test/new/service/shortcuts/command_shortcut_events/delete_command_test.dart

+      // Before
+      // Welcome to AppFlowy Editor 🔥!|
+      //    Welcome to AppFlowy Editor 🔥!
+      // After
+      // Welcome to AppFlowy Editor 🔥!|Welcome to AppFlowy Editor 🔥!
+      //
+      //   test('''Delete the collapsed selection's next node when cursor is at end
+      //       and current node contains a delta
+      //       and the next node is the child of the current node''', () async {
+      //     final document = Document.blank().addParagraph(
+      //       initialText: text,


Why comment out this code?

This is the only functionality that needs to be implemented, I have commented this test, because the functionality is not yet implemented. We will just have to un-comment it and modify it a little bit in the future.

LucasXu0 · 2023-07-03T02:41:59Z

@MayurSMahajan Nice try! Can you list all the cases that related to delete key?

For example,

collapsed selection
1. at the begin, ...
2. at the middle, ...
3. at the end, ...
non collapsed selection, ...

MayurSMahajan · 2023-07-03T04:48:19Z

@LucasXu0 Here are the cases for the delete key:

Collapsed Selection:

at the begin and middle, delete the right character, implemented
at the end, (needs to be implemented)

if the next node is TextNode: merge the next node in the current text node
if the next node is a bullet list or checkbox, anything that has some text then, remove the bullet point/checkbox and just merge the text into the current text node.
if the next node doesn't have any text, then ignore
if the current node is not a text node then also ignore it.

Non-Collapsed Selection:

Just delete the selection entirely, implemented

LucasXu0 · 2023-07-03T04:53:53Z

@LucasXu0 Here are the cases for the delete key:

Collapsed Selection:

at the begin and middle, delete the right character, implemented

at the end, (needs to be implemented)

if the next node is TextNode: merge the next node in the current text node

if the next node is a bullet list or checkbox, anything that has some text then, remove the bullet point/checkbox and just merge the text into the current text node.

if the next node doesn't have any text, then ignore

if the current node is not a text node then also ignore it.

Non-Collapsed Selection:

Just delete the selection entirely, implemented

Very clear. So do you need me to implement the missing part?

MayurSMahajan · 2023-07-03T05:04:51Z

Yes, I can't wrap my head around it 😅, I guess if you can do it, that would be great.

MayurSMahajan added 3 commits July 1, 2023 12:00

feat: add basic delete command

638a25b

chore: update comment

6789140

test: comment test for future

7fdd9ad

LucasXu0 reviewed Jul 3, 2023

View reviewed changes

LucasXu0 force-pushed the fr_delete_key_335 branch from b1396d8 to 7fdd9ad Compare July 7, 2023 08:06

LucasXu0 added 4 commits July 7, 2023 16:06

Merge branch 'main' into fr_delete_key_335

363ba35

Merge branch 'main' into fr_delete_key_335

c78dee7

feat: implement delete command

cc063e3

chore: flutter analyze

0e53201

LucasXu0 merged commit 3f7c9fe into AppFlowy-IO:main Jul 7, 2023

This was referenced Jul 12, 2023

chore: bump version 1.2.0 #305

Merged

chore: upgrade editor to 1.2.1 AppFlowy-IO/AppFlowy#2997

Merged

MayurSMahajan deleted the fr_delete_key_335 branch March 28, 2024 15:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: delete key functionality #272

feat: delete key functionality #272

MayurSMahajan commented Jul 1, 2023

codecov bot commented Jul 1, 2023 •

edited

Loading

LucasXu0 Jul 3, 2023

LucasXu0 Jul 3, 2023

LucasXu0 Jul 3, 2023

MayurSMahajan Jul 3, 2023

MayurSMahajan Jul 3, 2023

LucasXu0 Jul 3, 2023

MayurSMahajan Jul 3, 2023

LucasXu0 commented Jul 3, 2023 •

edited

Loading

MayurSMahajan commented Jul 3, 2023

LucasXu0 commented Jul 3, 2023

MayurSMahajan commented Jul 3, 2023

feat: delete key functionality #272

feat: delete key functionality #272

Conversation

MayurSMahajan commented Jul 1, 2023

codecov bot commented Jul 1, 2023 • edited Loading

Codecov Report

LucasXu0 Jul 3, 2023

Choose a reason for hiding this comment

LucasXu0 Jul 3, 2023

Choose a reason for hiding this comment

LucasXu0 Jul 3, 2023

Choose a reason for hiding this comment

MayurSMahajan Jul 3, 2023

Choose a reason for hiding this comment

MayurSMahajan Jul 3, 2023

Choose a reason for hiding this comment

LucasXu0 Jul 3, 2023

Choose a reason for hiding this comment

MayurSMahajan Jul 3, 2023

Choose a reason for hiding this comment

LucasXu0 commented Jul 3, 2023 • edited Loading

MayurSMahajan commented Jul 3, 2023

LucasXu0 commented Jul 3, 2023

MayurSMahajan commented Jul 3, 2023

codecov bot commented Jul 1, 2023 •

edited

Loading

LucasXu0 commented Jul 3, 2023 •

edited

Loading