Lists: Nuking Zero Width Spaces

[jleandroperez]

Description:

In this PR we're nuking Zero Width Spaces from the TextLists Formatter.

All of the Testing Scenarios are backed up by a unit test. Ain't that cool?

Needs Review: @diegoreymendez @SergioEstevao
Diego: Thanks for the infinite help on this one!!

Closes #421
Ref.: #414

Details:

• Yes. We're removing the Zero Width Spaces from the TextLists's inner working. Whenever a bullet must be rendered, and there's no text, we'll be inserting a \n character.

  Hook points for this behavior are insertText and the formatter's toggle themselves.

• And yes. We're overriding typingAttributes, because the didSet observer of both, selectedRange and selectedTextRange is not being always triggered (ie. user touch).

• TextListFormatter got a new helper to verify if lists of any kind are present in a collection of attributes.

• Removed a workaround from LayoutManager, which was being used to render the extra line fragment (Bullets when there was no text).

--

Testing:

Detailing below all of the test cases.

Scenario I: List after Backspace

1. Empty Document
2. Toggle List
3. Type something
4. Backspace

Verify that the list remains onscreen.

Scenario II: Deleting after List

1. Empty Document
2. Toggle List
3. Arrow Down
4. Backspace

Verify that the List is gone.

Scenario III: Newline Not Inserted

1. Empty Document
2. Toggle List
3. Arrow Down
4. Enter

Verify that the Newline gets inserted.

Scenario IV: New List Item when at the edge of the document

1. Empty Document
2. Toggle List
3. Type something
4. Arrow Down
5. Backspace
6. Enter

Verify that a new bullet gets added.

Scenario V: Typing Attributes on new line

1. Empty Document
2. Toggle List
3. Arrow Down

Verify that the Typing Attributes do not contain a Text List: indentation should be normal. Typing any character should not display a bullet.

Scenario VI: Empty Lines

1. Empty Document
2. Toggle List
3. Insert a newline

Verify that the bullet in the first line gets removed.

[diegoreymendez]

Jorge - this is working incredibly well. Very nice work!!!

I've pointed out a few changes that I think are key before merging this in. Can you give them a look? I'll keep reviewing the code to try and find any other issues.

PS: it hasn't crashed yet on me!

[diegoreymendez]

From this line below, the problem is that we're not only removing the textList style, but we're actually resetting the full paragraph configuration to the default, which is not really desired. For instance: we may not want to remove the blockquote style.

To avoid resetting the whole paragraph, you can use this code instead (or an equivalent code):

let orderedListFormatter = TextListFormatter(style: .ordered)
let unorderedListFormatter = TextListFormatter(style: .unordered)

if orderedListFormatter.present(in: attributes) {
    return orderedListFormatter.remove(from: attributes)
} else if unorderedListFormatter.present(in: attributes) {
    return unorderedListFormatter.remove(from: attributes)
} else {
    return attributes
}

PS: don't worry too much about the code being perfect. This code will change ENORMOUSLY once we change to using a mechanism closer to what Apple uses for nested lists, but this should be a good starting point.

[jleandroperez]

100% good call, thanks for the snippet!!

[diegoreymendez]

For reading simplicity I'd consider removing selectedRange.length == 0. If the caret position is indeed at EOF, then the length will always be zero.

[jleandroperez]

Absolutely right, updating as well, thanks!!

[diegoreymendez]

If the selected range location starts at EOF, then (+ there is no selected text) is always true.

[jleandroperez]

Updated, thank you!!

[diegoreymendez]

I'd consider adding extra documentation to explain why this is necessary. Specifically something like:

// This is necessary because when the caret is at EOF, and the previous `\n` character has
// a textList style, that style will remain in the `typingAttributes`.  We'll only allow the style
// to remain if there are contents in the current line with the textList style (in which case
// this condition won't ever trigger because we'll either no longer be at EOF, or the previous
// character won't be `\n`).

[jleandroperez]

Good call, thank you!

[diegoreymendez]

I removed all references to super in this method, and the code seems to work fine. Is it possible this was a problem when this code was called directly from the typingAttributes getter instead?

[jleandroperez]

For the record:

Removed the super.typingAttributes (it was probably crashing due to testing code!), but left the super.insertText, since, without that one, we do get a loop.

Re: extra docs, as discussed over Slack, we've got a unit test that would trigger the loop, so we're cool.

Thanks a lot Diego!

[diegoreymendez]

This is pure gold! Thanks @jleandroperez !

[jleandroperez]

Woooooo thanks SO much for the support Diego!! this one is more yours than mine!!!

[SergioEstevao]

@jleandroperez late to the party, but this PR was excellent!!!

[jleandroperez]

Thanks a lot @SergioEstevao!!! 🔥 🔥 🔥 🔥 🔥 🔥