Skip to content

Commit

Permalink
Fix: Align Hook: Impossible to set no alignment when a default align …
Browse files Browse the repository at this point in the history
…exists (#9634)

When we have a default value for an attribute, during the load of a document/block if that attribute is undefined the attribute gets the default value.
If we had a block that supports align but contained a default align we had a problem. If no align was set the post correctly saved the markup of no alignment set but the alignment attribute was not saved watching by the value of the attribute it was impossible to differentiate the state of default alignment being used, and the state of no alignment is used. The alignment attribute was just not stored in both cases.
If alignment was set to none (undefined), When the block was loaded no attribute was set, so the default value of alignment was used, but the block had the markup of alignment none, so the block becomes invalid.

This PR solves this problem by setting alignment to none when setting no alignment on the align hook, this way none is saved on the attribute and Gutenberg correctly differentiates the state of the block has the default align and the state the block has no alignment.


## How has this been tested?
I used the following test block: https://gist.github.com/jorgefilipecosta/b733f1745531dc9758c6eba0838ae3f5; I checked on the code editor that if setting no alignment for this block we set alignment comment attribute to 'none' and no align class is added. And I checked that after removing the alignment saving the post and loading it again everything works as expected. 

## Screenshots <!-- if applicable -->
Before:
![sep-05-2018 16-28-13](https://user-images.githubusercontent.com/11271197/45104639-708f9180-b12a-11e8-9ad7-bdbe34caa05e.gif)


After:
![sep-05-2018 15-46-36](https://user-images.githubusercontent.com/11271197/45104626-68cfed00-b12a-11e8-9a4d-854aa03d596e.gif)
  • Loading branch information
jorgefilipecosta authored Oct 8, 2018
1 parent 69ced17 commit 9d419fa
Show file tree
Hide file tree
Showing 7 changed files with 394 additions and 4 deletions.
13 changes: 13 additions & 0 deletions docs/block-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,19 @@ align: true,
// Pick which alignment options to display.
align: [ 'left', 'right', 'full' ],
```
When supports align is used the block attributes definition is extended to include an align attribute with a string type.
By default, no alignment is assigned to the block.
The block can apply a default alignment by specifying its own align attribute with a default e.g.:
```
attributes: {
...
align: {
type: 'string',
default: 'right'
},
...
}
```

- `alignWide` (default `true`): This property allows to enable [wide alignment](../docs/extensibility/theme-support.md#wide-alignment) for your theme. To disable this behavior for a single block, set this flag to `false`.

Expand Down
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 = '';
}
}
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\\":\\"\\"} -->
<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 -->"
`;
196 changes: 196 additions & 0 deletions test/e2e/specs/align-hook.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
/**
* Internal dependencies
*/
import {
newPost,
insertBlock,
getEditedPostContent,
setPostContent,
getAllBlocks,
selectBlockByClientId,
} 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(
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 setPostContent( htmlMarkup );
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":""' );
} );

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

0 comments on commit 9d419fa

Please sign in to comment.