Skip to content

Conversation

@pinarol
Copy link
Contributor

@pinarol pinarol commented Nov 26, 2018

This change adds switch to html/visual mode messaging to the bridge and demonstrates it on the example app.
On RN side: Updates the BlockManager to handle TOGGLE_BLOCK_MODE updates and re-render its contents.

screen shot 2018-11-26 at 16 33 52

Part of #182

To Test

Test 1:

  • Run yarn install & yarn start and yarn ios as usual to open the example app
  • Tap (...) button on the rightside of the navigation bar
  • Tap "Switch to HTML"
  • Verify that HTML editor is shown
  • Tap (...) button on the rightsize of the navigation bar again
  • Tap "Switch to Visual"
  • Verify that visual editor is shown

Test 2:

  • Reopen the example app
  • Tap (...) button on the rightside of the navigation bar
  • Tap "Switch to HTML"
  • Edit something in html edit mode(do not switch back to the visual mode)
  • Tap Save in the navigation bar
  • Verify that you see the edited version of the html in Xcode logs

Test 3:

  • Reopen the example app
  • Tap (...) button on the rightside of the navigation bar
  • Tap "Switch to HTML"
  • Edit something in html edit mode
  • Tap (...) button on the rightsize of the navigation bar again
  • Tap "Switch to Visual"
  • Verify that you see the edited version of the html in visual mode
  • Tap Save in the navigation bar
  • Verify that you see the edited version of the html in Xcode logs

Also:

  • Verify that both "Inspect blocks" and "Switch to html mode" switches are removed.

@pinarol pinarol changed the title Add switch to html ability through the bridge Add ability to toggle html/visual modes through the bridge Nov 26, 2018
@pinarol pinarol requested review from Tug and etoledom November 26, 2018 16:22
# Conflicts:
#	src/block-management/block-manager.js
@koke
Copy link
Member

koke commented Nov 28, 2018

I pushed a commit that gets rid of the extra padding at the top, which wasn't that noticeable before, but looked weird without the toggles

@koke
Copy link
Member

koke commented Nov 28, 2018

I tested this and works great, iOS code looks good as well.

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.

Great job @pinarol ! It looks super good and it's working flawlessly 🎉

I got one question as code comments sections, but whatever is the answer I'm happy to ✅ and :shipit: !

@etoledom
Copy link
Contributor

A little note @hypest .
By merging this PR, we are going to lose the ability to switch to HTML in the Android example app, until the bridge and a native-side switch is implemented there.

@etoledom
Copy link
Contributor

It would be good also, at this point, to bundle a new App.js file, since the current one is quite outdated and WPiOS won't recognize the new bridge message without this code.

But it could be on a follow up PR too. 👍
cc @koke

@pinarol pinarol merged commit 7ac7875 into master Nov 28, 2018
@pinarol pinarol deleted the feature/switch-to-html branch November 28, 2018 18:00
@koke
Copy link
Member

koke commented Nov 28, 2018

I'll be pushing a new version of the bundle tomorrow with all the latest changes

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants