wordpress-mobile / AztecEditor-Android

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

Detect change in html editor #704

Closed hypest closed 5 years ago

hypest commented 5 years ago

Fix #703

This is a follow up PR to https://github.com/wordpress-mobile/AztecEditor-Android/pull/674, extending the idea of using a hash-based mechanism to detect when the html (source) editor has user-initiated changes in comparison to the original content.

This functionality is needed by wpandroid to properly update a post remotely when the editor is in html mode.

A key detail in this implementation is that now, the source editor's displayStyledAndFormattedHtml() method requires a boolean flag to be passed, effectively adopting the "content dirty" flag from the visual editor.

Test

No test steps available.

daniloercoli commented 5 years ago

We should update current test cases for hasChange functionality, since they were already broken for the HTML editor, and it's a good time to include good tests in this PR. I had proposed a fix for tests here https://github.com/wordpress-mobile/AztecEditor-Android/pull/682/files#diff-96db0ae2a3c3076d079b181b7512c885R275

hypest commented 5 years ago

Yes, good call about adding tests @daniloercoli , will do. And thanks for the quick pointer!

daniloercoli commented 5 years ago

Not quite sure I'm testing this PR in the right way, but starting the demo app, and switching to HTML editor results in CHANGES returned by the hasChanges method of SourceViewEditText. Steps to repro:

hypest commented 5 years ago

Hmm, I do see getPureHtml(false) producing different results at various stages in the process/steps @daniloercoli . Will continue to investigate.

hypest commented 5 years ago

Made to fixes @daniloercoli :

  1. There was a bug where getPureHtml() was actually mutating the editor's text just to insert and return the cursor position. Fixed with 16902ea.
  2. The demo app was first initializing the source editor and then the visual editor. That way though, when switching back to html the source editor started detecting changes because the new text has different hash than the original one. I changed the demo app to prime the visual editor first, so source editor properly keeps the init hash from the text that includes the visual editor's mutations. Fixed with 3784dcd.
hypest commented 5 years ago

@daniloercoli pointed out that priming the source editor from the visual editor is not a good idea from a developer's point of view so, I reverted that commit.

Instead, with 3e1b4cb I changed the mode toggling logic to only re-set an editor's text if the other editor is reporting changes.

mzorz commented 5 years ago

LGTM, made a comment on the WPAndroid PR that might be worth considering implementing on this side of the world (i.e. adding a utility method to Aztec to encapsulate initialization calls for both AztecText and SourceViewEditText in a handy way), not a blocker for this PR though.

daniloercoli commented 5 years ago

Cool, thanks @mzorz for your review. Going to merge this then, and let @hypest open another PR if he/we decide to implement utility methods you've mentioned in the other PR.