-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[RNMobile][Monorepo] Make the native app customizable by a parent app #21332
[RNMobile][Monorepo] Make the native app customizable by a parent app #21332
Conversation
1e09ea8
to
ef169ed
Compare
Size Change: 0 B Total Size: 828 kB ℹ️ View Unchanged
|
4c2b6b2
to
2541642
Compare
…squash' into rnmobile/experiment-monorepo-new-setup
…ps from the parent app or not
57e7ad6
to
2cfee15
Compare
import { cloneElement } from './react'; | ||
|
||
const render = ( element, id ) => | ||
AppRegistry.registerComponent( id, () => ( propsFromParent ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why not to execute all this logic inside initializeEditor in the @wordpress/edit-post package?
Overriding React methods and adding WP hooks might have some performance implications but I guess it’s called only once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's to be able to customize the editor from an outside package. In this case a repo that's importing Gutenberg as a submodule. Correct me if I'm wrong @Tug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we could totally have this logic in @wordpress/edit-post
(or in @wordpress/react-native-editor
as it was the case in the monorepo branch before this PR).
The idea here is simply to get closer to having @wordpress/edit-post
work as cross-platform package. If you look at edit-post/src/index.native.js
you'll see that the init code is much closer to the web one now.
Overriding React methods and adding WP hooks might have some performance implications but I guess it’s called only once.
We're not overriding anything though, the render
function wasn't defined for native prior to this. And yes it's only rendered once on load.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It probably doesn't matter as much then now, but I guess initializeEditor
would be the best place to run all those side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready to merge, using the usual “Create a merge commit” option for preserving history on our feat/import-gutenberg-mobile-no-squash
branch
Will now merge with "Use merge commit" strategy and revert back to "squash/rebase strategy" right after. |
Description
This PR does multiple changes which allows external native apps to reuse and customize the native block editor.
The main repo which customizes WordPress apps for Android and iOS is still gutenberg-mobile.
Here is the related PR for this integration: wordpress-mobile/gutenberg-mobile#2172
List of changes:
render
method in react platform to mimick the web API. Update the code inedit-post
to use this new methodrender
method to customize root component props and run pre render initialization codepackages/react-native-bridge/android/build.gradle
to make WPAndroid build (switch from yarn to npm, update paths...)packages/react-native-bridge/android/vendor/hermes-engine
so we can build WPAndroid using jitpack and a jsbundle without runningnpm install
How has this been tested?
See Gutenberg mobile PR
Types of changes
React native changes
Checklist: