Contact Info jetpack native blocks behind DEV flag #1934

Tug · 2020-02-19T13:42:54Z

This is a work in progress

See Jetpack PR: Automattic/jetpack#15389

PR submission checklist:

I have considered adding unit tests where possible.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

cameronvoell · 2020-04-08T02:36:54Z

Issues remaining:

Save a block and return, it should still show as a contact block - b3c364c
Add a contact info on the web, edit on mobile, it should work - b3c364c
Fix weird focus problem where you can type in address/email/phone without the inner block showing as selected Automattic/jetpack@97092d7

cameronvoell · 2020-04-09T09:03:19Z

src/jetpack-editor-setup.js

@@ -19,7 +19,7 @@ const supportedJetpackBlocks = {
 	},
 };

-const setJetpackData = async ( {


Without removing this, we would start parsing html to blocks before the jetpack blocks got registered. I'm not sure if this changes our existing thoughts on when we should check for jetpack permission. Perhaps it should be done on the app side. But it will at least have to block the parsing of HTML to blocks otherwise existing jetpack blocks will show up as unknown blocks.

Agreed block parsing (and thus loading the editor) should be done afterwards. We can delay the loading of the editor for a bit but it wouldn't be a great UX if it happened each time. It would be nice if we could get jetpack data from the parent app, or at least cache here it somehow.

Tug · 2020-04-16T14:25:46Z

src/index.js

@@ -55,6 +56,7 @@ export class RootComponent extends React.Component {
 		super( props );
 		setupLocale( props.locale, props.translations );
 		setupApiFetch();
+		setupJetpackEditor( props.jetpackState || { blogId: 1, isJetpackActive: true } );


We'll have to make this async when we actually fetch the data from the site, but for now let's keep it that way 👍

Sounds good. Just going to note here that if we make an async call to check if we're enabling jetpack blocks, then we need to block the action of parsing raw html to blocks until after that function returns, otherwise jetpack blocks will show up as not enabled, even if api response says they should be enabled.

…-blocks

Tug · 2020-04-16T14:39:16Z

src/jetpack-editor-setup.js

+
+	const jetpackData = setJetpackData( jetpackState );
+
+	require( '../jetpack/extensions/editor' );


we need to add a DEV flag here

cameronvoell · 2020-04-17T01:58:03Z

@iamthomasbishop As we mentioned in the chat this feature is initially being added behind a DEV flag while we iterate on some of the API requirements. In the mean time, it can be tested in it's current state on iOS using AppCenter build number: 27246 (test PR with link here).

I created this issue for tracking design review and api requirements for enabling this block in production: #2166

cameronvoell

Verified it's disabled behind the DEV flag, looks good! I added one comment and one question that is related to code that's likely remaining for the API code that is still in progress. I believe we're ready to merge though.

cameronvoell · 2020-04-17T02:01:52Z

src/index.js

@@ -55,6 +56,7 @@ export class RootComponent extends React.Component {
 		super( props );
 		setupLocale( props.locale, props.translations );
 		setupApiFetch();
+		setupJetpackEditor( props.jetpackState || { blogId: 1, isJetpackActive: true } );


Sounds good. Just going to note here that if we make an async call to check if we're enabling jetpack blocks, then we need to block the action of parsing raw html to blocks until after that function returns, otherwise jetpack blocks will show up as not enabled, even if api response says they should be enabled.

cameronvoell · 2020-04-17T02:34:47Z

src/jetpack-editor-setup.js

+		tracksUserData: userData,
+		wpcomBlogId: blogId,
+	};
+	global.window[ JETPACK_DATA_PATH ] = jetpackEditorInitialState;


Just want to ask you @Tug about this line since it's hard to understand with the API calls temporarily disabled. Are we utilizing global.window here so jetpack code can access jetpackEditorInitialState when initializing, similar to how they're doing it on the web side in jetpack/extensions/editor.js?

maxme · 2020-04-21T09:37:38Z

I was wondering if the bundle went bigger with the dependency to Jetpack (and sub-dependencies). Can we also check if loading times went up?

cameronvoell · 2020-04-22T07:07:57Z

I was wondering if the bundle went bigger with the dependency to Jetpack (and sub-dependencies). Can we do a sanity check

@maxme I just performed yarn bundle on gutenberg-mobile develop before and after this PR is merged and I didn't see any increase in the size of the bundle directory.

Before jetpack PR (commit ca78090):
5.0M bundle/ios/assets
24M bundle/ios
5.0M bundle/android/raw
24M bundle/android
48M bundle

After jetpack PR (commit fd39dd9)
5.0M bundle/ios/assets
24M bundle/ios
5.0M bundle/android/raw
24M bundle/android
48M bundle

also verify if the loading times?
I can check this too, but figure it's worth clarifying first since I think you might have had a typo. If I'm understanding you correctly, a robust check of bundle load times would be to test how long it takes to launch the gutenberg editor in WPiOS and WPAndroid before and after the jetpack code is added. Let me know if that's what you meant, or it was something else.

cameronvoell · 2020-04-23T05:47:21Z

See differences in App.js files as well, before and after the update. Looks like it should be negligible.

	Before Jetpack	After Jetpack	differences
android App.js	8523434	8523915	-481
android App.js.map	11217927	11231035	-13108

ios App.js	8541370	8541748	-378
ios App.js.map	11269901	11283125	-13224

maxme · 2020-04-23T08:38:55Z

Thanks @cameronvoell - By also verify if the loading times? I meant Can we also check if loading times went up? ;) - but let's forget that since the bundle size didn't change much.

Tug added 2 commits February 19, 2020 14:41

Jetpack editor basic setup

369c173

prettier

4527d76

Tug requested review from cameronvoell and ceyhun February 19, 2020 13:43

Tug self-assigned this Feb 19, 2020

Tug added 2 commits February 20, 2020 10:46

Add support for contact-info inner blocks: address email and phone

99fb15a

Try fetching /gutenberg/available-extensions

d1c4d90

Tug force-pushed the try/jetpack-native-blocks branch from a310bc4 to d1c4d90 Compare February 26, 2020 14:43

Explicitely import native compatible blocks

9a52618

Tug marked this pull request as ready for review February 26, 2020 15:30

Tug and others added 5 commits March 18, 2020 13:05

Skip apiFetch for now while WP REST API Auth is not yet supported

ded8fff

Update jetpack reference

2cbb7a6

Merge branch 'develop' into try/jetpack-native-blocks

d5c1c95

Update gutenberg ref

81d7799

Register jetpack blocks before html parsing

b3c364c

Update jetpack ref to fix inner block focus

98d5259

cameronvoell mentioned this pull request Apr 9, 2020

Mobile Contact Info Block Automattic/jetpack#15389

Merged

cameronvoell reviewed Apr 9, 2020

View reviewed changes

Tug added 2 commits April 15, 2020 16:37

Update jetpack ref

ce92adf

Update jetpack ref

064013a

Tug commented Apr 16, 2020

View reviewed changes

Tug added 3 commits April 16, 2020 16:26

Remove unused dependency in jetpack-editor-setup

96d86b9

Target final jetpack ref in master

6bfa3ab

Merge remote-tracking branch 'origin/develop' into try/jetpack-native…

cb97bae

…-blocks

Tug added the .com/.org code separation label Apr 16, 2020

Tug assigned cameronvoell and unassigned Tug Apr 16, 2020

Tug commented Apr 16, 2020

View reviewed changes

Hide in production

f0d0b84

cameronvoell mentioned this pull request Apr 17, 2020

Contact Info Jetpack enabled in Production #2166

Closed

2 tasks

cameronvoell approved these changes Apr 17, 2020

View reviewed changes

cameronvoell changed the title ~~Try jetpack native blocks~~ Contact Info jetpack native blocks behind DEV flag Apr 17, 2020

cameronvoell merged commit fd39dd9 into develop Apr 17, 2020

Tug deleted the try/jetpack-native-blocks branch April 21, 2020 09:38

cameronvoell restored the try/jetpack-native-blocks branch April 21, 2020 16:53

Tug deleted the try/jetpack-native-blocks branch April 21, 2020 16:59

This was referenced Apr 27, 2020

Gutenberg: update to v1.27.0 wordpress-mobile/WordPress-Android#11756

Merged

Gutenberg: update to v1.27.0 wordpress-mobile/WordPress-iOS#14002

Merged

Release v1.27.0 #2186

Merged

fluiddot mentioned this pull request Aug 17, 2021

TT | 3471 | "Introduce allow-list and disallow-list for blocks." #3811

Draft

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contact Info jetpack native blocks behind DEV flag #1934

Contact Info jetpack native blocks behind DEV flag #1934

Tug commented Feb 19, 2020 •

edited by cameronvoell

Loading

cameronvoell commented Apr 8, 2020 •

edited

Loading

cameronvoell Apr 9, 2020

Tug Apr 9, 2020

Tug Apr 16, 2020

cameronvoell Apr 17, 2020

Tug Apr 16, 2020

cameronvoell commented Apr 17, 2020

cameronvoell left a comment

cameronvoell Apr 17, 2020

cameronvoell Apr 17, 2020

maxme commented Apr 21, 2020 •

edited

Loading

cameronvoell commented Apr 22, 2020

cameronvoell commented Apr 23, 2020

maxme commented Apr 23, 2020


		const jetpackData = setJetpackData( jetpackState );

		require( '../jetpack/extensions/editor' );

Contact Info jetpack native blocks behind DEV flag #1934

Contact Info jetpack native blocks behind DEV flag #1934

Conversation

Tug commented Feb 19, 2020 • edited by cameronvoell Loading

cameronvoell commented Apr 8, 2020 • edited Loading

cameronvoell Apr 9, 2020

Choose a reason for hiding this comment

Tug Apr 9, 2020

Choose a reason for hiding this comment

Tug Apr 16, 2020

Choose a reason for hiding this comment

cameronvoell Apr 17, 2020

Choose a reason for hiding this comment

Tug Apr 16, 2020

Choose a reason for hiding this comment

cameronvoell commented Apr 17, 2020

cameronvoell left a comment

Choose a reason for hiding this comment

cameronvoell Apr 17, 2020

Choose a reason for hiding this comment

cameronvoell Apr 17, 2020

Choose a reason for hiding this comment

maxme commented Apr 21, 2020 • edited Loading

cameronvoell commented Apr 22, 2020

cameronvoell commented Apr 23, 2020

maxme commented Apr 23, 2020

Tug commented Feb 19, 2020 •

edited by cameronvoell

Loading

cameronvoell commented Apr 8, 2020 •

edited

Loading

maxme commented Apr 21, 2020 •

edited

Loading