Port the core/code block to be used by the ReactNative mobile GB app #5816

hypest · 2018-03-27T12:32:01Z

Description

This PR introduces the minimal set of changes to make the core/code block ready to be consumed by a ReactNative app, in the road to implement a "native" mobile GB editing experience as per the Make post.

Among the goals is to eventually be able to introduce patterns for how to share code between GB the webapp and GB the native mobile app. Please, feel free to open PRs against this one with proposals towards that goal.

TODO before lifting the "In progress" label:

Make sure the (web) GB build and webapp work normally
Factorize the RN platform dependent files to minimize code duplication between Android and iOS
~~Have a proposal ready for how to change the core/code block definition so its high level definition is shared among the 3 platforms (web, Android, iOS)~~
For the time being, the .native.js variants of key files (index.js among which is the most important ones) will serve as the method we provide compatibility with an RN app. The goal is still to come up with a "common language" down the road but we need to port more code before patterns on what the common language is emerge.

Types of changes

What types of changes does your code introduce?

Introduces .native.js variants of various key files. Those are picked up instead of the corresponding .js files when the code is imported in an RN build. The regular Gutenberg build just ignores the .native.js files.

The specialized index.native.js files play a key role: They are importing only the submodules needed by the RN implementation. These submodules will gradually be ported and only where appropriate.

New feature (non-breaking change which adds functionality)

Ports the core/code block into a RN compatible component.

Breaking change (fix or feature that would cause existing functionality to not work as expected)

None.

Checklist:

My code is tested. (Yes, tests are on the gutenberg-mobile repo)
My code follows the WordPress code style.
My code has proper inline documentation.

* CSS related imports are suppressed * classnames are left in only to minimize diff among the files but do not play a role, nor style the elements

So far, that's the only way I found to have the android-specific jest tests succeed.

hypest · 2018-03-27T12:33:56Z

👋 @gziolo , please feel free to add more reviewers to this PR if you think it's good they keep an open eye to this PR. Thanks!

👋@maxme , feel free to add more TODO items to the list. Thanks!

aduth · 2018-03-27T12:35:23Z

Have a proposal ready for how to change the core/code block definition so it's high level definition is shared among the 3 platforms (web, Android, iOS)

May also tie into #2751 (registration of said high-level definition on the server).

gziolo · 2018-03-27T13:28:29Z

Have a proposal ready for how to change the core/code block definition so it's high level definition is shared among the 3 platforms (web, Android, iOS)

May also tie into #2751 (registration of said high-level definition on the server).

In general, we are looking into having a way to define all blocks on the server (minus JS specific bits like edit, save or other function based properties). See #5802. At some point, we will have to wrap it with REST API endpoint to make it easy to consume by the platforms that don't use HTML to expose data from the server. At the moment those definitions are serialized as JSON and assigned to the global JS variable _wpBlocks.

hypest · 2018-03-28T10:07:07Z

Collapsed (with 8ee1a4e) platform specific code into .native.js files instead of individual (android, ios) ones. Thanks @gziolo for the tip!

Will define a `wp` global via RN and reuse the `createElement()` from `@gutenberg/component`.

hypest · 2018-03-29T00:54:12Z

Update:

Merged from master and fixed any conflict
Lifted the #pragma override of the JSX Babel transformer plugin
Added the mobile native source files (.native.js and friends) to the eslint ignore list. Those files are currently processed via Flow and Prettier on the RN project side.
Tests now pass

This reverts commit a3d5ab4.

hypest · 2018-04-30T08:03:58Z

After some discussion with @gziolo I've :

cleaned up the "todo-looking" comments
minimized the PR even further
changed the target branch back to master
updated from master and fixed any incompatibilities (e.g. due to file structure change)
updated the PR description
lifted the [Status] In Progress label.

gziolo

I left some minor comments. In general, I'm okey with the proposed solution, because everything happens in RN specific files. I think it would be a good idea to move all edit components to their own files anyway. We can help with that if others agree that it is a way to move forward.

@mtias, @aduth, @youknowriad any thoughts?

gziolo · 2018-04-30T08:32:28Z

core-blocks/code/edit.native.js

+ */
+import { PlainText } from '../../blocks';
+
+export function edit( { attributes, setAttributes, style } ) {


It can be exported as export default function ....

Makes sense, done with 0edfb18.

gziolo · 2018-04-30T08:34:15Z

core-blocks/code/index.js

 } from '@wordpress/blocks';

 /**
 * Internal dependencies
 */
-import './editor.scss';
+import { edit } from './edit'; // import the specialized `edit` function. Is is different between the web and native mobile.


This comment can be removed as we should probably do the same for all edit implementations :)

edit can be exported as default to have:

import edit from './edit'; // and later { ... edit, ... }

The comment can be considered trivial if one reads the PR and has an understanding of the .native.js pattern. I had added it to help people pick up the pattern but, i'm perfectly fine with removing it 👍 . Done with 9b68ba3.

gziolo · 2018-04-30T08:35:17Z

core-blocks/code/edit.native.js

+
+export function edit( { attributes, setAttributes, style } ) {
+	return (
+		<View>


Shouldn't be style applied to View rather than PlainText to mirror web implementation?

Yes and no. Unfortunately, styling in RN is not cascading by default and we will need more work on establishing a pattern on how to pass and style the blocks/components/containers.

In this particular case for example, the style includes the fontFamily property which is supported by the PlaintText component (because it's a TextInput under the hood) but not by the View component.

All in all, styling should be considered alpha at this stage as well and i'd suggest we work on it in a different PR, if that's OK with you too @gziolo . I'll update the PR description to include some relative info.

That would be awesome to have it documented in the code next to RN version of the edit component.

Done with 0377c73.

gziolo · 2018-04-30T08:36:08Z

blocks/plain-text/style.native.scss

@@ -0,0 +1,9 @@
+.blocks-plain-text {


Did you check if it isn't included in web build by accident?

Not extensively, I didn't. What I only tested was modifying values in the style file and see if GB-web picks those up, visually. Tried out adding a background-color for example. Any practical way to test it out more thoroughly perhaps?

I can test it myself before merging, but I think if you put background-color: pink in the RN version, it shouldn't get applied in the web version. That's all we need to ensure :)

That's grand, that was the test I did and it succeeded (color wasn't picked up) so, feel free to test is your side as well, thanks!

hypest · 2018-04-30T09:42:52Z

Thanks for the review @gziolo , ready for another pass! FYI, I made a minor change in the meantime (5c93331).

gziolo

I tested web version and it still works properly. I would defer the final decision to @aduth and @youknowriad, but speaking myself it's good to go.

gziolo · 2018-05-04T06:53:45Z

Let's get it and iterate with further PRs.

mtias · 2018-05-04T13:35:10Z

Good to see this happening :)

hypest added 6 commits March 15, 2018 17:30

Android specific files for the Code block

850497c

* CSS related imports are suppressed * classnames are left in only to minimize diff among the files but do not play a role, nor style the elements

Breaking change: Don't override the default JSX translation

43590b2

So far, that's the only way I found to have the android-specific jest tests succeed.

Add the platform specific files for iOS as well

bf82dce

Set TextInput to multiline for Code block

16b7eb5

Hide the default TextInput underline on Android

20fd557

Expose registerBlockType, getBlockType to the RN app

f922044

hypest added the [Status] In Progress Tracking issues with work in progress label Mar 27, 2018

hypest requested a review from gziolo March 27, 2018 12:32

gziolo added [Type] Technical Prototype Offers a technical exploration into an idea as an example of what's possible [Feature] Extensibility The ability to extend blocks or the editing experience labels Mar 27, 2018

gziolo added the Mobile Web Viewport sizes for mobile and tablet devices label Mar 27, 2018

Use .native.js for common native code

8ee1a4e

hypest added 5 commits March 28, 2018 13:42

Merge branch 'master' into rnmobile/port-code-block

3e81e6b

Correct configuration for the babel plugins settings

9a2c431

transform-react-jsx for the production env as well

0f3b9ea

No need for pragma override

0aaba3d

Will define a `wp` global via RN and reuse the `createElement()` from `@gutenberg/component`.

Ignore mobile native files when linting

a3d5ab4

hypest mentioned this pull request Mar 29, 2018

Update from GB master wordpress-mobile/gutenberg-mobile#25

Merged

hypest added 5 commits March 30, 2018 14:54

Revert "Ignore mobile native files when linting"

a29e5bc

This reverts commit a3d5ab4.

Comment out ununsed imports

e37c8bc

Fix codestyle

a6f8089

Import is compatible anyway, uncomment it

c190e24

Remove the React import. The RN project auto-imports it.

c96b10b

hypest added 3 commits April 30, 2018 10:26

Codestyle fix

779c342

GB isn't using Prettier so, no need for the pragma

2a97aed

Comment on the need to import the edit function

489ae72

hypest removed the [Status] In Progress Tracking issues with work in progress label Apr 30, 2018

hypest changed the title ~~WIP: Port the core/code block to be used by the ReactNative mobile GB app~~ Port the core/code block to be used by the ReactNative mobile GB app Apr 30, 2018

gziolo reviewed Apr 30, 2018

View reviewed changes

gziolo assigned hypest Apr 30, 2018

gziolo requested review from youknowriad, aduth and mtias April 30, 2018 08:38

hypest added 3 commits April 30, 2018 11:58

Export the edit function as default

0edfb18

Remove reduntant line comment

9b68ba3

Import block using the new pattern

5c93331

Add a comment about styling

0377c73

gziolo approved these changes Apr 30, 2018

View reviewed changes

hypest mentioned this pull request Apr 30, 2018

Pulling in all the latest dev from the GB side wordpress-mobile/gutenberg-mobile#43

Merged

gziolo mentioned this pull request May 2, 2018

Add "Spacer" block to create empty areas. #6121

Merged

Use object property shorthand for edit

dfb1a12

gziolo merged commit 881617f into master May 4, 2018

gziolo deleted the rnmobile/port-code-block branch May 4, 2018 07:09

gziolo added this to the 2.9 milestone May 4, 2018

This was referenced May 8, 2018

Core blocks: Extract edit to their own file - part 1 #6637

Merged

Core blocks: Refactor blocks to use edit file - part 2 #6641

Merged

gziolo added Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) and removed Mobile Web Viewport sizes for mobile and tablet devices labels Oct 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port the core/code block to be used by the ReactNative mobile GB app #5816

Port the core/code block to be used by the ReactNative mobile GB app #5816

hypest commented Mar 27, 2018 •

edited

Loading

hypest commented Mar 27, 2018

aduth commented Mar 27, 2018

gziolo commented Mar 27, 2018 •

edited

Loading

hypest commented Mar 28, 2018

hypest commented Mar 29, 2018

hypest commented Apr 30, 2018

gziolo left a comment

gziolo Apr 30, 2018

hypest Apr 30, 2018

gziolo Apr 30, 2018

hypest Apr 30, 2018

gziolo Apr 30, 2018

hypest Apr 30, 2018 •

edited

Loading

gziolo Apr 30, 2018

hypest Apr 30, 2018

gziolo Apr 30, 2018

hypest Apr 30, 2018

gziolo Apr 30, 2018

hypest Apr 30, 2018

hypest commented Apr 30, 2018

gziolo left a comment •

edited

Loading

gziolo commented May 4, 2018

mtias commented May 4, 2018

Port the core/code block to be used by the ReactNative mobile GB app #5816

Port the core/code block to be used by the ReactNative mobile GB app #5816

Conversation

hypest commented Mar 27, 2018 • edited Loading

Description

TODO before lifting the "In progress" label:

Types of changes

Checklist:

hypest commented Mar 27, 2018

aduth commented Mar 27, 2018

gziolo commented Mar 27, 2018 • edited Loading

hypest commented Mar 28, 2018

hypest commented Mar 29, 2018

hypest commented Apr 30, 2018

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

hypest Apr 30, 2018 • 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

Choose a reason for hiding this comment

hypest commented Apr 30, 2018

gziolo left a comment • edited Loading

Choose a reason for hiding this comment

gziolo commented May 4, 2018

mtias commented May 4, 2018

hypest commented Mar 27, 2018 •

edited

Loading

gziolo commented Mar 27, 2018 •

edited

Loading

hypest Apr 30, 2018 •

edited

Loading

gziolo left a comment •

edited

Loading