Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Paragraphs re-work #472

Merged
merged 81 commits into from
Jan 27, 2023
Merged

Paragraphs re-work #472

merged 81 commits into from
Jan 27, 2023

Conversation

jmartinesp
Copy link
Contributor

@jmartinesp jmartinesp commented Jan 11, 2023

What?

The progression of the decisions around line breaks are as follow:

  1. In the beginning we used <br /> tags to represent new lines as most of our nodes were inline ones and it seemed like the simplest solution at the time.
  2. Since the introduction of lists, we realised that we’d have to add some extra offsets to represent intermediate positions between list items and before/after the list: to do that we started using zero-width spaces (ZWSP) characters to try to delimit that position and add an extra offset.
  3. This however, led to some issues with mapping indexes on Android/iOS, and also increased complexity quite a bit since we now had to check for special characters inside the different text nodes.
  4. Since using ZWSP characters was really complex, we tried using a ‘ZWSP node’ instead, which simplified some checks. This however, didn’t fix all issues with index mappings and we realised that we had add ZWSP to both the block nodes/lists and another one after it, which had its own issues (parsing, finding the right adjacent nodes when a ZWSP was present…).

None of these solutions actually worked, so we proposed a new approach: use paragraphs along with other block nodes instead of line breaks, remove the ZWSPs altogether from the codebase and add extra offsets where needed.

Steps:

  1. Replace line breaks with actual <p> tags as much as possible.
  2. Try to remove all traces of ZWSP nodes/chars in our codebase.
  3. Add an extra offset to the cursor position at the end of block nodes when they have a next sibling:
"Foo <pre>bar</pre>" // Doesn't need to add an extra offset at the end since there is no next sibling
"<pre>Foo</pre> bar" // Needs to add an extra offset there, as visually there will be a new line after the <pre> tag

// So, this cursor position is 3
"<pre>Foo|</pre> bar"
// And this one is 4
"<pre>Foo</pre>| bar"

This also has the extra benefit that we’ll always use inline nodes (formatting, links, text, etc.) with inline nodes and block nodes with block nodes (paragraphs, blockquotes, pre, lists), making it easier to make them work together.

Why?

// Say we have a list like this:
<ul><li>A</li><li>B</li></ul>

// This cursor position
<ul><li>A|</li><li>B</li></ul>
// And this one
<ul><li>A</li><li>|B</li></ul>
 
// Are technically the same (1)

// The same happens to
<pre>Code|</pre>Text
// And
<pre>Code</pre>|Text

So we need a way to differentiate them.

Work to do:

  • Add paragraph block nodes.
  • Use paragraphs instead of line break nodes as much as possible.
  • Update the way selection is read and written in the tests and find_pos.
  • Fix code blocks.
  • Fix quotes.
  • Fix lists.
  • Fix indentation.
  • Make sure a container node only has either inline nodes or block nodes inside.
  • Fix selection mis-matches in some corner cases.
  • Fix backspacing.
  • Remove ZWSP nodes.

`<br />` nodes.

They should be post-processed later to translate them to paragraphs.
* Fix selection writing
* Fix wrapping block nodes in block nodes
@jmartinesp
Copy link
Contributor Author

jmartinesp commented Jan 12, 2023

Status update of this task:

  • 41 failing tests (only 😅 ).
  • Quotes and code blocks now share code for creating new lines, and most of the code in their 'add' and 'remove' methods is actually the same. We might be able to also share that code if we pass some lambdas/functions to generate the new nodes and process the children.
  • Lists are mostly working too, only indentation is failing, but that's because there is a selection mismatch happening when we turn <li>Outer<br/>Inner</li> into <li>Outer<ul><li>Inner</li></ul>, the line break is lost there. A solution would be to turn it into <li><p>Outer</p><ul><li>Inner</li></ul> instead.

Next steps:

  • Fix those indentation issues, that is, create a function to wrap existing inline nodes in a list item into a paragraph.
  • Fix backspacing, I'll probably need to implement some new algorithm to merge the contents of 2 block nodes when backspacing between them.

Velin92 and others added 2 commits January 24, 2023 12:07
* fixing empty paragraphs not being rendered as newlines

* fixing as many tests and things as possible

* we should always remove the \n

* fix for links and inlinecode

* fixing some more tests

* created a function that removes the trailing new line only when strictly necessary

* improvement

* updating tests

* removed useless comments

* Fix mis-calculation in `ContainerNode.text_len()`

* last test fixed

* fixing the list case and improved the comment

* fix

* improved the comment

* Remove FIXME and restore working UI tests (#509)

* Empty paragraphs will return NBSP inside of them (#511)

* nbsp added for empty paragraphs

* fixed all the tests

* removing nbsp

* automatically trim enabled

* trimming whitespaces

Co-authored-by: Jorge Martín <[email protected]>
Co-authored-by: aringenbach <[email protected]>
aringenbach and others added 6 commits January 25, 2023 09:25
…er input (#514)

* Improve quote/code blocks display upon applying content or on character input

* fixed a bug on blockquotes where newlines where deleted and improved the formatting, now it renders after a newline has been added and a a character is typed (#515)

Co-authored-by: Velin92 <[email protected]>
* Fix nbsp parsing with discardable text

* we need to remove the blockquote check since this now since it's not needed anymore and the line is always added.

* fixing a typo

Co-authored-by: Mauro Romito <[email protected]>
* [iOS] Enable indent/unindent

* Add indent/unindent WysiwygComposerTests

* Add lists & indent/unindent UI tests

* Remove trailing whitespace
Velin92 and others added 6 commits January 26, 2023 11:10
* fix but not optimal

* there is a bug but we're almost there.

* failure

* added tests but found two failing

* improving the code but three tests are failing

* fix but some tests are failing and there is a small (but ignorable since it's an impossible case) regression

* failing test

* Try to fix link creation in range

* test improvements, removed some unused vars and imports

* Added some comments to the code

Co-authored-by: Jorge Martín <[email protected]>
* fixing tests

* nbsp in empty paragraphs in pre
- Fix parsing of line breaks in code blocks.
- Add these changes to the web parser too.
* fixing tests

* nbsp in empty paragraphs in pre

* this improvement will only add nbsp to leading and trailing
* Initial steps to integrate the new paragraphs behaviour into the Android lib.

* Add support for indent/unindent

* Prepare the Android code to work with NBSP chars in the HTML input for the parser

* Fix tests and add tests for paragraphs

* Fix new lines in code blocks, add documentation to `HtmlToSpansParser`.

* Fix wrong leading margin calculation on `BlockRendered`

* Use NBSP instead of ZWSP, otherwise multiple whitespaces are broken

* Fix instrumentation tests

* Allow backspacing in index 0 (#523)

* Fix code blocks after NBSP changes

* Improve how leading margin is calcualted for SpanBackgroundRenderers and add code block support for `EditorStyledTextView`.

* Try to uniffy span handling in HTML parser

* Remove `gradle.xml` file
artcodespace and others added 5 commits January 27, 2023 10:13
* add rust code to deal with splitting formatting nodes on pressing enter
* add tests for rust to cover this
* adds util in web to allow index/selection mapping to work
* Fix: insert paragraph at the middle of a block node with enter

* Make code block test a bit clearer

* Add extra tests

* Fix broken test
* close to the solution but still experiencing a misalignment

* partial fix for the end of the document

* replaceNewlinesWithDiscardableElements however it doesn't work properly yet

* improvements but the replace doesn't always work

* finally this works

* comment improvement

* code improvement, we only replace with zwsp when necessary
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
18.0% 18.0% Duplication

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants