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

Fix: Align Hook: Impossible to set no alignment when a default align exists #9634

Merged
merged 9 commits into from
Oct 8, 2018
17 changes: 13 additions & 4 deletions packages/editor/src/hooks/align.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@
* External dependencies
*/
import classnames from 'classnames';
import { assign, get, includes } from 'lodash';
import { assign, get, has, includes } from 'lodash';

/**
* WordPress dependencies
*/
import { createHigherOrderComponent } from '@wordpress/compose';
import { addFilter } from '@wordpress/hooks';
import { hasBlockSupport, getBlockSupport } from '@wordpress/blocks';
import { hasBlockSupport, getBlockSupport, getBlockType } from '@wordpress/blocks';

/**
* Internal dependencies
Expand All @@ -24,7 +24,7 @@ import { BlockControls, BlockAlignmentToolbar } from '../components';
*/
export function addAttribute( settings ) {
// allow blocks to specify their own attribute definition with default values if needed.
if ( get( settings.attributes, [ 'align', 'type' ] ) === 'string' ) {
if ( has( settings.attributes, [ 'align', 'type' ] ) ) {
return settings;
}
if ( hasBlockSupport( settings, 'align' ) ) {
Expand Down Expand Up @@ -77,7 +77,16 @@ export const withToolbarControls = createHigherOrderComponent( ( BlockEdit ) =>
return ( props ) => {
const validAlignments = getBlockValidAlignments( props.name );

const updateAlignment = ( nextAlign ) => props.setAttributes( { align: nextAlign } );
const updateAlignment = ( nextAlign ) => {
if ( ! nextAlign ) {
const blockType = getBlockType( props.name );
const blockDefaultAlign = get( blockType, [ 'attributes', 'align', 'default' ] );
if ( blockDefaultAlign ) {
nextAlign = null;
}
}
props.setAttributes( { align: nextAlign } );
};

return [
validAlignments.length > 0 && props.isSelected && (
Expand Down
43 changes: 43 additions & 0 deletions test/e2e/specs/__snapshots__/align-hook.test.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Align Hook Works As Expected Block with align array Correctly applies the selected alignment and correctly removes the alignment 1`] = `
"<!-- wp:test/test-align-array {\\"align\\":\\"center\\"} -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-align-array aligncenter\\">Test Align Hook</div>
<!-- /wp:test/test-align-array -->"
`;

exports[`Align Hook Works As Expected Block with align array Correctly applies the selected alignment and correctly removes the alignment 2`] = `
"<!-- wp:test/test-align-array -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-align-array\\">Test Align Hook</div>
<!-- /wp:test/test-align-array -->"
`;

exports[`Align Hook Works As Expected Block with align true Correctly applies the selected alignment and correctly removes the alignment 1`] = `
"<!-- wp:test/test-align-true {\\"align\\":\\"right\\"} -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-align-true alignright\\">Test Align Hook</div>
<!-- /wp:test/test-align-true -->"
`;

exports[`Align Hook Works As Expected Block with align true Correctly applies the selected alignment and correctly removes the alignment 2`] = `
"<!-- wp:test/test-align-true -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-align-true\\">Test Align Hook</div>
<!-- /wp:test/test-align-true -->"
`;

exports[`Align Hook Works As Expected Block with default align Correctly applies the selected alignment and correctly removes the alignment 1`] = `
"<!-- wp:test/test-default-align {\\"align\\":\\"center\\"} -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-default-align aligncenter\\">Test Align Hook</div>
<!-- /wp:test/test-default-align -->"
`;

exports[`Align Hook Works As Expected Block with default align Correctly applies the selected alignment and correctly removes the alignment 2`] = `
"<!-- wp:test/test-default-align {\\"align\\":null} -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-default-align\\">Test Align Hook</div>
<!-- /wp:test/test-default-align -->"
`;

exports[`Align Hook Works As Expected Block with no alignment set Does not save any alignment related attribute or class 1`] = `
"<!-- wp:test/test-no-alignment-set -->
<div style=\\"outline:1px solid gray;padding:5px\\" class=\\"wp-block-test-test-no-alignment-set\\">Test Align Hook</div>
<!-- /wp:test/test-no-alignment-set -->"
`;
200 changes: 200 additions & 0 deletions test/e2e/specs/align-hook.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/**
* Internal dependencies
*/
import {
newPost,
insertBlock,
getEditedPostContent,
setPostContent,
getAllBlocks,
selectBlockByClientId,
switchToEditor,
} from '../support/utils';
import { activatePlugin, deactivatePlugin } from '../support/plugins';

describe( 'Align Hook Works As Expected', () => {
beforeAll( async () => {
await activatePlugin( 'gutenberg-test-align-hook' );
} );

beforeEach( async () => {
await newPost();
} );

afterAll( async () => {
await deactivatePlugin( 'gutenberg-test-align-hook' );
} );

const getAlignmentToolbarLabels = async () => {
const buttonLabels = await page.evaluate( () => {
return Array.from(
Copy link
Member

Choose a reason for hiding this comment

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

This is cool in combination with map 👍

document.querySelectorAll(
'.editor-block-toolbar button[aria-label^="Align"]'
)
).map(
( button ) => {
return button.getAttribute( 'aria-label' );
}
);
} );
return buttonLabels;
};

const createShowsTheExpectedButtonsTest = ( blockName, buttonLabels ) => {
it( 'Shows the expected buttons on the alignment toolbar',
async () => {
await insertBlock( blockName );
expect(
await getAlignmentToolbarLabels()
).toEqual( buttonLabels );
} );
};

const createDoesNotApplyAlignmentByDefaultTest = ( blockName ) => {
it( 'Does not apply any alignment by default', async () => {
await insertBlock( blockName );
// verify no alignment button is in pressed state
const pressedButtons = await page.$$( '.editor-block-toolbar button[aria-label^="Align"][aria-pressed="true"]' );
expect( pressedButtons ).toHaveLength( 0 );
} );
};

const verifyMarkupIsValid = async ( htmlMarkup ) => {
await switchToEditor( 'Code' );
Copy link
Member

Choose a reason for hiding this comment

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

Given that you set post content using data layer, do you still need to switch to Code Editor?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea of this switch was to make sure everything is unmounted and mounted again.

Copy link
Member Author

Choose a reason for hiding this comment

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

But it was removed as it makes our tests faster and it does not seem required for this case.

await setPostContent( htmlMarkup );
await switchToEditor( 'Visual' );
const blocks = await getAllBlocks();
expect( blocks ).toHaveLength( 1 );
expect( blocks[ 0 ].isValid ).toBeTruthy();
};

const createCorrectlyAppliesAndRemovesAlignmentTest = ( blockName, alignment ) => {
it( 'Correctly applies the selected alignment and correctly removes the alignment',
async () => {
const BUTTON_SELECTOR = `.editor-block-toolbar button[aria-label="Align ${ alignment }"]`;
const BUTTON_PRESSED_SELECTOR = `${ BUTTON_SELECTOR }[aria-pressed="true"]`;
// set the specified alignment.
await insertBlock( blockName );
await page.click( BUTTON_SELECTOR );

// verify the button of the specified alignment is pressed.
let pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
expect( pressedButtons ).toHaveLength( 1 );

let htmlMarkup = await getEditedPostContent();

// verify the markup of the selected alignment was generated.
expect( htmlMarkup ).toMatchSnapshot();

// verify the markup can be correctly parsed
await verifyMarkupIsValid( htmlMarkup );

await selectBlockByClientId(
( await getAllBlocks() )[ 0 ].clientId
);

// remove the alignment.
await page.click( BUTTON_SELECTOR );

// verify no alignment button is in pressed state.
pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
expect( pressedButtons ).toHaveLength( 0 );

// verify alignment markup was removed from the block.
htmlMarkup = await getEditedPostContent();
expect( htmlMarkup ).toMatchSnapshot();

// verify the markup when no alignment is set is valid
await verifyMarkupIsValid( htmlMarkup );

await selectBlockByClientId(
( await getAllBlocks() )[ 0 ].clientId
);

// verify no alignment button is in pressed state after parsing the block.
pressedButtons = await page.$$( BUTTON_PRESSED_SELECTOR );
expect( pressedButtons ).toHaveLength( 0 );
}
);
};

describe( 'Block with no alignment set', () => {
const BLOCK_NAME = 'Test No Alignment Set';
it( 'Shows no alignment buttons on the alignment toolbar',
async () => {
expect( await getAlignmentToolbarLabels() ).toHaveLength( 0 );
}
);

it( 'Does not save any alignment related attribute or class',
async () => {
await insertBlock( BLOCK_NAME );
expect( await getEditedPostContent() ).toMatchSnapshot();
}
);
} );

describe( 'Block with align true', () => {
const BLOCK_NAME = 'Test Align True';

createShowsTheExpectedButtonsTest( BLOCK_NAME, [
'Align left',
'Align center',
'Align right',
] );

createDoesNotApplyAlignmentByDefaultTest( BLOCK_NAME );

createCorrectlyAppliesAndRemovesAlignmentTest( BLOCK_NAME, 'right' );
} );

describe( 'Block with align array', () => {
const BLOCK_NAME = 'Test Align Array';

createShowsTheExpectedButtonsTest( BLOCK_NAME, [
'Align left',
'Align center',
] );

createDoesNotApplyAlignmentByDefaultTest( BLOCK_NAME );

createCorrectlyAppliesAndRemovesAlignmentTest( BLOCK_NAME, 'center' );
} );

describe( 'Block with default align', () => {
const BLOCK_NAME = 'Test Default Align';
const PRESSED_BUTTON_SELECTOR = '.editor-block-toolbar button[aria-label="Align right"][aria-pressed="true"]';
createShowsTheExpectedButtonsTest( BLOCK_NAME, [
'Align left',
'Align center',
'Align right',
] );

it( 'Applies the selected alignment by default', async () => {
await insertBlock( BLOCK_NAME );
// verify the correct alignment button is pressed
const pressedButtons = await page.$$( PRESSED_BUTTON_SELECTOR );
expect( pressedButtons ).toHaveLength( 1 );
} );

it( 'The default markup does not contain the alignment attribute but contains the alignment class',
async () => {
await insertBlock( BLOCK_NAME );
const markup = await getEditedPostContent();
expect( markup ).not.toContain( '"align":"right"' );
expect( markup ).toContain( 'alignright' );
}
);

it( 'Can remove the default alignment and the align attribute equals none but alignnone class is not applied', async () => {
await insertBlock( BLOCK_NAME );
// remove the alignment.
await page.click( PRESSED_BUTTON_SELECTOR );
const markup = await getEditedPostContent();
expect( markup ).toContain( '"align":null' );
expect( markup ).not.toContain( 'alignnull' );
} );

createCorrectlyAppliesAndRemovesAlignmentTest( BLOCK_NAME, 'center' );
} );
} );
26 changes: 26 additions & 0 deletions test/e2e/support/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,29 @@ export async function clickOnCloseModalButton( modalClassName ) {
await page.click( closeButtonClassName );
}
}

/**
* Sets code editor content
* @param {string} content New code editor content.
*
* @return {Promise} Promise resolving with an array containing all blocks in the document.
*/
export async function setPostContent( content ) {
return await page.evaluate( ( _content ) => {
const { dispatch } = window.wp.data;
const blocks = wp.blocks.parse( _content );
dispatch( 'core/editor' ).resetBlocks( blocks );
}, content );
}

/**
* Returns an array with all blocks; Equivalent to calling wp.data.select( 'core/editor' ).getBlocks();
*
* @return {Promise} Promise resolving with an array containing all blocks in the document.
*/
export async function getAllBlocks() {
return await page.evaluate( () => {
const { select } = window.wp.data;
return select( 'core/editor' ).getBlocks();
} );
}
20 changes: 20 additions & 0 deletions test/e2e/test-plugins/align-hook.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php
/**
* Plugin Name: Gutenberg Test Align Hook
* Plugin URI: https://github.com/WordPress/gutenberg
* Author: Gutenberg Team
*
* @package gutenberg-test-align-hook
*/
wp_enqueue_script(
'gutenberg-test-align-hook',
plugins_url( 'align-hook/index.js', __FILE__ ),
array(
'wp-blocks',
'wp-element',
'wp-editor',
'wp-i18n'
),
filemtime( plugin_dir_path( __FILE__ ) . 'align-hook/index.js' ),
true
);
Loading