Skip to content

Conversation

@mzorz
Copy link
Contributor

@mzorz mzorz commented Aug 14, 2018

This PR performs a dataSource re-init (by recreating the object in the state) when a full parse has taken place. While we've avoided re-creating the list's dataSource in other cases, I think it makes sense to do so after an html-to-blocks conversion has taken place.

This makes sure the state of dataSource, and props are aligned, and fixes the detail described in #100 here:

On Android, this currently won't work, but this issue was taken care of by #95. [...]

To test (on Android):

  1. start the demo app
  2. edit one of the known blocks such as paragraph or heading
  3. wait a bit so Aztec makes the change signaling happen (at least 500 miliseconds ;) ) this is something we'll probably need to take a look into at some point
  4. switch to HTML view
  5. observe the html contents show the entered text in the edited blocks
  6. edit such blocks again (without breaking the delimiters)
  7. switch back from html
  8. observe the new text is in place, in the visual mode.

Idea to make this better at some point in the future: only pass the fullparse: true value when no edits have been in place (maybe at least check the length?)

@mzorz mzorz requested review from etoledom and hypest August 14, 2018 18:37
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this! It's working great 🎉

My only concern is adding more flags to the redux store. I don't really know if it's a common pattern or not but we could look into it in the future too.

@mzorz
Copy link
Contributor Author

mzorz commented Aug 15, 2018

My only concern is adding more flags to the redux store. I don't really know if it's a common pattern or not but we could look into it in the future too.

TBH I don't know either 😄 I think it's a valid concern and I wonder that too 🤔 - if you agree we can merge this one and if @hypest (or anyone else for the case) thinks there's a better solution we can always revert this one, as it's just one commit (586526b)

@mzorz
Copy link
Contributor Author

mzorz commented Aug 15, 2018

Merging, as agreed over DM, with possibility to revert if necessary :shipit:

@mzorz mzorz merged commit 59db4ae into feature/parse-html-into-blocks Aug 15, 2018
@mzorz mzorz deleted the try/mario-full-parse branch August 15, 2018 15:51
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.

3 participants