Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 42 commits into from
May 4, 2018
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
850497c
Android specific files for the Code block
hypest Mar 15, 2018
43590b2
Breaking change: Don't override the default JSX translation
hypest Mar 15, 2018
bf82dce
Add the platform specific files for iOS as well
hypest Mar 19, 2018
16b7eb5
Set TextInput to multiline for Code block
hypest Mar 21, 2018
20fd557
Hide the default TextInput underline on Android
hypest Mar 21, 2018
f922044
Expose registerBlockType, getBlockType to the RN app
hypest Mar 21, 2018
8ee1a4e
Use .native.js for common native code
hypest Mar 28, 2018
3e81e6b
Merge branch 'master' into rnmobile/port-code-block
hypest Mar 28, 2018
9a2c431
Correct configuration for the babel plugins settings
hypest Mar 28, 2018
0f3b9ea
transform-react-jsx for the production env as well
hypest Mar 28, 2018
0aaba3d
No need for pragma override
hypest Mar 29, 2018
a3d5ab4
Ignore mobile native files when linting
hypest Mar 29, 2018
a29e5bc
Revert "Ignore mobile native files when linting"
hypest Mar 30, 2018
e37c8bc
Comment out ununsed imports
hypest Mar 30, 2018
a6f8089
Fix codestyle
hypest Mar 30, 2018
c190e24
Import is compatible anyway, uncomment it
hypest Mar 30, 2018
c96b10b
Remove the React import. The RN project auto-imports it.
hypest Mar 30, 2018
685d178
Limit native code to the edit function alone
hypest Mar 30, 2018
f3e8a47
Merge branch 'master' into rnmobile/port-code-block
hypest Apr 4, 2018
ad934ad
Expose getBlockContent to RN
hypest Apr 4, 2018
9b5a3f8
RN app tests need createElement exported
hypest Apr 5, 2018
a2a1a04
Export serialize from serializer
hypest Apr 10, 2018
95b17a2
Load native version of plain-text's CSS module
hypest Apr 12, 2018
737a498
Make it clear that RN uses styles, not classnames
hypest Apr 12, 2018
1ab7542
Merge branch 'rnmobile/react-native-mobile-integration-first-steps' i…
maxme Apr 17, 2018
49ceaa8
Use the i18n package from @wordpress
maxme Apr 17, 2018
836c51e
Remove some commented out code
hypest Apr 27, 2018
ce3c5c4
Utils are not currently used on native
hypest Apr 27, 2018
33f9764
'Border' property not supported in shorthand form
hypest Apr 27, 2018
2bbcbc8
Use hypens in css class names as traditionally
hypest Apr 27, 2018
837f029
remove unused code
hypest Apr 27, 2018
2a98232
No need for platform-specific factory.js
hypest Apr 27, 2018
d530570
Merge branch 'master' into rnmobile/port-code-block
hypest Apr 27, 2018
eed0531
Move native files after update from master
hypest Apr 30, 2018
779c342
Codestyle fix
hypest Apr 30, 2018
2a97aed
GB isn't using Prettier so, no need for the pragma
hypest Apr 30, 2018
489ae72
Comment on the need to import the `edit` function
hypest Apr 30, 2018
0edfb18
Export the edit function as default
hypest Apr 30, 2018
9b68ba3
Remove reduntant line comment
hypest Apr 30, 2018
5c93331
Import block using the new pattern
hypest Apr 30, 2018
0377c73
Add a comment about styling
hypest Apr 30, 2018
dfb1a12
Use object property shorthand for `edit`
gziolo May 4, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions blocks/api/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export {
createBlock,
} from './factory';
export {
default as serialize,
getBlockContent,
} from './serializer';
export {
registerBlockType,
getBlockType,
} from './registration';
12 changes: 12 additions & 0 deletions blocks/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// A "block" is the abstract term used to describe units of markup that,
// when composed together, form the content or layout of a page.
// The API for blocks is exposed via `wp.blocks`.
//
// Supported blocks are registered by calling `registerBlockType`. Once registered,
// the block is made available as an option to the editor interface.
//
// Blocks are inferred from the HTML source of a post through a parsing mechanism
// and then stored as objects in state, from which it is then rendered for editing.
export * from './api';

export { default as PlainText } from './plain-text';
21 changes: 21 additions & 0 deletions blocks/plain-text/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* External dependencies
*/
import { TextInput } from 'react-native';

/**
* Internal dependencies
*/
import styles from './style.scss';

function PlainText( { onChange, className, ...props } ) {
return (
<TextInput
className={ [ styles[ 'blocks-plain-text' ], className ] }
onChangeText={ ( text ) => onChange( text ) }
{ ...props }
/>
);
}

export default PlainText;
9 changes: 9 additions & 0 deletions blocks/plain-text/style.native.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.blocks-plain-text {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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!

box-shadow: none;

border-width: 0;

padding: 0;
margin: 0;
width: 100%;
}
23 changes: 23 additions & 0 deletions core-blocks/code/edit.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import './editor.scss';
import { PlainText } from '@wordpress/blocks';

export function edit( { attributes, setAttributes, className } ) {
return (
<div className={ className }>
<PlainText
value={ attributes.content }
onChange={ ( content ) => setAttributes( { content } ) }
placeholder={ __( 'Write code…' ) }
aria-label={ __( 'Code' ) }
/>
</div>
);
}
28 changes: 28 additions & 0 deletions core-blocks/code/edit.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { View } from 'react-native';

/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { PlainText } from '../../blocks';

export function edit( { attributes, setAttributes, style } ) {
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, done with 0edfb18.

return (
<View>
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@hypest hypest Apr 30, 2018

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done with 0377c73.

<PlainText
value={ attributes.content }
style={ style }
multiline={ true }
underlineColorAndroid="transparent"
onChange={ ( content ) => setAttributes( { content } ) }
placeholder={ __( 'Write code…' ) }
aria-label={ __( 'Code' ) }
/>
</View>
);
}

16 changes: 2 additions & 14 deletions core-blocks/code/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
import { __ } from '@wordpress/i18n';
import {
createBlock,
PlainText,
} 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.
Copy link
Member

Choose a reason for hiding this comment

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

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,
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


export const name = 'core/code';

Expand Down Expand Up @@ -55,18 +54,7 @@ export const settings = {
],
},

edit( { attributes, setAttributes, className } ) {
return (
<div className={ className }>
<PlainText
value={ attributes.content }
onChange={ ( content ) => setAttributes( { content } ) }
placeholder={ __( 'Write code…' ) }
aria-label={ __( 'Code' ) }
/>
</div>
);
},
edit: edit,

save( { attributes } ) {
return <pre><code>{ attributes.content }</code></pre>;
Expand Down
19 changes: 19 additions & 0 deletions core-blocks/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* WordPress dependencies
*/
import {
registerBlockType,
} from '@wordpress/blocks';

/**
* Internal dependencies
*/
import * as code from './code';

export const registerCoreBlocks = () => {
[
code,
].forEach( ( { name, settings } ) => {
registerBlockType( name, settings );
} );
};
40 changes: 40 additions & 0 deletions element/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/**
* External dependencies
*/
import {
createElement,
Component,
} from 'react';

/**
* Internal dependencies
*/
import serialize from './serialize';

/**
* Returns a new element of given type. Type can be either a string tag name or
* another function which itself returns an element.
*
* @param {?(string|Function)} type Tag name or element creator
* @param {Object} props Element properties, either attribute
* set to apply to DOM node or values to
* pass through to element creator
* @param {...WPElement} children Descendant elements
*
* @return {WPElement} Element.
*/
export { createElement };

/**
* A base class to create WordPress Components (Refs, state and lifecycle hooks)
*/
export { Component };

/**
* Renders a given element into a string.
*
* @param {WPElement} element Element to render
*
* @return {string} HTML.
*/
export { serialize as renderToString };