-
Notifications
You must be signed in to change notification settings - Fork 56
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/experimental grid block all controls #3444
Changes from all commits
b3dd249
3c52eb7
718fe3b
9cdc8dc
e177aab
4092d0b
fe7ff2d
6b17f3d
c3baaeb
9f71767
8af87b9
03bcb75
3768f52
a6c6167
2d264d6
32d6a3d
0ca7e4f
06754ef
f486894
5496eb1
f14604f
4794ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| ; ignore the submodules | ||
| gutenberg | ||
| block-experiments | ||
| bundle | ||
| jetpack | ||
| bin | ||
|
|
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // This file is to set up the jetpack/layout-grid block that currently lives in block-experiments/blocks/layout-grid | ||
|
|
||
| import { registerBlock } from '../block-experiments/blocks/layout-grid/src'; | ||
|
|
||
| registerBlock(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,9 @@ addAction( 'native.render', 'gutenberg-mobile', ( props ) => { | |
| setupJetpackEditor( | ||
| props.jetpackState || { blogId: 1, isJetpackActive: true } | ||
| ); | ||
| // if ( __DEV__ ) { | ||
| require( './block-experiments-setup' ); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering why this needs to be required dynamically. Could we instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That comment refers to why we would use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good question! I think part of the reason why I included it like this is so that we don't load the js script when the flag is not set. but since we are not downloading the js I guess this part doesn't really matter. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be cool to explore something like this down the road for lazy-loading / code splitting. |
||
| // } | ||
| } ); | ||
|
|
||
| addFilter( 'native.block_editor_props', 'gutenberg-mobile', ( editorProps ) => { | ||
|
|
||
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 just had a thought about the relative import path used here. Not suggesting a change, but wondering if there's a way to scope this (similar to how we resolve
@wordpressdependencies) such that a future change in project structure wouldn't require any changes here. Something like'@something/blocks'🤔If there's too much "magic" in the module resolution configuration, perhaps we'd still achieve a similar effect by importing from an index at a higher level in
../block-experiments? It feels like "reaching deep into" the path here will be brittle to structure changes in the dependency. (This comment might belong there as well as here 😅 ) Wdyt?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 that is a great idea but I am not sure how to go about doing that. Any suggestions?
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.
The second idea would just involve creating an
index.js(or possiblyindex.native.js) file in../block-experiments/blocks/and re-exporting the block registration there. This would shift the responsibility of maintaining the deeper path traversals to the dependency, which probably makes more sense, since that's where the structure changes would occur.Regarding the module resolution magic, there is a plugin that might be worth investigating for this purpose, but I haven't tried it out. It essentially allows you to create an alias for the imports.
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've used that plugin in every React native project I've worked on, it's very useful. Also very easy to set up (usually).