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

Image block: Revise lightbox UI to remove 'behaviors' #53851

Merged
merged 20 commits into from
Sep 15, 2023
Merged

Image block: Revise lightbox UI to remove 'behaviors' #53851

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
08bb1ed
Begin removing theme.json dependency in block UI
artemiomorales Aug 21, 2023
c93f1b6
Remove the useHasBehaviorsPanel hook
michalczaplinski Aug 21, 2023
25aa1c6
Fix declaration to actually retrieve from user data
artemiomorales Aug 21, 2023
a8c09c9
Restructured use of global behaviors
michalczaplinski Aug 21, 2023
fac2e3a
More removal of behaviors.
michalczaplinski Aug 28, 2023
dfc1c46
Remove the reference to behaviors in Global styles and first iteratio…
michalczaplinski Aug 28, 2023
51e4802
Remove behaviors altogether and everywhere
michalczaplinski Aug 29, 2023
b416558
Fix linter & sniffer PHP errors
michalczaplinski Aug 30, 2023
321a213
Adjust schema properties count assertion
michalczaplinski Aug 30, 2023
0021116
Add the lightbox attribute
michalczaplinski Aug 30, 2023
6951a86
Remove unnecessary space
michalczaplinski Aug 30, 2023
92b084f
Merge branch 'trunk' into update/revise-lightbox-ui
michalczaplinski Sep 6, 2023
232161a
Add clarifying comment regarding skipped tests
artemiomorales Sep 7, 2023
cabe31b
Do not remove `behaviors` attribute from Image block's block.json.
michalczaplinski Sep 7, 2023
aa4450e
Revert "Do not remove `behaviors` attribute from Image block's block.…
michalczaplinski Sep 7, 2023
91b86ef
Merge branch 'trunk' into update/revise-lightbox-ui
michalczaplinski Sep 11, 2023
f9e48f4
Revert deletion of behaviors.php
artemiomorales Sep 11, 2023
8b95f4a
Merge branch 'trunk' into update/revise-lightbox-ui
michalczaplinski Sep 14, 2023
1b3a480
Image block: UI updates for the image lightbox (#54071)
artemiomorales Sep 15, 2023
ef8c396
Revert "Image block: UI updates for the image lightbox (#54071)"
artemiomorales Sep 15, 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
7 changes: 4 additions & 3 deletions lib/block-supports/behaviors.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ function gutenberg_render_behaviors_support_lightbox( $block_content, $block ) {
$lightbox_settings = $block['attrs']['behaviors']['lightbox'];
// If the lightbox setting is not set in the block attributes, get it from the theme.json file.
} else {
$theme_data = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()->get_data();
if ( isset( $theme_data['behaviors']['blocks'][ $block['blockName'] ]['lightbox'] ) ) {
$lightbox_settings = $theme_data['behaviors']['blocks'][ $block['blockName'] ]['lightbox'];
$user_data = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()->get_data();
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved

if ( isset( $user_data['behaviors']['blocks'][ $block['blockName'] ]['lightbox'] ) ) {
$lightbox_settings = $user_data['behaviors']['blocks'][ $block['blockName'] ]['lightbox'];
} else {
$lightbox_settings = null;
}
Expand Down
15 changes: 0 additions & 15 deletions lib/theme.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
{
"version": 2,
"behaviors": {
"blocks": {
"core/image": {
"lightbox": {
"enabled": false,
"animation": ""
}
}
}
},
Comment on lines -3 to -12
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just be removed or does it need a deprecation of some sort? Behaviors were shipped in GB 16.4 so never included in a WP release.

cc @ellatrix @gziolo @youknowriad

Copy link
Member

Choose a reason for hiding this comment

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

@artemiomorales and I looked at whether this is used in production, and it looks like you can change that setting in the UI, which most likely gets reflected in what gets saved in the database. While it isn't mandatory for WordPress core, it would still be nice to provide a migration path for sites that have the latest Gutenberg plugin enabled. It shouldn't be that difficult to code something in the plugin to read behaviors object from the site and transfer the value to the new structure.

I guess the same applies to the block attribute that changes from behaviors.lightbox to lightbox in the Image block.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not fully caught up on what's next for behaviors, but it seems that themes will no longer be able to tweak the lightbox feature (enable/disable, set animation) after this removal.

If we still plan to allow them to tweak it, just in a different place, we could add some code around this line to make the migration easier for them. I'll be AFK in the next few days, but wanted to share in case it's useful.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the followup PR: #54071 we've added backwards-compatibility for users that might have been using the previous behaviors.lightbox syntax.

In that PR (which is based on the current one) we're also now introducing a lightbox.allowEditing setting which allows themes to enable/disable the lightbox.

"settings": {
"appearanceTools": false,
"useRootPaddingAwareAlignments": false,
Expand Down Expand Up @@ -290,11 +280,6 @@
"style": true,
"width": true
}
},
"core/image": {
"behaviors": {
"lightbox": true
}
}
}
},
Expand Down
26 changes: 1 addition & 25 deletions packages/block-editor/src/components/global-styles/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import fastDeepEqual from 'fast-deep-equal/es6';
*/
import { useContext, useCallback, useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { store as blocksStore, hasBlockSupport } from '@wordpress/blocks';
import { store as blocksStore } from '@wordpress/blocks';
import { _x } from '@wordpress/i18n';

/**
Expand Down Expand Up @@ -546,27 +546,3 @@ export function __experimentalUseGlobalBehaviors( blockName, source = 'all' ) {

return { behavior, inheritedBehaviors: result, setBehavior };
}

export function __experimentalUseHasBehaviorsPanel(
settings,
name,
{ blockSupportOnly = false } = {}
) {
if ( ! settings?.behaviors ) {
return false;
}

// If every behavior is disabled on block supports, do not show the behaviors inspector control.
const hasSomeBlockSupport = Object.keys( settings?.behaviors ).some(
( key ) => hasBlockSupport( name, `behaviors.${ key }` )
);

if ( blockSupportOnly ) {
return hasSomeBlockSupport;
}

// If every behavior is disabled, do not show the behaviors inspector control.
return Object.values( settings?.behaviors ).some(
( value ) => value === true && hasSomeBlockSupport
);
}
michalczaplinski marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion packages/block-editor/src/components/global-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export {
__experimentalUseGlobalBehaviors,
__experimentalUseHasBehaviorsPanel,
useGlobalStylesReset,
useGlobalSetting,
useGlobalStyle,
Expand Down
39 changes: 2 additions & 37 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 @@ -6,34 +6,19 @@ import { SelectControl } from '@wordpress/components';
import { __ } from '@wordpress/i18n';
import { hasBlockSupport } from '@wordpress/blocks';
import { createHigherOrderComponent } from '@wordpress/compose';
import { useSelect } from '@wordpress/data';
import { useMemo } from '@wordpress/element';

/**
* Internal dependencies
*/
import { store as blockEditorStore } from '../store';
import { InspectorControls } from '../components';

function BehaviorsControl( {
blockName,
blockBehaviors,
onChangeBehavior,
onChangeAnimation,
disabled = false,
} ) {
const { settings } = useSelect(
( select ) => {
const { getSettings } = select( blockEditorStore );
return {
settings:
getSettings()?.__experimentalFeatures?.blocks?.[ blockName ]
?.behaviors || {},
};
},
[ blockName ]
);

const defaultBehaviors = {
default: {
value: 'default',
Expand All @@ -44,23 +29,8 @@ function BehaviorsControl( {
label: __( 'No behaviors' ),
},
};
const behaviorsOptions = Object.entries( settings )
.filter(
( [ behaviorName, behaviorValue ] ) =>
hasBlockSupport( blockName, `behaviors.${ behaviorName }` ) &&
behaviorValue
) // Filter out behaviors that are disabled.
.map( ( [ behaviorName ] ) => ( {
value: behaviorName,
// Capitalize the first letter of the behavior name.
label: `${ behaviorName.charAt( 0 ).toUpperCase() }${ behaviorName
.slice( 1 )
.toLowerCase() }`,
} ) );
const options = [
...Object.values( defaultBehaviors ),
...behaviorsOptions,
];

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

const { behaviors, behaviorsValue } = useMemo( () => {
const mergedBehaviors = {
Expand All @@ -80,11 +50,6 @@ function BehaviorsControl( {
};
}, [ blockBehaviors ] );

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

const helpText = disabled
? __( 'The lightbox behavior is disabled for linked images.' )
: '';
Expand Down
16 changes: 6 additions & 10 deletions packages/edit-site/src/components/global-styles/screen-block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ const {
useHasDimensionsPanel,
useHasTypographyPanel,
useHasBorderPanel,
__experimentalUseHasBehaviorsPanel: useHasBehaviorsPanel,
useGlobalSetting,
useSettingsForBlockElement,
useHasColorPanel,
Expand Down Expand Up @@ -101,7 +100,6 @@ function ScreenBlock( { name, variation } ) {
const blockVariations = useBlockVariations( name );
const hasTypographyPanel = useHasTypographyPanel( settings );
const hasColorPanel = useHasColorPanel( settings );
const hasBehaviorsPanel = useHasBehaviorsPanel( rawSettings, name );
const hasBorderPanel = useHasBorderPanel( settings );
const hasDimensionsPanel = useHasDimensionsPanel( settings );
const hasEffectsPanel = useHasEffectsPanel( settings );
Expand Down Expand Up @@ -274,14 +272,12 @@ function ScreenBlock( { name, variation } ) {
onChange={ setStyle }
inheritedValue={ inheritedStyle }
/>
{ hasBehaviorsPanel && (
<StylesBehaviorsPanel
value={ behavior }
onChange={ setBehavior }
behaviors={ inheritedBehaviors }
blockName={ name }
></StylesBehaviorsPanel>
) }
<StylesBehaviorsPanel
value={ behavior }
onChange={ setBehavior }
behaviors={ inheritedBehaviors }
blockName={ name }
></StylesBehaviorsPanel>
</PanelBody>
) }
</>
Expand Down
8 changes: 1 addition & 7 deletions packages/edit-site/src/hooks/push-changes-to-global-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const {
cleanEmptyObject,
GlobalStylesContext,
__experimentalUseGlobalBehaviors: useGlobalBehaviors,
__experimentalUseHasBehaviorsPanel: useHasBehaviorsPanel,
} = unlock( blockEditorPrivateApis );

// Block Gap is a special case and isn't defined within the blocks
Expand Down Expand Up @@ -286,10 +285,6 @@ function PushChangesToGlobalStylesControl( {
attributes,
setAttributes,
} ) {
const hasBehaviorsPanel = useHasBehaviorsPanel( attributes, name, {
blockSupportOnly: true,
} );

const { user: userConfig, setUserConfig } =
useContext( GlobalStylesContext );

Expand All @@ -301,8 +296,7 @@ function PushChangesToGlobalStylesControl( {

const { inheritedBehaviors, setBehavior } = useGlobalBehaviors( name );

const userHasEditedBehaviors =
attributes.hasOwnProperty( 'behaviors' ) && hasBehaviorsPanel;
const userHasEditedBehaviors = attributes.hasOwnProperty( 'behaviors' );

const pushChanges = useCallback( () => {
if ( changes.length === 0 && ! userHasEditedBehaviors ) {
Expand Down
Loading