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

Add post type to editor #1606

Merged
merged 5 commits into from
Nov 29, 2019
Merged

Add post type to editor #1606

merged 5 commits into from
Nov 29, 2019

Conversation

koke
Copy link
Member

@koke koke commented Nov 21, 2019

To be able to show the Page Templates selector only on pages, we need to differentiate between posts and pages. Currently we are hardcoding a postType of post in all cases.

This PR adds the necessary structure for the apps to inject that post type into Gutenberg, but the changes to the apps to actually do that will come in future PRs.

I have changed the bridge code in a way that would be compatible with the existing app implementations (it would just default to post), so these should be fine to merge before we make changes to the apps.

Part of #1463. The proper feature flag is coming up as part of #1576, and we'll need PRs in both apps to finish that issue.

To test:

From WPAndroid, hardcode the "page" postType, or try with the approach in wordpress-mobile/WordPress-Android#10775:

diff --git a/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergContainerFragment.java b/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergContainerFragment.java
index 2d874b7105..1549e391cf 100644
--- a/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergContainerFragment.java
+++ b/libs/editor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergContainerFragment.java
@@ -79,4 +79,5 @@ public class GutenbergContainerFragment extends Fragment {
                 BuildConfig.DEBUG,
                 BuildConfig.BUILD_GUTENBERG_FROM_SOURCE,
+                "page",
                 isNewPost,
                 localeString,

From WPiOS: wordpress-mobile/WordPress-iOS@c7885da

Then check with the JS debugger that the editor is initialized with the right post type.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@koke koke added this to the 1.18 milestone Nov 21, 2019
@koke koke requested a review from pinarol November 21, 2019 15:02
@koke koke marked this pull request as ready for review November 21, 2019 15:03
Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

This is looking pretty good. I was able to verify that postType is passed properly with WPiOS but couldn't get WPAndroid PR working, it seems like it needs update from develop. But we can handle that separately.

@pinarol
Copy link
Contributor

pinarol commented Nov 27, 2019

small note, I had to change

 func gutenbergPostType() -> String? {
         return post is Page ? "page" : "post"
     }

into

 func gutenbergPostType() -> String {
         return post is Page ? "page" : "post"
 }

on WPiOS side, it was just giving a warning but the method was not getting called.

@koke koke merged commit 8bad4ab into develop Nov 29, 2019
@koke koke deleted the add/post-type branch November 29, 2019 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants