Skip to content

Conversation

@hypest
Copy link
Contributor

@hypest hypest commented Jun 6, 2018

Fix #703

This is a follow up PR to #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
Copy link
Contributor

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
Copy link
Contributor Author

hypest commented Jun 7, 2018

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

@daniloercoli
Copy link
Contributor

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:

  • Start the demo app
  • Switch to HTML
  • Call the method hasChanges on the HTML editor.
  • It returns CHANGES instead of NO_CHANGES.

@hypest
Copy link
Contributor Author

hypest commented Jun 7, 2018

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

@hypest
Copy link
Contributor Author

hypest commented Jun 7, 2018

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
Copy link
Contributor Author

hypest commented Jun 7, 2018

@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.

@daniloercoli daniloercoli self-assigned this Jun 7, 2018
@mzorz
Copy link
Contributor

mzorz commented Jun 8, 2018

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants