[RNMobile] Fix crash bringing gutenberg master to mobile #17228

etoledom · 2019-08-28T08:57:55Z

Description

This patch makes the minimal adjustments to make the native version of gutenberg work with the latest change to the core data entities. Since we don't yet have networking support on mobile, the approach explored here is to preload the store with the required core data entities so it does not fail on load.

Testing Instruction

Checkout Gutenberg Mobile PR Fix store crash with gutenberg master wordpress-mobile/gutenberg-mobile#1366 and update submodule
Start demo app yarn start:reset and yarn android
Check that the app boots and that nothing is broken in the UI
Check that the tests pass

gziolo · 2019-09-23T08:12:09Z

I see many changes which were reviewed in the past and merged into rnmobile/master. Is it possible to identify only those changes which are necessary to merge rnmobile/master into master? This would make the whole process much easier.

gziolo

General observation, I noticed that you introduce new public APIs which are used only in native implementations. withTheme is something introduced recently. I know that there is mobile subfolder in @wordpress/components. How about, we always prefix them with either __unstable or __experimental to make sure they are never documented until we consider them as production-ready. I have a feeling that the fact that you introduced them doesn't mean they fit mobile use case only. I can see withTheme applicable to both web and mobile. Bottom sheet component is another example where the pattern used for native mobile would fit the web mobile as well. Having this clear indicator that some new APIs were created would help to identify them and think more broadly how to expose them for web.

/cc @hypest @koke @pinarol @Tug

gziolo · 2019-09-23T08:13:34Z

packages/rich-text/src/component/index.native.js

@@ -20,6 +20,7 @@ import { childrenBlock } from '@wordpress/blocks';
 import { decodeEntities } from '@wordpress/html-entities';
 import { BACKSPACE } from '@wordpress/keycodes';
 import { isURL } from '@wordpress/url';
+import { withTheme } from '@wordpress/components';


That's interesting. It's not part of the web public API. Do you think it would be helpful to start using the wrapper components for theming in general?

I wouldn't port this HOC to the web, at least not in its current state. It uses an internal event emitter to listen to native events and refresh the styles and is very much tied to the use case of dark mode right now. I think we will want to refactor this even for native first and use the store to dispatch events at least.

Sounds like __experimentalWithDarkMode then :)

gziolo · 2019-09-23T08:14:14Z

packages/rich-text/src/component/index.native.js

@@ -770,25 +771,28 @@ export class RichText extends Component {
 			style,
 			__unstableIsSelected: isSelected,
 			children,
+			useStyle,


I don't think it's a good idea to prefix the prop name with use as this confuses it with React hooks.

Agreed here. cc @etoledom

Something like extendStyles() or extendStylesToTheme() maybe?

I agree too. There already were some issues with linting.

gziolo · 2019-09-23T08:17:13Z

packages/block-library/src/separator/edit.js

-				>
-				</PanelColorSettings>
-			</InspectorControls>
+			<SeparatorSettings


The following might be easier to follow for those not familiar with the internal implementation:

Suggested change

<SeparatorSettings

<SeparatorInspectorConrols

gziolo · 2019-09-23T08:19:29Z

.travis.yml

@@ -20,6 +20,7 @@ cache:
 branches:
  only:
    - master
+    - rnmobile/master


Yes, let's do it if you plan to use rnmobile/master more often 👍

We might want to revert that I'm not sure. I merely merged rnmobile/master in here (thus the amount of changes unrelated to the PR description) but it might need some work before we merge it back to master

We might actually need to branch off again as we focus on the Open Beta. Keeping it for now 👍

I'm sure you will branch more often so it's fine to keep it here until we merge the mobile repo in here.

pinarol · 2019-09-23T08:46:38Z

I agree about marking withTheme as __unstable or __experimental. Apart from it being mobile only, we still haven't figured out a common design language between mobile/web so it might even change in the near future.

koke · 2019-09-23T08:48:13Z

packages/components/src/mobile/dark-mode/index.native.js

+	eventEmitter.setMaxListeners( 150 );
+}
+
+export function withTheme( WrappedComponent ) {


Using theme for this could be misleading, as WordPress has already a "theme" concept for a different thing. I think we could borrow the term "preferred color scheme" from the web implementation of dark mode and implement this cross platform?

Just like we have a useReducedMotion hook, we could also have a usePreferredColorScheme. For now, since you can't use hooks in class components, we could still have a withPreferredColorScheme HOC for this.

The web implementation seems simple enough:

const isDarkMode = useMediaQuery( '(prefers-color-scheme: dark)' );

Agreed, I much prefer this approach 👍

withPreferredColorScheme - sounds like a good one. We still might want to refactor the hook as follows:

const isDarkMode = usePreferredColorScheme( 'dark' );

and use it inside the HOC. useMediaQuery might be not applicable to native mobile, right?

Tug

This LGTM 👍
Since I authored some good bits of it, I think it could benefit from another review. @etoledom do you mind having a look?
Given that rnmobile/master is merged in, it might be a bit tricky to review now, I think I can try to group all our changes in a single commit if that makes sense.

…ail to load the current post

Squashed commit of the following: commit df025a6 Author: Luke Walczak <[email protected]> Date: Thu Sep 26 11:11:53 2019 +0200 Fix lint issue (#17598) commit 1c9b133 Author: Sérgio Estêvão <[email protected]> Date: Wed Sep 25 22:30:59 2019 +0100 Add missing heading levels to the UI (H4, H5, H6) (#17533) commit ae6d2ce Author: Tugdual de Kerviler <[email protected]> Date: Wed Sep 25 13:19:10 2019 +0200 [RNMobile] Refactor Dark Mode HOC (#17552) * [RNMobile] Refactor the Dark Mode HOC to fix naming antipatterns * Fix lint errors * Add .native.js suffix to usePreferredColorScheme * Update usage of theme props renamed to preferredColorScheme * Update usage of theme props renamed to preferredColorScheme commit 69da85e Author: Danilo Ercoli <[email protected]> Date: Wed Sep 25 11:08:33 2019 +0200 Autosave monitor - Make the mobile editor ping the native at each keystroke, since the deboucing logic is already well defined in the apps. (#17548) commit e99d365 Author: Luke Walczak <[email protected]> Date: Wed Sep 25 10:29:20 2019 +0200 Add isAppender functionality on mobile (#17195) * Add isAppender functionality on mobile * refactor isAppender conditions * Replace dropZoneUIOnly in favour of showMediaSelectionUI * deprecate dropZoneUIOnly and add disableMediaSelection prop * Update test * Refactor tests and change prop name * Remove redundant empty lines * Refactor conditions inside MediaPlaceholder * Update block-editor CHANGELOG * Update packages/block-editor/CHANGELOG.md Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> commit 648a1b9 Author: Danilo Ercoli <[email protected]> Date: Thu Sep 19 09:47:44 2019 -0400 [RNMobile] Add autosave to mobile apps (#17329) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193) * Add a preliminary version of the AutosaveMonitor for mobile that calls the "bridge" and asks the native side to save the content * Add autosave mock function for tests * Fix merge conflicts * Fix lint * Re-add autosave on mobile that was removed erroneously during import-merge from rnmobile/master * Remove native variant of AutosaveMonitor and introduces changes at editor store level * Default to false for `isEditedPostAutosaveable` on mobile. There was a typo in the returing value on the previous commit. * Make sure to consider edits to the Title when checking if auto-save is needed * Fix lint commit 264b178 Author: etoledom <[email protected]> Date: Wed Sep 4 14:03:38 2019 +0200 [RNMobile] DarkMode improvements (#17309) * Remove the need to import `useStyle` and pass the theme prop on every instance that `withStyle` is used * Implement dark-mode refactor on all components * Fix broken native tests * Fix default block appender background color on DarkMode * DarkMode: Make `useStyle` a class function commit fc8c3da Author: Luke Walczak <[email protected]> Date: Wed Sep 4 14:02:20 2019 +0200 Remove redundant bg color within button appender (#17325) commit 89664eb Author: Luke Walczak <[email protected]> Date: Tue Sep 3 12:18:11 2019 +0200 Support group block on mobile (#17251) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots * Support group block on mobile * Extend shouldShowInsertionPoint condition to be false when group is selected * Code refactor * Update package-lock commit 14d482b Author: Matt Chowning <[email protected]> Date: Tue Aug 6 17:04:35 2019 -0400 [RNMobile] Insure tapping at end of post inserts at end Previously, tapping at the end of the post would insert a block immediately after the currently selected block. In addition, this commit is cleaning out a few unusued props in the block-list file. commit 7b12673 Author: etoledom <[email protected]> Date: Fri Aug 30 18:09:31 2019 +0200 Recover border colors (#17269) commit f9fa455 Author: Gerardo Pacheco <[email protected]> Date: Fri Aug 30 11:06:27 2019 +0200 [RNMobile] Fix dismiss keyboard button for the post title (#17260) commit 7aa44a2 Author: Luke Walczak <[email protected]> Date: Fri Aug 30 11:03:46 2019 +0200 Unify media placeholder and upload props within media-text (#17268) commit 3db95b7 Author: Drapich Piotr <[email protected]> Date: Fri Aug 30 09:05:11 2019 +0200 MediaUpload and MediaPlaceholder unify props (#17145) commit a78f204 Author: Tugdual de Kerviler <[email protected]> Date: Thu Aug 29 16:53:06 2019 +0200 Add native support for the MediaText block (#16305) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots commit 643c1b2 Author: etoledom <[email protected]> Date: Wed Aug 28 12:16:19 2019 +0200 Activate Travis CI on rnmobile/master branch (#17229) commit 635108e Author: etoledom <[email protected]> Date: Wed Aug 28 10:31:39 2019 +0200 [RNMobile] Native mobile release v1.11.0 (#17181) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193)

Tug · 2019-09-26T09:31:50Z

Ok this is rebased ontop of master, all commits related to fixing master for our RN build are grouped into 2 commits: the first one is your initial commit and the second one groups all my previous commits. The last commit is a squashed merge of rnmobile/master. Should be much easier to review now :)

Edit: I merged rnmobile/master in again but it brought all the commits from the list, I guess I'll have to keep squashing until we merge this one!

Tug · 2019-09-26T09:37:29Z

packages/block-editor/CHANGELOG.md

@@ -1,3 +1,9 @@
+## Master
+
+### Deprecation


Not sure what this change is doing here 🤔

Looks like we're fine, this is part of #17195

Yes, this PR landed yesterday 👍

Squashed commit of the following: commit d8b0d83 Author: Sérgio Estêvão <[email protected]> Date: Thu Sep 26 11:24:18 2019 +0100 Fix list filter on paste for RN mobile. (#17550) * Fix method for RN mobile. * Use array.From instead of slice. * Remove comment and use Array.from directly * Convert from NodeList spreadable to Array.from * Fix lint errors. * Fix documentation examples to use Array.from * Add empty line. commit df025a6 Author: Luke Walczak <[email protected]> Date: Thu Sep 26 11:11:53 2019 +0200 Fix lint issue (#17598) commit 1c9b133 Author: Sérgio Estêvão <[email protected]> Date: Wed Sep 25 22:30:59 2019 +0100 Add missing heading levels to the UI (H4, H5, H6) (#17533) commit ae6d2ce Author: Tugdual de Kerviler <[email protected]> Date: Wed Sep 25 13:19:10 2019 +0200 [RNMobile] Refactor Dark Mode HOC (#17552) * [RNMobile] Refactor the Dark Mode HOC to fix naming antipatterns * Fix lint errors * Add .native.js suffix to usePreferredColorScheme * Update usage of theme props renamed to preferredColorScheme * Update usage of theme props renamed to preferredColorScheme commit 69da85e Author: Danilo Ercoli <[email protected]> Date: Wed Sep 25 11:08:33 2019 +0200 Autosave monitor - Make the mobile editor ping the native at each keystroke, since the deboucing logic is already well defined in the apps. (#17548) commit e99d365 Author: Luke Walczak <[email protected]> Date: Wed Sep 25 10:29:20 2019 +0200 Add isAppender functionality on mobile (#17195) * Add isAppender functionality on mobile * refactor isAppender conditions * Replace dropZoneUIOnly in favour of showMediaSelectionUI * deprecate dropZoneUIOnly and add disableMediaSelection prop * Update test * Refactor tests and change prop name * Remove redundant empty lines * Refactor conditions inside MediaPlaceholder * Update block-editor CHANGELOG * Update packages/block-editor/CHANGELOG.md Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> commit 648a1b9 Author: Danilo Ercoli <[email protected]> Date: Thu Sep 19 09:47:44 2019 -0400 [RNMobile] Add autosave to mobile apps (#17329) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193) * Add a preliminary version of the AutosaveMonitor for mobile that calls the "bridge" and asks the native side to save the content * Add autosave mock function for tests * Fix merge conflicts * Fix lint * Re-add autosave on mobile that was removed erroneously during import-merge from rnmobile/master * Remove native variant of AutosaveMonitor and introduces changes at editor store level * Default to false for `isEditedPostAutosaveable` on mobile. There was a typo in the returing value on the previous commit. * Make sure to consider edits to the Title when checking if auto-save is needed * Fix lint commit 264b178 Author: etoledom <[email protected]> Date: Wed Sep 4 14:03:38 2019 +0200 [RNMobile] DarkMode improvements (#17309) * Remove the need to import `useStyle` and pass the theme prop on every instance that `withStyle` is used * Implement dark-mode refactor on all components * Fix broken native tests * Fix default block appender background color on DarkMode * DarkMode: Make `useStyle` a class function commit fc8c3da Author: Luke Walczak <[email protected]> Date: Wed Sep 4 14:02:20 2019 +0200 Remove redundant bg color within button appender (#17325) commit 89664eb Author: Luke Walczak <[email protected]> Date: Tue Sep 3 12:18:11 2019 +0200 Support group block on mobile (#17251) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots * Support group block on mobile * Extend shouldShowInsertionPoint condition to be false when group is selected * Code refactor * Update package-lock commit 14d482b Author: Matt Chowning <[email protected]> Date: Tue Aug 6 17:04:35 2019 -0400 [RNMobile] Insure tapping at end of post inserts at end Previously, tapping at the end of the post would insert a block immediately after the currently selected block. In addition, this commit is cleaning out a few unusued props in the block-list file. commit 7b12673 Author: etoledom <[email protected]> Date: Fri Aug 30 18:09:31 2019 +0200 Recover border colors (#17269) commit f9fa455 Author: Gerardo Pacheco <[email protected]> Date: Fri Aug 30 11:06:27 2019 +0200 [RNMobile] Fix dismiss keyboard button for the post title (#17260) commit 7aa44a2 Author: Luke Walczak <[email protected]> Date: Fri Aug 30 11:03:46 2019 +0200 Unify media placeholder and upload props within media-text (#17268) commit 3db95b7 Author: Drapich Piotr <[email protected]> Date: Fri Aug 30 09:05:11 2019 +0200 MediaUpload and MediaPlaceholder unify props (#17145) commit a78f204 Author: Tugdual de Kerviler <[email protected]> Date: Thu Aug 29 16:53:06 2019 +0200 Add native support for the MediaText block (#16305) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots commit 643c1b2 Author: etoledom <[email protected]> Date: Wed Aug 28 12:16:19 2019 +0200 Activate Travis CI on rnmobile/master branch (#17229) commit 635108e Author: etoledom <[email protected]> Date: Wed Aug 28 10:31:39 2019 +0200 [RNMobile] Native mobile release v1.11.0 (#17181) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193)

etoledom · 2019-09-27T10:25:14Z

Since I authored some good bits of it, I think it could benefit from another review. @etoledom do you mind having a look?

Sure! I'll give it a deep test in both iOS and Android 👍

Squashed commit of the following: commit f3085c1 Author: Gerardo Pacheco <[email protected]> Date: Fri Sep 27 09:55:44 2019 +0200 [RNMobile] Move MediaUploadPorgress to its own component folder (#17392) * Move MediaUploadPorgress to its own component folder (native) * MediaUploadProgress - Fix import to code standards * MediaUploadProgress readme * Mobile - MediaUploadProgress README update commit d8b0d83 Author: Sérgio Estêvão <[email protected]> Date: Thu Sep 26 11:24:18 2019 +0100 Fix list filter on paste for RN mobile. (#17550) * Fix method for RN mobile. * Use array.From instead of slice. * Remove comment and use Array.from directly * Convert from NodeList spreadable to Array.from * Fix lint errors. * Fix documentation examples to use Array.from * Add empty line. commit df025a6 Author: Luke Walczak <[email protected]> Date: Thu Sep 26 11:11:53 2019 +0200 Fix lint issue (#17598) commit 1c9b133 Author: Sérgio Estêvão <[email protected]> Date: Wed Sep 25 22:30:59 2019 +0100 Add missing heading levels to the UI (H4, H5, H6) (#17533) commit ae6d2ce Author: Tugdual de Kerviler <[email protected]> Date: Wed Sep 25 13:19:10 2019 +0200 [RNMobile] Refactor Dark Mode HOC (#17552) * [RNMobile] Refactor the Dark Mode HOC to fix naming antipatterns * Fix lint errors * Add .native.js suffix to usePreferredColorScheme * Update usage of theme props renamed to preferredColorScheme * Update usage of theme props renamed to preferredColorScheme commit 69da85e Author: Danilo Ercoli <[email protected]> Date: Wed Sep 25 11:08:33 2019 +0200 Autosave monitor - Make the mobile editor ping the native at each keystroke, since the deboucing logic is already well defined in the apps. (#17548) commit e99d365 Author: Luke Walczak <[email protected]> Date: Wed Sep 25 10:29:20 2019 +0200 Add isAppender functionality on mobile (#17195) * Add isAppender functionality on mobile * refactor isAppender conditions * Replace dropZoneUIOnly in favour of showMediaSelectionUI * deprecate dropZoneUIOnly and add disableMediaSelection prop * Update test * Refactor tests and change prop name * Remove redundant empty lines * Refactor conditions inside MediaPlaceholder * Update block-editor CHANGELOG * Update packages/block-editor/CHANGELOG.md Co-Authored-By: Grzegorz (Greg) Ziółkowski <[email protected]> commit 648a1b9 Author: Danilo Ercoli <[email protected]> Date: Thu Sep 19 09:47:44 2019 -0400 [RNMobile] Add autosave to mobile apps (#17329) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193) * Add a preliminary version of the AutosaveMonitor for mobile that calls the "bridge" and asks the native side to save the content * Add autosave mock function for tests * Fix merge conflicts * Fix lint * Re-add autosave on mobile that was removed erroneously during import-merge from rnmobile/master * Remove native variant of AutosaveMonitor and introduces changes at editor store level * Default to false for `isEditedPostAutosaveable` on mobile. There was a typo in the returing value on the previous commit. * Make sure to consider edits to the Title when checking if auto-save is needed * Fix lint commit 264b178 Author: etoledom <[email protected]> Date: Wed Sep 4 14:03:38 2019 +0200 [RNMobile] DarkMode improvements (#17309) * Remove the need to import `useStyle` and pass the theme prop on every instance that `withStyle` is used * Implement dark-mode refactor on all components * Fix broken native tests * Fix default block appender background color on DarkMode * DarkMode: Make `useStyle` a class function commit fc8c3da Author: Luke Walczak <[email protected]> Date: Wed Sep 4 14:02:20 2019 +0200 Remove redundant bg color within button appender (#17325) commit 89664eb Author: Luke Walczak <[email protected]> Date: Tue Sep 3 12:18:11 2019 +0200 Support group block on mobile (#17251) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots * Support group block on mobile * Extend shouldShowInsertionPoint condition to be false when group is selected * Code refactor * Update package-lock commit 14d482b Author: Matt Chowning <[email protected]> Date: Tue Aug 6 17:04:35 2019 -0400 [RNMobile] Insure tapping at end of post inserts at end Previously, tapping at the end of the post would insert a block immediately after the currently selected block. In addition, this commit is cleaning out a few unusued props in the block-list file. commit 7b12673 Author: etoledom <[email protected]> Date: Fri Aug 30 18:09:31 2019 +0200 Recover border colors (#17269) commit f9fa455 Author: Gerardo Pacheco <[email protected]> Date: Fri Aug 30 11:06:27 2019 +0200 [RNMobile] Fix dismiss keyboard button for the post title (#17260) commit 7aa44a2 Author: Luke Walczak <[email protected]> Date: Fri Aug 30 11:03:46 2019 +0200 Unify media placeholder and upload props within media-text (#17268) commit 3db95b7 Author: Drapich Piotr <[email protected]> Date: Fri Aug 30 09:05:11 2019 +0200 MediaUpload and MediaPlaceholder unify props (#17145) commit a78f204 Author: Tugdual de Kerviler <[email protected]> Date: Thu Aug 29 16:53:06 2019 +0200 Add native support for the MediaText block (#16305) * First working version of the MediaText component for native mobile * Fix adding a block to an innerblock list * Disable mediaText on production * MediaText native: improve editor visuals * Move BlockToolbar from BlockList to Layout * Remove BlockEditorProvider from BlockList and add native version of EditorProvider to Editor. Plus support InsertionPoint and BlockListAppender * Update BlockMover for native to hide if locked or if it's the only block * Make the vertical align button work, add more styling options for toolbar buttons * Make sure registerCoreBlocks does not break in production * Copy docblock comment from the web version for registerCoreBlocks * Fix focusing on the media placeholder * Only support adding image for now * Update usage of MediaPlaceholder in MediaContainer * Enable autoScroll for just the out most block list * Fix JS Unit tests * Roll back to IconButton refactor and fix tests * Fix BlockVerticalAlignmentToolbar buttons style on mobile * Fix thing for web and ensure ariaPressed is always passed down * Use AriaPressed directly to style SVG on mobile * Update snapshots commit 643c1b2 Author: etoledom <[email protected]> Date: Wed Aug 28 12:16:19 2019 +0200 Activate Travis CI on rnmobile/master branch (#17229) commit 635108e Author: etoledom <[email protected]> Date: Wed Aug 28 10:31:39 2019 +0200 [RNMobile] Native mobile release v1.11.0 (#17181) * [RNMobile] Fix crash when adding separator * Build: remove global install of latest npm since we want to use the paired node/npm version (#17134) * Build: remove global install of latest npm since we want to use the paired node/npm version * Also update travis to remove --latest-npm flag * [RNMobile] Try dark mode (iOS) (#17067) * Adding dark mode component implemented on list and list block * Adding DarkMode handling to RichText, ToolBar and SafeArea * Mobile: Using DarkMode as HOC * iOS DarkMode: Modified colors on block list and block container * iOS DarkMode: Improved Header Toolbar colors * iOS DarkMode: Removing background from buttons * iOS DarkMode warning and unsupported * iOS DarkMode: MediaPlaceholder * iOS DarkMode: BottomSheets * iOS DarkMode: Inserter * iOS DarkMode: DefaultBlockAppender * iOS DarkMode: PostTite * Update hardcoded colors with variables * iOS DarkMode: Fix bottom-sheet cell value color * iOS DarkMode: More - PageBreak - Add Block Here * iOS DarkMode: Better text color * iOS Darkmode: Code block * iOS DarkMode: HTML View * iOS DarkMode: Improve colors on SafeArea * Fix toolbar not avoiding keyboard regression * Fix native unit tests * Fix gutenberg-mobile unit tests * Adding RNDarkMode mocks * RNMobile: Fix crash when viewing HTML on iOS * [RNMobile] Remove toolbar from html view * [RNMobile] Fix MaxListenersExceededWarning caused by dark-mode event emitter (#17186) * Fix MaxListenersExceededWarning caused by dark-mode event emitter * Checking for setMaxListeners trying to avoid CI error * Adding remove listener to DarkMode HOC * DarkMode: Binding this.onModeChanged to `this` * DarkMode: Adding conditional needed to pass UI Tests on CI * Fix focus title on new posts regression (#17180) * BottomSheet: Setting DashIcon color directly when theme is default (light) (#17193)

mchowning · 2019-09-27T13:45:29Z

packages/api-fetch/src/index.native.js

+
+function apiFetch( options ) {
+	// eslint-disable-next-line no-console
+	console.warn( 'apiFetch called with options', options );


Instead of console.warn, what would you think about making this console.info or console.log so we don't add another yellow box warning to the debug apps?

I think it's pretty alarming if the app is trying to make an api request and we should be aware of it. At least it's as alarming as state update warning or deprecation notice imo.
What we could do if you think those warnings are disrupting the experience when testing on mobile is disable them completely. If we want to see those we can simply attach a debugger.

Disabled in wordpress-mobile/gutenberg-mobile@fb6db57

I think it's pretty alarming if the app is trying to make an api request and we should be aware of it.

I'm seeing that warning every time I run the app. Is that expected?

Yeah I see it as well, I haven't tracked down the issue yet, but we might have to when we tackle editing image options since it seems related to media.

Thanks for explaining. If this is a we-need-to-fix-this-as-soon-as-reasonably-possible type of thing, I have no concern about leaving the warning if you want. Alternatively, removing the warning and filing an issue to add the warning back in (and fix whatever is causing it to fire) could make sense so this doesn't get forgotten.

etoledom · 2019-09-27T14:02:06Z

✅ (I created this PR so I can't approve it on GH ¯\_(ツ)_/¯ )
I did a deep test on iOS and Android example apps, and also on WPiOS and WPAndroid.

I found some issues on the way, but by communicating early with @Tug , they have been fixed already 🎉

The Heading block Hx buttons were no displaying (fixed on 74ee26f)
The Post Title was being removed when the editor loaded (fixed on 5a7ee6d)
A warning about reaching the maximum number of listener reappeared (fixed on d62ead0)

After the Hx buttons bug was fixed, there's one remaining issue related to these buttons:

The Hx buttons on the Heading block won't change its icon and subscript color when it becomes active.

I'm happy to merge and fix this later on, or if you prefer to fix it right away that works for me too!
You can choose!

Great work @Tug as always, and thanks a lot for all this!

!

mchowning

Smoke tested this on both platforms and it LGTM! Great job @etoledom and @Tug ! 🙇

The only issue I saw was the issue that @etoledom noted with the Heading level icons text color not updating appropriately when selected (i.e. they stay gray instead of turning white-ish).

I would lean toward merging this PR as-is and fixing that minor issue in a follow-up PR myself.

gziolo · 2019-09-30T05:48:04Z

Nice work on this PR 👍

youknowriad · 2019-09-30T16:44:34Z

packages/components/src/icon-button/index.js

@@ -56,7 +56,7 @@ function IconButton( props, ref ) {
 			className={ classes }
 			ref={ ref }
 		>
-			{ isString( icon ) ? <Dashicon icon={ icon } ariaPressed={ ariaPressed } /> : icon }
+			<Icon icon={ icon } ariaPressed={ ariaPressed } />


This line breaks the web because ariaPressed is not a valid react prop in the web. Dashicon addresses t his properly but not the Icon component.

This PR removes this ariaPressed #17356
It's practically ready to merge.

Cool, thank you

Thanks for the heads up! I'll let you know when it gets merged.

youknowriad · 2019-10-03T08:18:14Z

I think changes to Icon component here also broke this #17719

Any idea how to solve it? it looks like previously the size passed to custom component icons was "undefined" and now it's "24"

etoledom added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Aug 28, 2019

etoledom self-assigned this Aug 28, 2019

etoledom mentioned this pull request Aug 28, 2019

Crash on the latest master on gutenberg repo wordpress-mobile/gutenberg-mobile#1332

Closed

This was referenced Sep 13, 2019

Fix store crash with gutenberg master wordpress-mobile/gutenberg-mobile#1366

Merged

[Monorepo] Import gutenberg-mobile as new WordPress react-native packages #17456

Closed

Tug marked this pull request as ready for review September 19, 2019 12:14

Tug self-assigned this Sep 19, 2019

Tug self-requested a review September 19, 2019 12:15

Tug force-pushed the rnmobile/fix-store-crash branch from 1c51f25 to 22a0119 Compare September 20, 2019 17:52

etoledom requested review from ajitbohra, chrisvanpatten, ellatrix, gziolo, jorgefilipecosta, Soean, talldan and youknowriad as code owners September 20, 2019 17:59

gziolo reviewed Sep 23, 2019

View reviewed changes

koke reviewed Sep 23, 2019

View reviewed changes

Tug mentioned this pull request Sep 24, 2019

[RNMobile] Refactor Dark Mode HOC #17552

Merged

5 tasks

Tug force-pushed the rnmobile/fix-store-crash branch 2 times, most recently from 71020fb to 4865f0a Compare September 25, 2019 15:32

Tug approved these changes Sep 26, 2019

View reviewed changes

Tug force-pushed the rnmobile/fix-store-crash branch from 4865f0a to bbf153f Compare September 26, 2019 09:22

Bring changes from editor reducer to mobile

05f7506

Tug force-pushed the rnmobile/fix-store-crash branch from bbf153f to 2ee9d93 Compare September 26, 2019 09:25

Tug added 2 commits September 26, 2019 11:27

Initialize the core data store with default entities so it does not f…

ad8d8d5

…ail to load the current post

Tug force-pushed the rnmobile/fix-store-crash branch from 2ee9d93 to 8fd916a Compare September 26, 2019 09:27

Tug reviewed Sep 26, 2019

View reviewed changes

Tug force-pushed the rnmobile/fix-store-crash branch from 7d2f297 to ea43258 Compare September 26, 2019 11:08

etoledom mentioned this pull request Sep 27, 2019

[RNMobile] iOS DarkMode toolbar buttons #17356

Merged

Tug added 4 commits September 27, 2019 12:32

Fix: Heading toolbar not displayed

74ee26f

Fix: Title being cleared on load

5a7ee6d

Fix: hide warning message about MaxListenersExceededWarning

d62ead0

mchowning reviewed Sep 27, 2019

View reviewed changes

Add back react-native-dark-mode mocks that are needed for e2e tests

df3a356

mchowning approved these changes Sep 27, 2019

View reviewed changes

Tug merged commit fea6377 into master Sep 27, 2019

Tug deleted the rnmobile/fix-store-crash branch September 27, 2019 14:38

youknowriad added this to the Gutenberg 6.6 milestone Sep 30, 2019

pinarol mentioned this pull request Sep 30, 2019

[RNMobile] refactor InspectorControls #17279

Merged

5 tasks

youknowriad reviewed Sep 30, 2019

View reviewed changes

etoledom mentioned this pull request Oct 2, 2019

Gutenberg/1.14.0 wordpress-mobile/WordPress-iOS#12574

Merged

1 task

desaiuditd mentioned this pull request Oct 2, 2019

Fix the browser warning for IconButton component. #17695

Closed

5 tasks

etoledom mentioned this pull request Oct 3, 2019

With Gutenberg 6.6, the pinned plugins menu icons are not scaled properly #17719

Closed

gziolo mentioned this pull request Oct 3, 2019

Components: Allow to apply isPressed prop to Button and SVG components to indicate the state of the button #17748

Merged

7 tasks

jsnajdr mentioned this pull request Oct 18, 2019

Optimize exports of the wp/compose package #17945

Merged

[RNMobile] Fix crash bringing gutenberg master to mobile #17228

[RNMobile] Fix crash bringing gutenberg master to mobile #17228

Conversation

etoledom commented Aug 28, 2019 • edited by Tug Loading

Description

Testing Instruction

gziolo commented Sep 23, 2019

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinarol commented Sep 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tug left a comment

Choose a reason for hiding this comment

Tug commented Sep 26, 2019 • edited Loading

Choose a reason for hiding this comment

Tug Sep 26, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etoledom commented Sep 27, 2019

mchowning Sep 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etoledom commented Sep 27, 2019 • edited Loading

mchowning left a comment

Choose a reason for hiding this comment

gziolo commented Sep 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad commented Oct 3, 2019

etoledom commented Aug 28, 2019 •

edited by Tug

Loading

Tug commented Sep 26, 2019 •

edited

Loading

Tug Sep 26, 2019 •

edited

Loading

mchowning Sep 27, 2019 •

edited

Loading

etoledom commented Sep 27, 2019 •

edited

Loading