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

[super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction (Resolves #1866) #1896

Merged
merged 14 commits into from
Apr 12, 2024

Conversation

matthew-carroll
Copy link
Contributor

@matthew-carroll matthew-carroll commented Mar 17, 2024

Parse and apply inline Markdown styles in an EditorReaction.

Applying inline Markdown means recognizing inline Markdown syntax, removing the syntax, and adding appropriate attributions where the syntax was. Example: "Hello *italics*" with "Hello italics".

Inline Markdown could appear as a result of multiple causes, e.g., the user typing a character, the user pasting text, or a programmer edit operation. Therefore, it's not always clear what we should consider when attempting to apply inline Markdown parsing.

This PR inspects all text content in every TextNode that's edited during a given transaction. All inline Markdown that's found in an edited node is applied.

Ambiguous Changes

Markdown syntax for bold and italics makes it ambiguous to identify the user's intention when the user enters only one character at a time.

Imagine that the user wants to achieve '"Hello bold"'. Consider the following steps to type that:

Hello **bold
Hello **bold*
Hello **bold**

If the inline Markdown is blindly applied to the text, the user would end up with the following results:

Hello **bold

Hello *bold

Hello bold

The user would end up with italic text, not bold text, because each "*" was applied on its own. The user never got a chance to enter two "**" back to back.

To solve this problem, the inline Markdown reaction first converts the existing TextNodes AttributedText to a Markdown string. Then, that Markdown string is parsed back to AttributedText. That process looks like the following:

Start with: Hello **bold

User types "*", creating text: Hello **bold*

The reaction serializes the AttributedText and gets: Hello **bold* (no change)

The reaction deserializes the Markdown and gets: "Hello *bold" (with italics)

User types "*", creating text: Hello *bold* (with italics)

The reaction serializes the AttributedText and gets: Hello **bold** (notice that we've recovered the extra "*"s)

The reaction deserializes the Markdown and gets: "Hello bold" (with bold)

By going from AttributedText -> Markdown -> AttributedText, we successfully honor ambiguous Markdown syntax.

Surgical changes

The easiest way to apply a Markdown change is to throw away the previous paragraph and replace it with the newly parsed and styled paragraph. However, when considering undo/redo systems, which remember all deleted text, repeatedly deleting and recreating entire paragraphs quickly becomes costly.

This PR brings in a text diff'ing package so that text and styles are only inserted/deleted where necessary. This adds complexity to the reaction, but reduces the memory footprint for the history ledger.

Selection adjustments

The selection base/extent might sit within the TextNode where inline Markdown is applied. Therefore, the reaction in this PR adjusts the selection bounds based on the amount of content that's either inserted or deleted.

@matthew-carroll
Copy link
Contributor Author

CC @dhikshith12

@matthew-carroll matthew-carroll changed the title [super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction [super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction (Resolves #1866) Mar 17, 2024
@dhikshithrm
Copy link

Curious to know how ***Bold And Italic*** is handled while typing the closing *** stars.

In Notion, It will start with bold and then become *BOLD and as soon as another star is added at the end it then becomes bold and italic.

@dhikshithrm
Copy link

Screen.Recording.2024-03-18.at.15.46.37.mov

@matthew-carroll
Copy link
Contributor Author

@miguelcmedeiros I fixed the situation you identified over in the issue ticket where starting a block of text with Markdown syntax causes that syntax to disappear.

@matthew-carroll matthew-carroll changed the title [super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction (Resolves #1866) Markdown reaction Mar 24, 2024
@matthew-carroll matthew-carroll changed the title Markdown reaction [super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction (Resolves #1866) Mar 24, 2024
@matthew-carroll
Copy link
Contributor Author

For reference, I filed this issue in the markdown package: dart-lang/markdown#599

Copy link
Collaborator

@miguelcmedeiros miguelcmedeiros left a comment

Choose a reason for hiding this comment

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

LGTM

@matthew-carroll
Copy link
Contributor Author

@angelosilvestre do you wanna take a look at this before merging?

@angelosilvestre
Copy link
Collaborator

@matthew-carroll Yeah, I'll take a look in a few minutes

Copy link
Collaborator

@angelosilvestre angelosilvestre left a comment

Choose a reason for hiding this comment

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

I left some comments.

@matthew-carroll matthew-carroll merged commit d18698f into main Apr 12, 2024
11 checks passed
@matthew-carroll matthew-carroll deleted the 1866_markdown-styling branch April 12, 2024 00:52
github-actions bot pushed a commit that referenced this pull request Apr 12, 2024
matthew-carroll added a commit that referenced this pull request Apr 12, 2024
BazinC added a commit to BazinC/super_editor that referenced this pull request Apr 15, 2024
* Updated Super Editor dependencies. (superlistapp#1869)

* Fix getAllAttributionsThroughout (superlistapp#1868)

* [SuperEditor] Fix markdown sublist deserialization (Resolves superlistapp#1821) (Resolves superlistapp#1822) (superlistapp#1827)

* Fix demo website (Resolves superlistapp#1874) (superlistapp#1875)

* Fix iOS cursor blinking while typing. (Resolves superlistapp#1876) (superlistapp#1877)

* [SuperEditor][Demo][Docs] Implement formatting toolbar for the Docs Demo (Resolves superlistapp#1783) (superlistapp#1819)

* [SuperEditor][Web] Fix cmd + left to move selection (Resolves superlistapp#1852) (superlistapp#1860)

* [SuperTextField] Fix layout with custom font height (Resolves superlistapp#1864) (superlistapp#1888)

* [SuperEditor][SuperReader] Fix setting attributions across multiple nodes (Resolves superlistapp#1865) (superlistapp#1880)

* checking for existing attributions across multiple nodes before toggling incoming attributions on nodes

* checking for single attribution throughout node, docs updated

* tests added

* refactored logic for checking if all nodes have all attributions for any of their characters

* refactored approach to validating whether a particular attribution should be toggled for a node

* added findEditor helper to SuperEditorInspector

* added toggleAttributionsForDocumentSelection to SuperEditorInspector

* using newly added test tools

* added some helpers to reduce boilerplate

* refactor

* added more tests for various combinations for attributions toggling on nodes

* accounting for scenario when multiple attributions are being applied to a selection, tests updated

* redundant comments removed

* updates

* tests comments updated

* reverted back to a more simple solution for the issue

* formatting updates

* added helper methods to reduce boilerplate

* refactor

* using add/remove attribution methods from AttributedText instead of spans

* checking text markers instead of serializing to markdown for no attributions cheks

* formatting updates

* test comments updated

* tests naming updates

* tests added for partial selections across multiple nodes

* moved few new document creators to test_documents, refactoring

* moved toggleAttributionsForDocumentSelection to test file where it's used

* renamed newly added test documents

* refactored how we fetch editor in tests

* test documents updated

* Added change event for adding/removing attributions, also switched from generic event to a specific TextInsertionEvent for InsertAttributedTextCommand (Resolves superlistapp#1884) (superlistapp#1908)

* [SuperEditor][ExampleApp] Replace PopoverScaffold with overlord version (Resolves superlistapp#1892) (superlistapp#1922)

* [SuperEditor][Web] Fix crash when merging paragraphs (Resolves superlistapp#1921) (superlistapp#1929)

* [SuperEditor] Avoid throwing UnimplementedError in currentAutofillScope (Resolves superlistapp#1923) (superlistapp#1931)

* [super_editor_markdown] - Parse and apply inline Markdown styles in an EditorReaction (Resolves superlistapp#1866) (superlistapp#1896)

* Upgrade uuid package to v4.0.0 (Resolves superlistapp#1899) (superlistapp#1917)

---------

Co-authored-by: Matt Carroll <[email protected]>
Co-authored-by: Angelo Silvestre <[email protected]>
Co-authored-by: Rutvik Tak <[email protected]>
Co-authored-by: Aloïs Deniel <[email protected]>
Co-authored-by: KevinBrendel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants