Style lost when deleting a newline before a block span

[0nko]

Edit: Updated steps to reproduce in the comment here: https://github.com/wordpress-mobile/AztecEditor-Android/issues/335#issuecomment-298574701

(Leaving the following steps for reference) ~~1. Start with a newline followed by a block span (heading, for example)

2. Put the cursor at the start of the block span
3. Remove the newline with a backspace
4. Notice the original block style was removed~~

[0nko]

The style is being removed by the the ParagraphCollapseRemover. The logic seems to assume that by deleting a newline we want to merge 2 block spans, which is not always the case. @hypest, could you confirm this please?

[hypest]

The behaviour observed with the described steps is the intended one. Deleting the newline before some block element "attracts" the text into the block element before. For example, in the demo app, deleting the newline at the end of "Heading 1" (either by deleting it, or backspacing from "Heading 2's line) causes the "Heading 2" text to be included in "Heading 1"'s block style. That's how text is extracted out of the start of a block element as well.

The same behaviour, for example, is observed while deleting the final newline from a list, causing text that lies after the list to be attracted into the list.

What should be the desired behaviour instead? Why? Some specific examples might also help. Thanks!

EDIT: I think this ticket might be about deleting an empty line before the element, right?

[hypest]

The style is being removed by the the ParagraphCollapseRemover. The logic seems to assume that by deleting a newline we want to merge 2 block spans, which is not always the case. @hypest, could you confirm this please?

Here's what happening: PARAGRAPH_SPANs try to re-anchor when their leading anchor (the preceding newline) is deleted from the text. The android framework tries to move the span's start to the next (moving towards the end of the text) available anchor. If the span doesn't include a newline other than its ending newline than the span will actually collapse and will be found at the end of the (whole) text. That's where ParagraphCollapseRemover comes into play, detecting that situation and "garbage collecting" the collapsing span.

In essence, it's not the ParagraphCollapseRemover that removes the span; it merely cleans up the text from collapsing (block) spans. The collapse itself happens because the span's leading anchor disappears. That's a "fortunate" event in our case since the way editing works, we do want the collapsing span to be removed and its text become assimilated to the preceding block.

And that indeed happens: the newline removal will cause the preceding block span's end to automatically move down to find a new end anchor, effectively assimilating the text previously belonging to the block that collapsed.

Now, there might be cases where we would want to retain the block span and not let it collapse. I think a case like that is what this particular ticket is about. Specifically, the case of the preceding block being "empty" in the first place. Right?

[0nko]

Now, there might be cases where we would want to retain the block span and not let it collapse. I think a case like that is what this particular ticket is about. Specifically, the case of the preceding block being "empty" in the first place. Right?

Yeah, that's right. In the demo text, deleting the character before "Heading 1" should remove the image, but it removes the heading style instead. That's not a desired behavior :).

[hypest]

In the demo text, deleting the character before "Heading 1" should remove the image, but it removes the heading style instead.

The demo app seems to have a newline between the image and "Heading 1" so, not sure if the image should be deleted. I'd say the image needs to stay as is since what the deletion did was to actually join the two lines (image and text). Visually the "Heading 1" text will be wrapped below the image when the screen width is small enough.

Then, since the "Heading 1" is "pulled up" to a line that is not a heading (or any other block element for that matter), removal of the heading style is proper behaviour. I don't see a problem there.

Alternatively, if you mean that the heading style should stay, then the whole line needs to adopt it and that means the image will need to be engulfed in it (and any other text that might be on that line anyway). That would contradict how backspacing typically works (check out Calypso editor and Google docs editor as examples).

[0nko]

Hmm, you're right. 😕 Calypso behaves the same way. The visual editor tries but there seems to be a bug. It's definitely not something I'd expect, though. But let's be consistent and keep it as it is then. Thanks for the insights, Stefanos ;).

[hypest]

@0nko , may I suggest reopening this ticket, but targeting it at the case where there is an issue. Namely, the empty preceding line. Steps to reproduce:

1. with the demo app, place the cursor at the start of "Heading 2"
2. add a few newlines via the keyboard
3. tap backspace
4. the heading styling got removed

I think that case is counter-intuitive since all I want to do there is to move the block "up". It seems that both Calypso and Google docs apply the heading style to the empty lines so, when tapping backspace preserves the styling. I think we can match that behaviour just by applying the same block style to the empty lines as well.

[0nko]

Oh! Ok then. Reopening.

[daniloercoli]

Not sure if this is in some way related to #567, but keeping a reference to the other issue here makes sense to me. (also applied the same priority to both of them).

[mzorz]

Closing via #606