Allow paragraph spans that do not extend AlignmentSpan

[mchowning]

Summary

This PR makes it possible to have a ParagraphSpan ("span" in the sense of implementing Android's Spanned interface, not html spans) that does not override the View's gravity. The View's gravity was being overriden because ParagraphSpan implemented Android's AlignmentSpan interface, which overrides gravity and sets the text's alignment.

This PR allows paragraphs to not implement AlignmentSpan by removing AlignmentSpan from a relatively central/high position in our inheritance hierarchy and instead we now only implement that interface in the specific classes where alignment is needed. This allows the creation of two types of "paragraph" spans. One that does not implement AlignmentSpan (for use when there is no alignment set on the span, which is the case for Gutenberg which uses the view's gravity to control alignment) and one that does implement AlignmentSpan (for use when alignment is being set on the span, i.e., the classic editor).

Although this PR changes quite a few files, many of those changed files are only updates to explicitly implement the AlignmentSpan interface since it is no longer a part of the inheritance hierarchy.

Description

Problem with using Android's text spans

Paragraph alignment for the gutenberg-mobile editor (gutenberg-mobile issue, gutenberg PR) does not work on Android because Aztec assigns alignment to individual spans, whereas Gutenberg assigns alignment at the view/block level. Assigning alignment at the span level creates difficulties around things like the merging of two blocks with different alignments. You can end up with a single block that has the text using the alignment from its previous blocks. In other words, a single block may now contain two spans with different alignments, whereas Gutenberg blocks are only allowed to have a single alignment.

In other circumstances, I ran into plain parsing errors where we would end up with invalid html output by Aztec (i.e. missing closing tags), or all the text for a given view would be lost.

At a higher level, a Span doesn't seem like the right abstraction for setting alignment on the entire view like Gutenberg wants to do.

Gravity instead of Span

Using the view's gravity to set alignment makes sense from the Gutenberg perspective both because it fits better with the single-alignment-for-the-entire-view approach of Gutenberg blocks and because it avoids a significant amount of complexity with parsing an alignment across multiple different spans but then outputting the appropriate alignment property for the entire view. In fact, using gravity for alignment means that, for purposes of gutenberg, AztecAndroid-Editor doesn't have to handle alignment at all—it just needs to not interfere with gravity (that's right, I'm talking about you AlignmentSpan).

In fact, if we only had to support Gutenberg, we could just remove all alignment logic from this library since Gutenberg just uses the view's gravity. I actually think this may be the biggest point in favor of using gravity. We undoubtedly could find a way to make the Span-based approach work, but it would almost certainly involve a significant increase in complexity in Aztec's parsing. On the other hand, using gravity will allow us to (eventually) reduce the complexity in Aztec.

A final issue with the spans implementation is that they do not update the alignment of the view's placeholder text like gravity does.

FWIW, iOS is also currently setting alignment at the view level for gutenberg-mobile instead of at the level of text spans, so gravity seems more consistent with that

Issue with gravity being overridden any time a <p> tag is present

The gravity-based approach did not work for paragraph blocks though because paragraph blocks use the <p> tag which gets parsed into a ParagraphSpan by Aztec (caption blocks do not send a <p> tag down to Aztec). ParagraphSpans implement the AlignmentSpan interface. When a Span implements AlignmentSpan it must return a non-null Layout.Alignment. Because there is not a "no alignment" Layout.Alignment value (the options are ALIGN_NORMAL, ALIGN_CENTER, ALIGN_OPPOSITE), we were defaulting any "no alignment" scenario to be ALIGN_NORMAL which would override the view's gravity for the purposes of the text's alignment.

Internally, the android system is checking for spans that implement the AlignmentSpan interface, so this PR removes the requirement that ParagraphSpans implement the AlignmentSpan interface. This was accomplished by removing the AlignmentSpan from the root IAztecParagraphStyle interface. The portion of that interface that dealt with the alignment was extracted to an IAztecAlignmentSpan interface (which implements AlignmentSpan). All the classes that previously implemented IAztecParagraphStyle (and there were quite a few of these) were updated to also implement IAztecAlignmentSpan in order to maintain previous behavior.

In the case of the ParagraphSpan, I have broken that into two classes:

1. A base ParagraphSpan class that is the same as the previous ParagraphSpan class except that it does not implement IAztecAlignmentSpan; and
2. A ParagraphSpanAligned class that both extends ParagraphSpan and implements IAztecAlignmentSpan.

I then created a builder method that has the same signature as the previous ParagraphSpan constructor, and returns the appropriate paragraph-based span class based on whether or not the method's align parameter is null. Because AlignmentSpans cannot have a null alignment we were previously treating null as equivalent to ALIGN_NORMAL (which is why the view's gravity was being overridden). The intended result is that anytime an align parameter is passed, the behavior will be exactly the same as before, and anytime a null align parameter is passed, you will get the same behavior except for the fact that the returned paragraph span class will not implement AlignemntSpan.

Test

Test for Regressions

This PR makes changes to quite a few classes. Although these changes seem safe because they are mostly just refactoring to move the AlignmentSpan out of the class hierarchy and instead implementing AlignmentSpan by applying it to the specific classes that need it, there is certainly a risk of regressions and it would be a good idea to give a bit of a smoke test to the Aztec editor to make sure that there have not been any regressions.

Testing Alignment set via gravity

The code adding gravity based alignment to paragraph blocks is not merged into Gutenberg (PR), but you can download an apk here of the WordPress that was created using this version of Aztec along with the code from this PR to test the use of gravity to set alignment for paragraph blocks in the Gutenberg editor.

Make sure strings will be translated

• If there are new strings that have to be translated, I have added them to the client's strings.xml as a part of the integration PR.

[daniloercoli]

Tested and worked as expected!
Nice work, and well detailed description of the problem.

Note: I've also noticed (both in develop) and this PR, that you cannot apply 2 different alignments to 2 different part of text, even if those parts are separated by an empty line. You need to insert between the 2 parts of text something that is not a paragraph.

[mchowning]

Thanks for bringing this up to date with develop @daniloercoli ! 🙌

I've also noticed (both in develop) and this PR, that you cannot apply 2 different alignments to 2 different part of text, even if those parts are separated by an empty line. You need to insert between the 2 parts of text something that is not a paragraph.

I can recreate this on the demo app if I add text inside the demo content. But I do not observe this issue if I add text to the very beginning of the demo content or first delete all the demo content. I also see that Aztec allows different alignment for each line in the current production WPAndroid app if I start a new post with the AztecEditor. So I'm not sure if this is an issue any users would experience. WDYT?

Even if it is an issue, it seems unrelated to this PR. Do you think this is good to merge?

[daniloercoli]

Do you think this is good to merge?

Sure, go ahead with the merging. We can open a new issue for the problem i reported later if we see it happening again and often.

[hypest]

The View's gravity was being overriden because ParagraphSpan implemented Android's AlignmentSpan interface, which overrides gravity and sets the text's alignment.

If there are multiple aligned spans in the content, which one is Android using to override the whole View gravity?

[mchowning]

If there are multiple aligned spans in the content, which one is Android using to override the whole View gravity?

I believe if there is any aligned span in the view then the view's gravity is ignored