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.

Sign up for GitHub

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

Remove reset button in behaviors, use dropdown option instead #51464

Closed
wants to merge 3 commits into from
Closed

Remove reset button in behaviors, use dropdown option instead #51464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
240de50
Remove reset button in behaviors, use dropdown option instead
cbravobernal Jun 13, 2023
ce79f31
Use default option reading from the theme, prevent autoupdate
cbravobernal Jun 14, 2023
286541f
The default option changes how we have to do the e2e
cbravobernal Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 45 additions & 47 deletions packages/block-editor/src/hooks/behaviors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@
* WordPress dependencies
*/
import { addFilter } from '@wordpress/hooks';
import {
SelectControl,
Button,
__experimentalHStack as HStack,
} from '@wordpress/components';
import { SelectControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { hasBlockSupport } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
Expand All @@ -18,11 +14,6 @@ import { useSelect } from '@wordpress/data';
import { store as blockEditorStore } from '../store';
import { InspectorControls } from '../components';

/**
* External dependencies
*/
import merge from 'deepmerge';

function BehaviorsControl( {
blockName,
blockBehaviors,
Expand All @@ -43,62 +34,69 @@ function BehaviorsControl( {
[ blockName ]
);

const noBehaviorsOption = {
value: '',
label: __( 'No behaviors' ),
const defaultBehaviors = {
default: {
value: 'default',
label: __( 'Default' ),
},
noBehaviors: {
value: '',
label: __( 'No behaviors' ),
},
};

const behaviorsOptions = Object.entries( settings )
.filter(
( [ behaviorName, behaviorValue ] ) =>
hasBlockSupport( blockName, 'behaviors.' + behaviorName ) &&
hasBlockSupport( blockName, `behaviors.${ behaviorName }` ) &&
behaviorValue
) // Filter out behaviors that are disabled.
.map( ( [ behaviorName ] ) => ( {
value: behaviorName,
label:
// Capitalize the first letter of the behavior name.
behaviorName[ 0 ].toUpperCase() +
behaviorName.slice( 1 ).toLowerCase(),
// Capitalize the first letter of the behavior name.
label: `${ behaviorName.charAt( 0 ).toUpperCase() }${ behaviorName
.slice( 1 )
.toLowerCase() }`,
} ) );

// If every behavior is disabled, do not show the behaviors inspector control.
if ( behaviorsOptions.length === 0 ) return null;

const options = [ noBehaviorsOption, ...behaviorsOptions ];
const options = [
...Object.values( defaultBehaviors ),
...behaviorsOptions,
];

// If every behavior is disabled, do not show the behaviors inspector control.
if ( behaviorsOptions.length === 0 ) {
return null;
}
// Block behaviors take precedence over theme behaviors.
const behaviors = merge( themeBehaviors, blockBehaviors || {} );
const behaviors = { ...themeBehaviors, ...( blockBehaviors || {} ) };

const helpText = disabled
? __( 'The lightbox behavior is disabled for linked images.' )
: __( 'Add behaviors.' );

const value = () => {
if ( blockBehaviors === undefined ) {
return 'default';
}
if ( behaviors?.lightbox ) {
return 'lightbox';
}
return '';
};

return (
<InspectorControls group="advanced">
{ /* This div is needed to prevent a margin bottom between the dropdown and the button. */ }
<div>
<SelectControl
label={ __( 'Behaviors' ) }
// At the moment we are only supporting one behavior (Lightbox)
value={ behaviors?.lightbox ? 'lightbox' : '' }
options={ options }
onChange={ onChange }
hideCancelButton={ true }
help={ helpText }
size="__unstable-large"
disabled={ disabled }
/>
</div>
<HStack justify="flex-end">
<Button
isSmall
disabled={ disabled }
onClick={ () => onChange( undefined ) }
>
{ __( 'Reset' ) }
</Button>
</HStack>
<SelectControl
label={ __( 'Behaviors' ) }
value={ value() }
options={ options }
onChange={ onChange }
hideCancelButton={ true }
help={ helpText }
size="__unstable-large"
disabled={ disabled }
/>
</InspectorControls>
);
}
Expand Down Expand Up @@ -130,7 +128,7 @@ export const withBehaviors = createHigherOrderComponent( ( BlockEdit ) => {
blockName={ props.name }
blockBehaviors={ props.attributes.behaviors }
onChange={ ( nextValue ) => {
if ( nextValue === undefined ) {
if ( nextValue === 'default' ) {
props.setAttributes( {
behaviors: undefined,
} );
Expand Down
95 changes: 6 additions & 89 deletions test/e2e/specs/editor/various/behaviors.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,43 +49,6 @@ test.describe( 'Testing behaviors functionality', () => {
await page.waitForLoadState();
} );

test( '`No Behaviors` should be the default as defined in the core theme.json', async ( {
admin,
editor,
requestUtils,
page,
behaviorUtils,
} ) => {
await requestUtils.activateTheme( 'twentytwentyone' );
await admin.createNewPost();
const media = await behaviorUtils.createMedia();
await editor.insertBlock( {
name: 'core/image',
attributes: {
alt: filename,
id: media.id,
url: media.source_url,
},
} );

await editor.openDocumentSettingsSidebar();
const editorSettings = page.getByRole( 'region', {
name: 'Editor settings',
} );
await editorSettings
.getByRole( 'button', { name: 'Advanced' } )
.click();
const select = editorSettings.getByRole( 'combobox', {
name: 'Behavior',
} );

// By default, no behaviors should be selected.
await expect( select ).toHaveValue( '' );

// By default, you should be able to select the Lightbox behavior.
await expect( select.getByRole( 'option' ) ).toHaveCount( 2 );
} );

test( 'Behaviors UI can be disabled in the `theme.json`', async ( {
admin,
editor,
Expand Down Expand Up @@ -162,8 +125,8 @@ test.describe( 'Testing behaviors functionality', () => {
// attributes takes precedence over the theme's value.
await expect( select ).toHaveValue( 'lightbox' );

// There should be 2 options available: `No behaviors` and `Lightbox`.
await expect( select.getByRole( 'option' ) ).toHaveCount( 2 );
// There should be 3 options available: `No behaviors` and `Lightbox`.
await expect( select.getByRole( 'option' ) ).toHaveCount( 3 );

// We can change the value of the behaviors dropdown to `No behaviors`.
await select.selectOption( { label: 'No behaviors' } );
Expand All @@ -173,50 +136,6 @@ test.describe( 'Testing behaviors functionality', () => {
// lightbox even though the theme.json has it set to false.
} );

test( 'You can set the default value for the behaviors in the theme.json', async ( {
admin,
editor,
requestUtils,
page,
behaviorUtils,
} ) => {
// In this theme, the default value for settings.behaviors.blocks.core/image.lightbox is `true`.
await requestUtils.activateTheme( 'behaviors-enabled' );
await admin.createNewPost();
const media = await behaviorUtils.createMedia();

await editor.insertBlock( {
name: 'core/image',
attributes: {
alt: filename,
id: media.id,
url: media.source_url,
},
} );

await editor.openDocumentSettingsSidebar();
const editorSettings = page.getByRole( 'region', {
name: 'Editor settings',
} );
await editorSettings
.getByRole( 'button', { name: 'Advanced' } )
.click();
const select = editorSettings.getByRole( 'combobox', {
name: 'Behavior',
} );

// The behaviors dropdown should be present and the value should be set to
// `lightbox`.
await expect( select ).toHaveValue( 'lightbox' );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test now makes no sense, as the behavior will be always 'Default' if we don't interact with the selector.


// There should be 2 options available: `No behaviors` and `Lightbox`.
await expect( select.getByRole( 'option' ) ).toHaveCount( 2 );

// We can change the value of the behaviors dropdown to `No behaviors`.
await select.selectOption( { label: 'No behaviors' } );
await expect( select ).toHaveValue( '' );
} );

test( 'Lightbox behavior is disabled if the Image has a link', async ( {
admin,
editor,
Expand Down Expand Up @@ -254,7 +173,7 @@ test.describe( 'Testing behaviors functionality', () => {
await expect( select ).toBeDisabled();
} );

test( 'Lightbox behavior control has a Reset button that removes the markup', async ( {
test( 'Lightbox behavior control has a default option that removes the markup', async ( {
admin,
editor,
requestUtils,
Expand Down Expand Up @@ -293,13 +212,11 @@ test.describe( 'Testing behaviors functionality', () => {
.last()
.click();

const resetButton = editorSettings.getByRole( 'button', {
name: 'Reset',
const select = editorSettings.getByRole( 'combobox', {
name: 'Behavior',
} );

expect( resetButton ).toBeDefined();

await resetButton.last().click();
await select.selectOption( { label: 'Default' } );
expect( await editor.getEditedPostContent() )
.toBe( `<!-- wp:image {"id":${ media.id }} -->
<figure class="wp-block-image"><img src="http://localhost:8889/wp-content/uploads/${ year }/${ month }/1024x768_e2e_test_image_size.jpeg" alt="1024x768_e2e_test_image_size.jpeg" class="wp-image-${ media.id }"/></figure>
Expand Down