Add post title to Gutenberg Mobile #450

diegoreymendez · 2018-12-26T16:10:10Z

Description:

Implements the editor title in RN for Gutenberg, also removes the native title.

This PR also adds the Noto font family to the repository.

Focus points:

For @hypest and anyone else wanting to chime in:

Please review if the way in which I'm storing the title in AppContainer.js makes sense. I'm not sure how it would work to have that in redux so I just went for a simple initial solution.
My idea in this PR is to offer a working solution so we have a good baseline to work on. While I do want to make this perfect, I also suggest that we try to be a bit optimistic with this and work incrementally on things that are not too urgent / serious. Just a though since the PRs for this feature are already quite complicated.

Related PRs:

gutenberg: WordPress/gutenberg#13199
WPiOS: wordpress-mobile/WordPress-iOS#10794
WPAndroid: wordpress-mobile/WordPress-Android#9027

Testing:

Test 1:

Run the app.
Edit the title and make sure the state is properly maintained.
Tap save.

Check the iOS console log that the title is properly retrieved. (not sure if there's a similar log in Android)

Test 2:

Run the app.
Delete the title and make sure the placeholder is shown.

…er plays nicely with it.

pinarol · 2019-01-15T13:40:32Z

@diegoreymendez Just wanted to check, did you update the JS bundles intentionally for this PR or are they some side effect of merges? If the second one is true I'd say we can just reset those changes. They seem to conflict anyway.

pinarol · 2019-01-15T15:23:04Z

Currently selected block seems to stay selected even if we focus to title. We can also handle this on a separate PR If you want, it's also related with propagating changes to the store. (dispatching clearSelectedBlock to the store should do it)

hypest · 2019-01-15T15:40:41Z

src/app/initial-html.js

@@ -17,7 +17,7 @@ Hello World
 <!-- /wp:title -->

 <!-- wp:heading {"level": 2} -->
-<h2>Welcome to Gutenberg</h2>
+<h2>What is Gutenberg?</h2>


Is this a change needed for the PR? Asking since it introduces a trivial chance in a test as well and, not sure it's important for some reason.

Well only because the post looks weird with "Welcome to Gutenberg" written twice. Once in the title and the other in this header down in the content.

Ah, cool, I see.

diegoreymendez · 2019-01-15T15:50:43Z

@pinarol -

Just wanted to check, did you update the JS bundles intentionally for this PR or are they some side effect of merges? If the second one is true I'd say we can just reset those changes. They seem to conflict anyway.

I updated them on purpose so that testing this PR integrated into WPiOS is easier. I can regenerate them before merging the PR if all looks fine.

Currently selected block seems to stay selected even if we focus to title

That's interestingly broken. I'll look into this.

diegoreymendez · 2019-01-16T21:34:48Z

@pinarol - All of the issues with title focus should be resolved now. I believe I've covered all of the existing feedback from your end, but let me know if I missed anything.

Thanks for the help. This is ready to review again.

pinarol

Tested with steps on WPiOS PR, works all good! 🎉

I just have a small comment in gutenberg side, it will make it possible to access the title as @Tug described here

but feel free to create an issue for it and handle that separately, at this time it is enough if we just don't forget about it.

… bridge code, and use it the glue code called from the parent wp-android app.

…o-gb-mobile Android - Add post title to Gutenberg Mobile

daniloercoli · 2019-01-24T10:22:55Z

src/app/AppContainer.js

 		const html = serialize( this.props.getBlocks() );
-		RNReactNativeGutenbergBridge.provideToNative_Html( html, this.lastHtml !== html );
+		const hasChanges = this.state.title !== this.lastTitle || this.lastHtml !== html;


Noticed that hasChanges is returned to the Android (wp-android) side as false, even if i changed the content of the post in the editor. The first time the native side calls the method to retrieve the content from RN, true is returned, and then the next calls to retrieve the content return false.

Is this a regression or is this a problem with the original logic (before my changes) too?

The reason I'm asking is that the condition and logic haven't changed much in terms of what happens when you edit the content, and if this issue was part of the original logic, fixing it is outside of the purpose of this PR.

…ng content from the React side. This was necessary since the call to retrieve content and title are separate on Android.

…rg-mobile into issue/372-add-title-to-gutenberg-mobile * 'develop' of https://github.com/wordpress-mobile/gutenberg-mobile: Make sure to use `setContent` when initializating the GlueCode Dock the keyboard hide button to the right side of the screen (#480) RNAztecView: Removing branch spec from podspec Moving RNTAztecView.podspec to the gutenberg-mobile root dir. Aztec iOS: Force send height information to JS after pasting text # Conflicts: # gutenberg # react-native-gutenberg-bridge/android/src/main/java/org/wordpress/mobile/WPAndroidGlue/WPAndroidGlueCode.java

diegoreymendez · 2019-01-24T21:23:45Z

Merging, but I'll be going through these PRs tomorrow to list any pending work.

Thanks for the feedback everyone.

diegoreymendez added 3 commits December 26, 2018 11:36

WIP: adding the post title to the editor component.

8c89460

WIP: The title is now properly formatted and the default block append…

6c5d701

…er plays nicely with it.

Lint fixes.

eb361cb

diegoreymendez added the [Status] Not Ready For Review label Dec 26, 2018

diegoreymendez self-assigned this Dec 26, 2018

diegoreymendez added 3 commits January 3, 2019 13:40

WIP: stashing.

d80dba6

Implements an initial version of post-title in gutenberg.

b788654

Updates the gutenberg submodule.

16181fd

diegoreymendez mentioned this pull request Jan 4, 2019

Add title to gutenberg mobile WordPress/gutenberg#13199

Merged

5 tasks

diegoreymendez added 18 commits January 4, 2019 19:09

Updates the gutenberg submodule reference.

739cf56

Adds an interface for setting the post title from native.

ea8288c

The title can now be set from the parent app.

818325f

Adds initial title to the datasource.

fc425ca

Fixes an issue with the initial setup of the title.

99fe984

WIP: implementing js -> native title updates.

e4e4ed5

Now updates the title through provideToNative_Html.

dcca2c6

Fixes an issue that was causing the title not to report changes.

271da87

Fixes a mistake in the previous commit.

469c3fc

Improvements to set the title in our RN code.

46a20ca

Fixes the appearance of the title and separator.

efceada

Updates the gutenberg submodule.

b53123a

Merge branch 'master' into issue/372-add-title-to-gutenberg-mobile

ebaf39e

Fixes some issues with the linting.

6f723db

Updates the gutenberg submodule.

e31e844

Fixes some breaking unit tests.

7ca5656

Merges the latest from develop.

f487db8

Adds NotoSerif to the demo project.

bec3537

diegoreymendez mentioned this pull request Jan 10, 2019

Implements the editor title in RN for Gutenberg. wordpress-mobile/WordPress-iOS#10794

Merged

Regenerated the bundles.

e98f6fc

diegoreymendez requested a review from etoledom January 10, 2019 22:00

hypest reviewed Jan 15, 2019

View reviewed changes

Updates to the latest changes in Gutenberg to handle title focusing.

10e112a

diegoreymendez added 6 commits January 17, 2019 10:07

Updates the gutenberg submodule.

c29bdca

Updates the gutenberg submodule.

87dfddf

Merges the latest from develop.

5c97308

Fixes a bug that was causing the title not to update anymore.

21f39b6

WIP: Adds two symlinks to try and fix the breaking tests.

1d64d25

Adds symlinks to fix issues with jest.

1062340

pinarol approved these changes Jan 17, 2019

View reviewed changes

Modify the interface for setting the content/title from native in the…

1e6776a

… bridge code, and use it the glue code called from the parent wp-android app.

This was referenced Jan 21, 2019

Implements the editor title in RN for Gutenberg wordpress-mobile/WordPress-Android#9027

Merged

Android - Add post title to Gutenberg Mobile #482

Merged

diegoreymendez and others added 2 commits January 21, 2019 10:16

Merge pull request #482 from wordpress-mobile/372-android-add-title-t…

6968c62

…o-gb-mobile Android - Add post title to Gutenberg Mobile

Update RNAztecWrapper hash

3293e97

daniloercoli mentioned this pull request Jan 22, 2019

[Android] Add the ability to set font family, weight, size, and style from RN wordpress-mobile/react-native-aztec#114

Merged

Merged the latest from develop.

0b5e9b1

daniloercoli removed the [Status] Needs Android Dev label Jan 23, 2019

daniloercoli reviewed Jan 24, 2019

View reviewed changes

daniloercoli added 3 commits January 24, 2019 15:55

[Android] - Make sure mContentChanged has the correct value when aski…

e7b3b8f

…ng content from the React side. This was necessary since the call to retrieve content and title are separate on Android.

Update GB hash

ec1418f

diegoreymendez merged commit 1559268 into develop Jan 24, 2019

diegoreymendez deleted the issue/372-add-title-to-gutenberg-mobile branch January 24, 2019 21:38

diegoreymendez mentioned this pull request Jan 25, 2019

Uses the global store for the post title and adds undo/redo support. #501

Merged

koke mentioned this pull request Jan 25, 2019

Post title writing flow #372

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add post title to Gutenberg Mobile #450

Add post title to Gutenberg Mobile #450

diegoreymendez commented Dec 26, 2018 •

edited

Loading

pinarol commented Jan 15, 2019

pinarol commented Jan 15, 2019

hypest Jan 15, 2019

diegoreymendez Jan 15, 2019

hypest Jan 15, 2019

diegoreymendez commented Jan 15, 2019

diegoreymendez commented Jan 16, 2019

pinarol left a comment

daniloercoli Jan 24, 2019

diegoreymendez Jan 24, 2019 •

edited

Loading

diegoreymendez commented Jan 24, 2019

Add post title to Gutenberg Mobile #450

Add post title to Gutenberg Mobile #450

Conversation

diegoreymendez commented Dec 26, 2018 • edited Loading

Description:

Focus points:

Related PRs:

Testing:

Test 1:

Test 2:

pinarol commented Jan 15, 2019

pinarol commented Jan 15, 2019

hypest Jan 15, 2019

Choose a reason for hiding this comment

diegoreymendez Jan 15, 2019

Choose a reason for hiding this comment

hypest Jan 15, 2019

Choose a reason for hiding this comment

diegoreymendez commented Jan 15, 2019

diegoreymendez commented Jan 16, 2019

pinarol left a comment

Choose a reason for hiding this comment

daniloercoli Jan 24, 2019

Choose a reason for hiding this comment

diegoreymendez Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

diegoreymendez commented Jan 24, 2019

diegoreymendez commented Dec 26, 2018 •

edited

Loading

diegoreymendez Jan 24, 2019 •

edited

Loading