wordpress-mobile / AztecEditor-Android

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

Issue/258 fixing missing paragraphs #309

Closed khaykov closed 7 years ago

khaykov commented 7 years ago

This PR fixed #258 and adds support for input with mix of visual/html formatting.

Short story: wpcom api return’s post where p and br tags are replaced with newlines and mixed with visual newlines (attached to block elements, for example).

To address this, we are replication "calypso style” pre/post processing of content (with some changes related to span2html parser). p and br tags are replaced with newlines, empty span tags are removed.

Now, this brings a lot of “calypso” like editor behavior (mostly newline merging) when you switch between html and visual modes, so if you think something is wrong, check out how it works in calypso first.

I don't particularly like calypso's processing of html, so In a future, ideally, we should get the "real" post html with unparsed shortcodes, and preprocessed excerpt which will require some changes to API.

khaykov commented 7 years ago

Added "paragraph on enter" feature. Now, the post editing experience should be really similar to calypso. Couple of warning: calypso mode is enabled by default, for easier integration with wpandroid. All the tests except for CalypsoFormattingTest are not using calypso mode. 'toHtml()' method in calypso mode produces mix of HTML and newlines (which is converted by calypso backend into normal post).

0nko commented 7 years ago

I've noticed a curious behavior. I opened a post with multiple paragraphs. Within a paragraph, each successive line had larger and larger height:

image

khaykov commented 7 years ago

Fixed!

khaykov commented 7 years ago

Regarding removal of &nbsp;. This was not supposed to happen! They are now replaced with regular spaces instead. Also, in anticipation of Tyler's PR I fixed the broken <pre> processing.

About "Calypso mode", yes, I totally agree that it should be separated from main lib. I think that the current priority is to make Aztec compatible with WP, but for the general wellbeing of a library we will need to work on it a bit. We have all the code, so it's a matter of refactoring and library design. There are 2 independent components, that both depend on Format.kt, so looks it's a good candidate for becoming a formatting plugin. I talked before about global Aztec configuration, and we will need to implement it in some way, and make sure both source and visual editor are getting necessary plugins from it.

hypest commented 7 years ago

The "Calypso mode" code is deeply intertwined with the core editor. If Aztec is to be a general-purpose editor library I think this feature shouldn't be there.

I too agree with @0nko that this mode is deeply intertwined with the rest of the lib and would prefer it factored out.

By the way, I think I notice some code changes not being wrapped in the isInCalypsoMode either (example: the EndOfParagraphMarker code in AztecParser.kt) and should probably be wrapped as well. While at it, I think the marking of EndOfParagraphMarkers should be in its own function rather than in addVisualNewlinesToBlockElements.

Even though it will be quite an effort to design a "plugin system" to have the Calypso paragraph treatment in one and so we can do it in a separate PR, I think some work should be done here in this PR first. Otherwise I can easily see it becoming even harder to do it in the future. For now, I would highly recommend to at least have all Calypso-mode code be in a single class, it's methods of which will be used throughout the lib, instead of the code snippets be scattered around the lib as it is at the moment. The benefit will be to at least make it easy for our future-selves to identify/pinpoint which code we need to factor out to a plugin.

0nko commented 7 years ago

I think some work should be done here in this PR first.

Since we have a beta-release today and this PR is functionally OK I don't want this to be left out just for the refactoring. We can start working on it right away but I'd like this feature to ship because the editor isn't really usable in WP without it.

khaykov commented 7 years ago

I fixed the issue @hypest mentioned, and also did a quick refactoring - moved most of the pre/post processing code to the Format.kt, and to EndOfParagraphMarkerAdded. Also fixed visual appearance of paragraph in quote.

Hope I didn't break anything :)

khaykov commented 7 years ago

One moment, forgot to add another minor change.

khaykov commented 7 years ago

Done!

0nko commented 7 years ago

A return inside a quote adds a paragraph break in the visual mode but when I switch to HTML there is only a regular single newline. When I switch back to visual the paragraph break is gone.

0nko commented 7 years ago

I also noticed it's now not possible to close the quote. Newlines just expand it.

khaykov commented 7 years ago

This fix for closing blockquotes will work in calypso, because we don't really use paragraph spans after certain point. It will still happen in normal mode, so I will make an issue for it.

0nko commented 7 years ago

Awesome! :shipit: