From 8e5701c2702403f91ea82f672771e6502a71ce3f Mon Sep 17 00:00:00 2001 From: Anthony Ledesma <30462574+AnthonyLedesma@users.noreply.github.com> Date: Mon, 28 Aug 2023 14:44:33 -0700 Subject: [PATCH] Fix Lightbox for Masonry block (#2565) * fix lightbox and add tests to catch this issue * tests are false positive. Correct tests add helper * fix comment spacing --- .dev/tests/cypress/helpers.js | 19 +++++ includes/class-coblocks-block-assets.php | 26 ++++--- includes/class-coblocks-register-blocks.php | 22 ++++++ package.json | 4 +- src/blocks/gallery-masonry/block.json | 1 + .../test/lightbox-controls-gallery.cypress.js | 4 +- .../test/lightbox-controls-image.cypress.js | 4 +- .../test/lightbox-controls-masonry.cypress.js | 72 +++++++++++++++++++ 8 files changed, 134 insertions(+), 18 deletions(-) create mode 100644 src/extensions/lightbox-controls/test/lightbox-controls-masonry.cypress.js diff --git a/.dev/tests/cypress/helpers.js b/.dev/tests/cypress/helpers.js index 81c74f95ee9..f8db424cd17 100644 --- a/.dev/tests/cypress/helpers.js +++ b/.dev/tests/cypress/helpers.js @@ -290,6 +290,25 @@ export function selectBlock( name ) { } ); } +/** + * Helper function to set the block alignment. + * + * @param {string} alignment The alignment to set. + * + */ +export function setBlockAlignment( alignment ) { + // Open alignment toolbar for selected block. + cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).click(); + + if ( alignment !== 'wide' && alignment !== 'full' ) { // Label prefixed with "Align". + alignment = `Align ${ alignment }`; + } else { // Label starts with capitalized letter. + alignment = alignment.charAt( 0 ).toUpperCase() + alignment.slice( 1 ); + } + + cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).contains( alignment ).click(); +} + /** * Set a value within the input box * diff --git a/includes/class-coblocks-block-assets.php b/includes/class-coblocks-block-assets.php index 3d2e7ff91e9..dd38f6007cf 100644 --- a/includes/class-coblocks-block-assets.php +++ b/includes/class-coblocks-block-assets.php @@ -142,7 +142,7 @@ public function block_assets() { wp_register_style( 'coblocks-frontend', COBLOCKS_PLUGIN_URL . $filepath . $rtl . '.css', - array(), + $asset_file['dependencies'], $asset_file['version'] ); @@ -154,8 +154,8 @@ public function block_assets() { wp_enqueue_style( 'coblocks-extensions', COBLOCKS_PLUGIN_URL . $filepath . $rtl . '.css', - array(), - $asset_file['version'] + $asset_file['dependencies'], + $asset_file['version'], ); } @@ -410,6 +410,7 @@ public function frontend_scripts() { ); } + // Gist block script. wp_register_script( 'coblocks-gist-script', $this->assets_dir . 'coblocks-gist-script.js', @@ -501,6 +502,17 @@ public function frontend_scripts() { true ); + $name = 'coblocks-lightbox'; + $filepath = 'dist/js/' . $name; + $asset_file = $this->get_asset_file( $filepath ); + wp_register_script( + 'coblocks-lightbox', + $this->assets_dir . 'coblocks-lightbox.js', + $asset_file['dependencies'], + $asset_file['version'], + true + ); + $this->localize_lightbox_controls(); } @@ -538,13 +550,7 @@ public function coblocks_enqueue_scripts_for_core_blocks( $block_content, $block ) ) { - wp_enqueue_script( - 'coblocks-lightbox', - $this->assets_dir . 'coblocks-lightbox.js', - array(), - COBLOCKS_VERSION, - true - ); + wp_enqueue_script( 'coblocks-lightbox' ); // Script must be localized after the 'handle' script is registered. // `coblocks-lightbox` is the handle that is shared between the scripts. diff --git a/includes/class-coblocks-register-blocks.php b/includes/class-coblocks-register-blocks.php index 21a94a10f3f..db57e052d63 100644 --- a/includes/class-coblocks-register-blocks.php +++ b/includes/class-coblocks-register-blocks.php @@ -108,6 +108,23 @@ public function load_block_metadata( $block_name, $is_child_block = false ) { return $metadata; } + /** + * Some blocks are conditionally registered this function determines if block should be conditional. + * Returns true if restricted and false otherwise. + * + * @param string $path The path to the block.json file. + * @return bool + */ + public function is_excluded_block_filepaths( $path ) { + $restricted = array( '/gallery-masonry/v1' ); + foreach ( $restricted as $r ) { + if ( strpos( $path, $r ) !== false ) { + return true; + } + } + return false; + } + /** * Build a block manifest. The resulting array is used to register blocks and parses block.json files. * For now we need to collect the `render` property from the block.json files and require manually. @@ -125,6 +142,11 @@ public function load_block_manifest() { foreach ( $block_json_paths as $block_json_path ) { $block_json_files = glob( $block_json_path, GLOB_NOSORT ); foreach ( $block_json_files as $block_json_file ) { + // We conditionally load some block for backward compat. + if ( $this->is_excluded_block_filepaths( $block_json_file ) ) { + continue; + } + $block_json = json_decode( file_get_contents( $block_json_file ), true ); $manifest[ $block_json['name'] ]['path'] = str_replace( '/block.json', '', $block_json_file ); diff --git a/package.json b/package.json index 75733e023db..ca6a12095bf 100644 --- a/package.json +++ b/package.json @@ -32,8 +32,8 @@ "json2po": "cd languages && find . -name '*.json' ! -name 'coblocks*.json' -execdir /bin/bash -c 'FROM=\"$0\" && TO=\"coblocks-`basename $0 .json`.po\" && echo \"$FROM > $TO\" && ../vendor/bin/json2po coblocks.json $FROM $TO && msgmerge --previous $TO coblocks.pot > $TO-msgmerge && mv $TO-msgmerge $TO && if [[ \"$OSTYPE\" == \"darwin\"* ]]; then sed -i \"\" -e \"/^#, fuzzy$/d\" $TO; else sed -i -e \"/^#, fuzzy$/d\" $TO; fi;' '{}' \\;", "lint:css": "wp-scripts lint-style", "lint:js": "wp-scripts lint-js", - "lint:php": "wp-env run tests-cli --env-cwd=/var/www/html composer run lint -d /var/www/html/wp-content/plugins/coblocks/; #we use phpunit container because composer container only use php 8;", - "lint:php:fix": "wp-env run tests-cli --env-cwd=/var/www/html composer run lint:fix -d /var/www/html/wp-content/plugins/coblocks/; #we use phpunit container because composer container only use php 8;", + "lint:php": "wp-env run tests-cli --env-cwd=/var/www/html composer run lint -- -d /var/www/html/wp-content/plugins/coblocks/; #we use phpunit container because composer container only use php 8;", + "lint:php:fix": "wp-env run tests-cli --env-cwd=/var/www/html composer run lint:fix -- -d /var/www/html/wp-content/plugins/coblocks/; #we use phpunit container because composer container only use php 8;", "makepot": "./vendor/bin/wp i18n make-pot . --skip-audit --exclude=\".dev,.github,.wordpress-org,build,docs,dist,node_modules,vendor,wordpress\" --headers='{\"Last-Translator\":\"plugins@godaddy.com\",\"Report-Msgid-Bugs-To\":\"https://github.com/godaddy-wordpress/coblocks/issues\"}' --file-comment=\"Copyright (c) $(date +'%Y') GoDaddy Operating Company, LLC. All Rights Reserved.\" languages/coblocks.pot && yarn run pot2json", "po2jed": "cd languages && find . -name '*.po' -execdir /bin/bash -c 'FROM=\"$0\" && TO=\"`basename $0 .po`-coblocks-editor.json\" && echo \"$FROM > $TO\" && po2json $FROM $TO -f jed' '{}' \\;", "po2mo": "cd languages && find . -name '*.po' -execdir /bin/bash -c 'FROM=\"$0\" && TO=\"`basename $0 .po`.mo\" && echo \"$FROM > $TO\" && msgfmt $FROM -o $TO' '{}' \\;", diff --git a/src/blocks/gallery-masonry/block.json b/src/blocks/gallery-masonry/block.json index be040aab0d0..53f5027978b 100644 --- a/src/blocks/gallery-masonry/block.json +++ b/src/blocks/gallery-masonry/block.json @@ -111,6 +111,7 @@ "imageCrop": "imageCrop" }, "editorScript": ["coblocks-5"], + "viewScript": ["coblocks-lightbox"], "supports": { "anchor": true, "gutter": { diff --git a/src/extensions/lightbox-controls/test/lightbox-controls-gallery.cypress.js b/src/extensions/lightbox-controls/test/lightbox-controls-gallery.cypress.js index a865d35a160..16c54367be2 100644 --- a/src/extensions/lightbox-controls/test/lightbox-controls-gallery.cypress.js +++ b/src/extensions/lightbox-controls/test/lightbox-controls-gallery.cypress.js @@ -54,9 +54,7 @@ describe( 'Test CoBlocks Lightbox Controls extension on core/gallery', function( cy.contains( 'Lightbox' ); - cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).click(); - - cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).contains( new RegExp( alignment, 'i' ) ).click(); + helpers.setBlockAlignment( alignment ); helpers.selectBlock( 'Gallery' ); diff --git a/src/extensions/lightbox-controls/test/lightbox-controls-image.cypress.js b/src/extensions/lightbox-controls/test/lightbox-controls-image.cypress.js index 6cbed6f89c5..1a5823faaa0 100644 --- a/src/extensions/lightbox-controls/test/lightbox-controls-image.cypress.js +++ b/src/extensions/lightbox-controls/test/lightbox-controls-image.cypress.js @@ -50,9 +50,7 @@ describe( 'Test CoBlocks Lightbox Controls extension on core/image', function() helpers.upload.imageToBlock( 'core/image' ); - cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).click(); - - cy.get( '[aria-label="Change alignment"], [aria-label="Align"]' ).contains( new RegExp( alignment, 'i' ) ).click(); + helpers.setBlockAlignment( alignment ); helpers.toggleSettingCheckbox( /Lightbox/ ); diff --git a/src/extensions/lightbox-controls/test/lightbox-controls-masonry.cypress.js b/src/extensions/lightbox-controls/test/lightbox-controls-masonry.cypress.js new file mode 100644 index 00000000000..b734af25ac0 --- /dev/null +++ b/src/extensions/lightbox-controls/test/lightbox-controls-masonry.cypress.js @@ -0,0 +1,72 @@ +/* + * Include our constants + */ +import * as helpers from '../../../../.dev/tests/cypress/helpers'; + +describe( 'Test CoBlocks Lightbox Controls extension on core/masonry', function() { + /** + * Test that we can add a image block to the content add image, + * and alter image using the Lightbox Controls extension + */ + it( 'Test coblocks/gallery-masonry block extends with Lightbox Controls component.', function() { + const { imageBase } = helpers.upload.spec; + helpers.addBlockToPost( 'coblocks/gallery-masonry', true ); + + helpers.upload.imageToBlock( 'coblocks/gallery-masonry' ); + + cy.get( '.has-lightbox' ).should( 'not.exist' ); + + helpers.savePage(); + + helpers.selectBlock( 'coblocks/gallery-masonry' ); + + helpers.toggleSettingCheckbox( /Lightbox/ ); + + cy.get( '.has-lightbox' ).should( 'exist' ); + + cy.get( 'figure.wp-block-image img[src*="http"]' ).should( 'have.attr', 'src' ).should( 'include', imageBase ); + + helpers.savePage(); + + helpers.checkForBlockErrors( 'core/image' ); + + helpers.viewPage(); + + cy.get( 'figure.wp-block-image img[src*="http"]' ).should( 'have.attr', 'src' ).should( 'include', imageBase ); + + cy.get( '.has-lightbox' ).should( 'exist' ); + + helpers.editPage(); + } ); + + /** + * Test that we can add a image block to the content add image, + * and open Lightbox on Front-end when aligned + */ + [ 'wide', 'full' ].forEach( ( alignment ) => { + return it( `Test ${ alignment } alignment coblocks/gallery-masonry block with Lightbox Controls component.`, function() { + const { imageBase } = helpers.upload.spec; + helpers.addBlockToPost( 'coblocks/gallery-masonry', true ); + + helpers.upload.imageToBlock( 'coblocks/gallery-masonry' ); + + helpers.setBlockAlignment( alignment ); + + helpers.toggleSettingCheckbox( /Lightbox/ ); + + helpers.savePage(); + + helpers.viewPage(); + + cy.get( `figure[class*="align${ alignment }"] img[src*="http"][class^="wp-image-"]` ).should( 'have.attr', 'src' ).should( 'include', imageBase ); + + cy.get( '.coblocks-lightbox' ).should( 'be.hidden' ); + + cy.get( `figure[class*="align${ alignment }"] img[src*="http"][class^="wp-image-"]` ).click( { force: true } ); + + cy.get( '.coblocks-lightbox' ).should( 'be.visible' ); + + helpers.editPage(); + } ); + } ); +} );