wordpress-mobile / AztecEditor-Android

A reusable native Android rich text editor component.
Mozilla Public License 2.0
675 stars 112 forks source link

Issue/527 quote issues #628

Closed 0nko closed 6 years ago

0nko commented 6 years ago

Fixes #527. Toggling the quote on any line removes the entire blockquote.

As I worked on this issue I've discovered a series of problems, namely incorrect block nesting when applying one block over another, applying block elements over list items, applying block elements across different levels of nesting (plain text + list item, for example), broken list formatting when applied to the last line, and more. I've refactored the block styling logic so that addresses all of the above.

daniloercoli commented 6 years ago

Weird, I see testRemoveQuoteFormatting failing, even though Travis is green.

0nko commented 6 years ago

@daniloercoli, this PR is ready for another pass. About the failing UI tests - it seems like something has changed with the emulator and styled text is not copied as HTML anymore. The tests are also failing in develop so it's not caused by this.

daniloercoli commented 6 years ago

@0nko I see Lint errors on this PR. Would you mind to fix it? In the meanwhile I start reviewing it.

daniloercoli commented 6 years ago

I've tested this PR a bit and found out some weird behaviours when mixing list and block-quote.

First scenario:

You will see that the HTML code isn't the right one:

screen shot 2018-02-26 at 15 54 07 screen shot 2018-02-26 at 15 54 16

0nko commented 6 years ago

Ready for another pass. This thing was another newline can of worms but I think I got all the edge cases nailed 😓.

The problem was that the list did not expect multi-line blocks inside list items, so they'd show the line indicator. This wasn't obvious previously because list items that contain other blocks didn't show (which was wrong) any line indicators.

I've refactored the way that lines are counted. I've tweaked & tested it with all kinds of different blocks inside lists, such as headings, quotes and also other lists (which should only display the inner line indicators).

daniloercoli commented 6 years ago

It seems to me that the background applied to a blockquote item doesn't work fine on RTL. This is on develop screenshot_20180228-124126

This PR screenshot_20180228-124337

0nko commented 6 years ago

I found a few more problems with block-closing using a newline, when there are multiple nested blocks. The RTL quote background is also fixed now. Thanks for the thorough review, @daniloercoli!

daniloercoli commented 6 years ago

Ship it! 🥇 :shipit: 🚀