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

Tiled gallery block: load assets #10728

Merged
merged 2 commits into from
Nov 29, 2018
Merged

Conversation

simison
Copy link
Member

@simison simison commented Nov 27, 2018

Load Tiled Gallery Gutenberg block assets.

Testing

  • Apply the commit 9e27e59 to fix loading block assets at appropiate loading order. (PR Gutenberg block: change initialization for later #10739)
  • In Calypso, switch to branch update/tiled-gallery-updates (PR Gutenberg: Tiled gallery block wp-calypso#27458) and build the blocks to get _inc/blocks/tiled-gallery/view* files:
    npm run sdk -- gutenberg client/gutenberg/extensions/presets/jetpack/ \
        --output-dir=~/jetpack/_inc/blocks/
    
  • Insert tiled gallery block in the editor, add some awesome images and publish the post
  • Check the post frontend, you should observe tiled-gallery/view.* files load and the gallery shouldn't look too broken. It might look just a tad bit broken because it's work in progress. ;-)

@simison simison added [Feature] Tiled Gallery A different way to display image galleries on your site, in different organizations and shapes. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack labels Nov 27, 2018
@simison simison added this to the 6.9 milestone Nov 27, 2018
@simison simison requested review from a team November 27, 2018 14:14
@simison simison requested a review from a team as a code owner November 27, 2018 14:14
@matticbot
Copy link
Contributor

D21373-code. (newly created revision)

@@ -17,6 +17,7 @@ public function __construct() {
add_filter( 'jetpack_gallery_types', array( $this, 'jetpack_gallery_types' ), 9 );
add_filter( 'jetpack_default_gallery_type', array( $this, 'jetpack_default_gallery_type' ) );

jetpack_register_block( 'tiled-gallery' );
Copy link
Member Author

@simison simison Nov 27, 2018

Choose a reason for hiding this comment

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

I'm a bit unsure if I should instead do this:

if ( $this->tiles_enabled() ) {
	jetpack_register_block( 'tiled-gallery' );
} else {
	jetpack_register_block(
		'tiled-gallery',
		array(),
		array( 'available' => false, 'unavailable_reason' => 'missing_module' )
	);
}

Copy link
Member

Choose a reason for hiding this comment

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

The idea behind missing_module is that we would have a component that would just allow us to show a toggle to the user to activate the missing module. I am not sure this would work the same way.

I am not too familiar with the tile gallery code and how that would work.
I think the check makes sense but I would just be careful to make sure that reason still make sense in this context.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for elaborating @enejb

I am not too familiar with the tile gallery code and how that would work.

Me neither and it seems like we could leave this for future iterations.

@jetpackbot
Copy link

Warnings
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is automated check which relies on PULL_REQUEST_TEMPLATE.We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS

@simison simison changed the title Tiled gallery block: initialize Tiled gallery block: load assets Nov 28, 2018
@simison
Copy link
Member Author

simison commented Nov 28, 2018

Blocked by #10739 for now solved

- Add wp-rich text dependency for tiled gallery
@simison simison force-pushed the update/load-tiled-gallery-block branch from 1106d71 to 2740f7a Compare November 28, 2018 14:29
@simison simison added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels Nov 28, 2018
@simison simison self-assigned this Nov 28, 2018
@simison
Copy link
Member Author

simison commented Nov 29, 2018

Noting that Since tiled gallery module gets disabled when disabling Photon from jetpack settings, we might need to change something here later on.

Copy link
Contributor

@lezama lezama left a comment

Choose a reason for hiding this comment

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

💥

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

Working 👍 🚢

$dependencies = array(
'lodash',
'wp-i18n',
'wp-token-list',
Copy link
Member

Choose a reason for hiding this comment

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

We'll try to get this list down: Automattic/wp-calypso#28933

@sirreal sirreal added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Nov 29, 2018
@@ -537,7 +537,7 @@ private function __construct() {
* Prepare Gutenberg Editor functionality
*/
require_once JETPACK__PLUGIN_DIR . 'class.jetpack-gutenberg.php';
add_action( 'init', array( 'Jetpack_Gutenberg', 'load_blocks' ) ); // Registers all the Jetpack blocks .
add_action( 'init', array( 'Jetpack_Gutenberg', 'load_blocks' ), 99 ); // Registers all the Jetpack blocks .
Copy link
Member

Choose a reason for hiding this comment

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

We need to make sure this or something similar makes its way to WordPress.com

Fusuion won't handle this.

Copy link
Contributor

Choose a reason for hiding this comment

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

good call

Copy link
Contributor

Choose a reason for hiding this comment

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

@simison could you take care of it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup 👌

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

Looking good. Merging.

@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Nov 29, 2018
@jeherve jeherve deleted the update/load-tiled-gallery-block branch November 29, 2018 13:58
gititon pushed a commit that referenced this pull request Dec 7, 2018
Load Tiled Gallery Gutenberg block assets.

## Testing

- Apply the commit 9e27e59 to fix loading block assets at appropiate loading order. (PR #10739)
- In Calypso, switch to branch `update/tiled-gallery-updates` (PR Automattic/wp-calypso#27458) and build the blocks to get `_inc/blocks/tiled-gallery/view*` files:
    ```
    npm run sdk -- gutenberg client/gutenberg/extensions/presets/jetpack/ \
        --output-dir=~/jetpack/_inc/blocks/
    ```
- Insert tiled gallery block in the editor, add some awesome images and publish the post
- Check the post frontend, you should observe `tiled-gallery/view.*` files load and the gallery shouldn't look too broken. It might look just a tad bit broken because it's work in progress. ;-)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Tiled Gallery A different way to display image galleries on your site, in different organizations and shapes. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants