feat: Added divider to work with * #118

glunkad · 2023-05-14T06:14:11Z

Description

Underline parser implementation to support markdown code format

Feature Preview

Partially Implemented divider to work with star .
Changes to decoder and encoder are yet to be made

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings

PR Checklist

My code adheres to the AppFlowy Style Guide
I've listed at least one issue that this PR fixes in the description above.
I've added a test(s) to validate changes in this PR, or this PR only contains semantic changes.
All existing tests are passing.

CLAassistant · 2023-05-14T06:14:16Z

All committers have signed the CLA.

LucasXu0 · 2023-05-15T01:45:30Z

Hey, @glunkad. Why do you add divider to this repo? any related issue?

glunkad · 2023-05-15T06:52:35Z

Hey, @glunkad. Why do you add divider to this repo? any related issue?

AppFlowy-IO/AppFlowy#2371

LucasXu0 · 2023-05-15T12:20:45Z

lib/src/render/divider/divider_shortcut_event.dart

+// ---
+ShortcutEvent insertDividerEvent = ShortcutEvent(
+  key: 'Divider',
+  command: 'Minus,shift+digit 8',


use character instead of the command.

LucasXu0 · 2023-05-15T12:30:07Z

Hey, @glunkad. Can you implement it in the mobile branch?

Here's the migration guide. #97 (comment)
Also, you can refer to the branch develop_new_editor in appflowy project.

glunkad · 2023-05-16T08:22:19Z

Hey, @glunkad. Can you implement it in the mobile branch?

Here's the migration guide. #97 (comment)
Also, you can refer to the branch develop_new_editor in appflowy project.

Absolutely! I'll be happy to implement it .

Xazin · 2023-05-18T00:52:32Z

lib/src/render/divider/divider_node_widget.dart

+    Key? key,
+    required this.node,
+    required this.editorState,
+  }) : super(key: key);


Suggested change

Key? key,

required this.node,

required this.editorState,

}) : super(key: key);

super.key,

required this.node,

required this.editorState,

});

Xazin · 2023-05-18T00:53:30Z

lib/src/render/divider/divider_node_widget.dart

+      padding: const EdgeInsets.symmetric(vertical: 10),
+      child: Container(
+        height: 1,
+        color: Colors.grey,


Do we not have a color for the divider in editor style? If not we should add one.

Do we need a if statement to check?

Xazin · 2023-05-18T01:00:33Z

lib/src/render/divider/divider_shortcut_event.dart

+// ---
+ShortcutEvent insertDividerEvent = ShortcutEvent(
+  key: 'Divider',
+  command: 'Minus,shift+digit 8',


Suggested change

command: 'Minus,shift+digit 8',

character: '-',

glunkad · 2023-05-18T14:18:08Z

Should I add multiple characters ie '_','*' in character ?

Xazin · 2023-05-18T16:03:50Z

Should I add multiple characters ie '_','*' in character ?

No, not for now. It's not supported. I will make a PR this week for support for that.

glunkad · 2023-05-20T06:38:15Z

Ok works.

codecov · 2023-05-20T12:21:43Z

Codecov Report

Merging #118 (b61ced3) into main (25ac5b2) will decrease coverage by 0.16%.
The diff coverage is 50.40%.

@@            Coverage Diff             @@
##             main     #118      +/-   ##
==========================================
- Coverage   59.05%   58.89%   -0.16%     
==========================================
  Files         211      215       +4     
  Lines        9512     9624     +112     
==========================================
+ Hits         5617     5668      +51     
- Misses       3895     3956      +61

Impacted Files	Coverage Δ
...vider_block_component/divider_block_component.dart	`6.81% <6.81%> (ø)`
...ent/divider_block_component/divider_menu_item.dart	`20.00% <20.00%> (ø)`
lib/src/service/standard_block_components.dart	`98.46% <80.00%> (-1.54%)`	⬇️
.../render/selection_menu/selection_menu_service.dart	`76.69% <80.76%> (-1.33%)`	⬇️
...src/render/selection_menu/selection_menu_icon.dart	`87.50% <87.50%> (ø)`
...er_block_component/divider_character_shortcut.dart	`100.00% <100.00%> (ø)`

... and 18 files with indirect coverage changes

Xazin · 2023-05-20T13:36:45Z

lib/src/render/divider/divider_shortcut_event.dart

+ShortcutEventHandler _insertDividerHandler = (editorState, event) {
+  final selection = editorState.service.selectionService.currentSelection.value;
+  final textNodes = editorState.service.selectionService.currentSelectedNodes
+      .whereType<TextNode>();
+  if (textNodes.length != 1 || selection == null) {
+    return KeyEventResult.ignored;
+  }
+  final textNode = textNodes.first;
+  if (textNode.toPlainText() != '--') {
+    return KeyEventResult.ignored;
+  }
+  final transaction = editorState.transaction
+    ..deleteText(textNode, 0, 2) // remove the existing minuses.
+    ..insertNode(textNode.path, Node(type: kDividerType)) // insert the divder
+    ..afterSelection = Selection.single(
+      // update selection to the next text node.
+      path: textNode.path.next,
+      startOffset: 0,
+    );
+  editorState.apply(transaction);
+  return KeyEventResult.handled;
+};


I am thinking to allow this to be used for multiple different characters, we can change it like this:

Suggested change

ShortcutEventHandler _insertDividerHandler = (editorState, event) {

final selection = editorState.service.selectionService.currentSelection.value;

final textNodes = editorState.service.selectionService.currentSelectedNodes

.whereType<TextNode>();

if (textNodes.length != 1 || selection == null) {

return KeyEventResult.ignored;

}

final textNode = textNodes.first;

if (textNode.toPlainText() != '--') {

return KeyEventResult.ignored;

}

final transaction = editorState.transaction

..deleteText(textNode, 0, 2) // remove the existing minuses.

..insertNode(textNode.path, Node(type: kDividerType)) // insert the divder

..afterSelection = Selection.single(

// update selection to the next text node.

path: textNode.path.next,

startOffset: 0,

);

editorState.apply(transaction);

return KeyEventResult.handled;

};

ShortcutEventHandler _insertDividerHandler = (editorState, event) {

final character = event?.character;

if (character == null) {

return KeyEventResult.ignored;

}

final selection = editorState.service.selectionService.currentSelection.value;

final textNodes = editorState.service.selectionService.currentSelectedNodes

.whereType<TextNode>();

if (textNodes.length != 1 || selection == null) {

return KeyEventResult.ignored;

}

final textNode = textNodes.first;

if (textNode.toPlainText() != (character * 2)) {

return KeyEventResult.ignored;

}

final transaction = editorState.transaction

..deleteText(textNode, 0, 2) // remove the existing characters.

..insertNode(textNode.path, Node(type: kDividerType)) // insert the divider

..afterSelection = Selection.single( // update selection

path: textNode.path.next,

startOffset: 0,

);

editorState.apply(transaction);

return KeyEventResult.handled;

};

I suggest we can add if block to check if character is '-','*','_' and only then insert the divider .

Why? The event handler will be hooked to a certain character, and so that will decide which is allowed.

glunkad · 2023-05-22T14:26:48Z

Can I know to files that need to be updated ?

LucasXu0 reviewed May 15, 2023

View reviewed changes

annieappflowy added new feature this is something new for the end user editor features related to the rich-text editor labels May 15, 2023

annieappflowy assigned glunkad May 15, 2023

Xazin reviewed May 18, 2023

View reviewed changes

Xazin reviewed May 20, 2023

View reviewed changes

glunkad and others added 5 commits June 5, 2023 09:20

feat: Added divider to work with *

b4a99d8

chore: Resolved the suggested changes

2afc4d9

refactor: removed the check for star character

46d8515

feat:added character variable to insert divider .

ba7c965

feat: support converting stars ** to divider

b61ced3

LucasXu0 merged commit eeae6fe into AppFlowy-IO:main Jun 7, 2023

glunkad deleted the feat_markdown_divider_star branch June 13, 2023 04:39

Xazin mentioned this pull request Jun 13, 2023

[FR] Markdown divider implement ___ AppFlowy-IO/AppFlowy#2372

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added divider to work with * #118

feat: Added divider to work with * #118

glunkad commented May 14, 2023

CLAassistant commented May 14, 2023 •

edited

Loading

LucasXu0 commented May 15, 2023 •

edited

Loading

glunkad commented May 15, 2023

LucasXu0 May 15, 2023

LucasXu0 commented May 15, 2023 •

edited

Loading

glunkad commented May 16, 2023

Xazin May 18, 2023

Xazin May 18, 2023

glunkad May 18, 2023

Xazin May 18, 2023

glunkad commented May 18, 2023 •

edited

Loading

Xazin commented May 18, 2023

glunkad commented May 20, 2023

codecov bot commented May 20, 2023 •

edited

Loading

Xazin May 20, 2023

glunkad May 21, 2023

Xazin May 21, 2023

glunkad May 21, 2023 •

edited

Loading

glunkad commented May 22, 2023

feat: Added divider to work with * #118

feat: Added divider to work with * #118

Conversation

glunkad commented May 14, 2023

Description

Feature Preview

Partially Implemented divider to work with star . Changes to decoder and encoder are yet to be made

Type of change

Checklist:

PR Checklist

CLAassistant commented May 14, 2023 • edited Loading

LucasXu0 commented May 15, 2023 • edited Loading

glunkad commented May 15, 2023

Choose a reason for hiding this comment

LucasXu0 commented May 15, 2023 • edited Loading

glunkad commented May 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glunkad commented May 18, 2023 • edited Loading

Xazin commented May 18, 2023

glunkad commented May 20, 2023

codecov bot commented May 20, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glunkad May 21, 2023 • edited Loading

Choose a reason for hiding this comment

glunkad commented May 22, 2023

Partially Implemented divider to work with star .
Changes to decoder and encoder are yet to be made

CLAassistant commented May 14, 2023 •

edited

Loading

LucasXu0 commented May 15, 2023 •

edited

Loading

LucasXu0 commented May 15, 2023 •

edited

Loading

glunkad commented May 18, 2023 •

edited

Loading

codecov bot commented May 20, 2023 •

edited

Loading

glunkad May 21, 2023 •

edited

Loading