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

Image block: UI updates for the image lightbox #54071

Merged

Conversation

artemiomorales
Copy link
Contributor

@artemiomorales artemiomorales commented Aug 30, 2023

What?

This PR updates the editor UI for the image lightbox in preparation for WP 6.4.

Why?

Address #53851

The lightbox was originally part of a proposed feature set called behaviors; however, we have since decided that the lightbox will instead exist as a just property of the image block for now, and we need to clarify the UI for end users as a result.

This PR is based off of #53851, which removes the behaviors syntax. Both of these PRs should be merged at the same time to ensure that the lightbox continues to function as expected.

How?

  • We've created a new Settings panel for the image block, where the lightbox is surfaced as a toggle in both the Global Styles and block settings. This design follows the discussion in Image block: Revise Lightbox UI to remove reference to Behaviors #53403 (comment).
  • We need to offer backwards support for folks who already enabled the lightbox using the old behaviors syntax, so we've set up an inheritance scheme across two filter functions — one to be included in WP Core as part of the image block, the other to remain as part of the Gutenberg plugin, where the legacy support will reside.
  • The panel is a ToolPanel, which means that the lightbox can be reset to default in the global styles or block settings.

How to enable

Via Global Styles

Open the global styles for the image block, then locate the Settings panel and enable the Expand on Click toggle.

lightbox-via-global-styles.mp4

Via Block Settings in Full Site Editor

Open the full site editor, add an image block, locate the Settings panel under the block's settings, and enable the Expand on Click toggle.

Lightbox.via.Block.Settings.in.Full.Site.Editing.mp4

Via Block Settings in Post Editor

Open the post editor, add an image block, locate the Settings panel under the block's settings, and enable the Expand on Click toggle.

ligthbox-via-block-settings.mp4

Via theme.json file

You can enable the lightbox in the theme.json by using either of the following syntaxes under settings:

Syntax 1:

"settings": {
	"blocks": {
		"core/image": {
			"lightbox": {
				"enabled": true
			}
		}
	}
}
lightbox-via-theme-json-block

Syntax 2

"settings": {
	"lightbox": {
		"enabled": true
	}
}
lightbox-via-theme-json-top-level

Legacy Syntax

To test backwards support for the legacy syntax, please enable the lightbox using both the theme.json and block attributes. Note that the old syntax allows for specifying the animation, either zoom or fade. If neither is specified, the lightbox defaults to zoom.

Via Theme.json

"behaviors": {
	"blocks": {
		"core/image": {
			"lightbox": {
				"enabled": true | false
				"animation": "" | "zoom" | "fade"
			}
		}
	}
}

256213365-0b8b53f9-c966-437c-8a1d-308e593f1701

256213813-d99364dd-35a9-4b60-aab6-808353d2d492

Via Block Attributes

"behaviors": {
	"lightbox": {
		"enabled": true | false
		"animation": "" | "zoom" | "fade"
	}
}
Screenshot 2023-09-05 at 10 56 04 PM

Resetting Values

You can reset the user-defined Expand on Click setting in the global styles or block settings using the Settings panel menu.

Lightbox.Reset.Settings.mp4

What to Test

  • Lightbox is applied, defined by theme.json.
  • In global styles, lightbox value inherits from theme.json, but this value can also be overridden so the global styles setting takes priority over the theme.json.
  • In the block settings, the lightbox value inherits first from the global styles if the lightbox value is present, if not, from the theme.json. In any case, the lightbox value can also be overridden so the setting at the block level takes priority over the theme.json and global styles config.
  • Lightbox user-defined settings can be reset at the global styles and block settings levels.
  • Lightbox is applied when using legacy syntax, defined by theme.json.
  • Lightbox is applied when using legacy syntax, defined by block attributes.
  • Lightbox legacy syntax is applied when the new syntax is not present.
  • When present, new syntax overrides legacy syntax.
  • Lightbox fade animation can be enabled via legacy syntax.

Testing Instructions for Keyboard

No special instructions here — the UI should be accessible as it was created using standard Gutenberg components.

@artemiomorales artemiomorales changed the title Add initial implementation of image settings panel Image block: Add settings panel to image inspector to allow for enabling of the lightbox Aug 30, 2023
@github-actions
Copy link

github-actions bot commented Aug 31, 2023

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files
❔ lib/class-wp-theme-json-schema-gutenberg.php
❔ lib/block-supports/behaviors.php
❔ lib/blocks.php
❔ lib/class-wp-theme-json-gutenberg.php
❔ lib/load.php
❔ lib/theme.json

michalczaplinski and others added 12 commits September 2, 2023 19:56
Simplified the ImageSettingsPanel and ScreenBlock components. More specific changes:
 - Removed `name` and `settings` from the ImageSettingsPanel
 - Use `userSettings` instead of `settings` in the ImageSettingsPanel
 - Modified `onChangeLightbox` logic, it now takes a new setting instead of a boolean and directly passes to `onChange`
 - Updated the ScreenBlock component to account for this refactor
A new option has been added to the lightbox settings in the Theme JSON reference guide and Gutenberg class. This `showUI` option allows users to toggle whether the Lightbox UI is displayed in the block editor or not. Also, updated the JSON schema accordingly to reflect these changes in theme.json files.
I moved the logic to determine whether the lightbox should display
or not to two render_block_data filters.

One of these filters is inside of the index.php
so that itc can exist in WP core, the other inside of blocks.php
in order to offer legacy support for the Behaviors syntax in
the Gutenberg plugin.

Using the render_block_data instead of render_block allows us to
store a 'lightboxEnabled' value on the block, which we can use to
determine whether the lightbox should be rendered in these two
separate locations relatively cleanly without needing to touch
the markup.

I added behaviors back to the valid top-level keys so that we can
read it to offer legacy support.

Lastly, I set the lightbox.enabled attribute to NULL by default
so that we can determine whether the Behaviors syntax should override
it or not.
…ock editor UI should inherit the value from `theme.json`.

Likewise, if no value is set in the block attributes, the block editor UI should inherit the value from the Global Styles of the Image.
@artemiomorales artemiomorales changed the title Image block: Add settings panel to image inspector to allow for enabling of the lightbox Image block: UI improvements for the image lightbox Sep 6, 2023
@artemiomorales artemiomorales changed the title Image block: UI improvements for the image lightbox Image block: UI updates for the image lightbox Sep 6, 2023
Rather than trying to check if the 'enabled' has been set or not
and falling back to other levels of the theme.json inheritance
structure, I decided to just read and use the settings as defined
in theme.json. This is the expected behavior in Gutenberg from
what I understand and has less edge cases.
@artemiomorales
Copy link
Contributor Author

I think an easier solution than adding a custom helper would be to move the settings in /lib/theme.json to the top-level:

That works. I went ahead and made that change.

Along these lines, I also realized I was complicating this too much — previously I was trying to inherit the top-level settings.lightbox.enabled if the settings.blocks.core/image.lightbox.enabled was undefined, but from what I'm seeing this is not the expected behavior.

As long as the lightbox object exists now at the block level, we'll just use that as written, which I believe is how it's supposed to work.

schemas/json/theme.json Outdated Show resolved Hide resolved
michalczaplinski and others added 4 commits September 14, 2023 12:00
Co-authored-by: Alex Lende <alex+github.com@lende.xyz>
…hp` is only put in GB as a temporary migration.
@github-actions
Copy link

github-actions bot commented Sep 14, 2023

Flaky tests detected in 485c197.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6198328954
📝 Reported issues:

@artemiomorales artemiomorales merged commit 1b3a480 into update/revise-lightbox-ui Sep 15, 2023
48 of 50 checks passed
@artemiomorales artemiomorales deleted the update/lightbox-settings-panels branch September 15, 2023 16:59
artemiomorales added a commit that referenced this pull request Sep 15, 2023
@artemiomorales artemiomorales restored the update/lightbox-settings-panels branch September 15, 2023 17:08
artemiomorales added a commit that referenced this pull request Sep 15, 2023
michalczaplinski added a commit that referenced this pull request Sep 15, 2023
* Begin removing theme.json dependency in block UI

* Remove the useHasBehaviorsPanel hook

* Fix  declaration to actually retrieve from user data

* Restructured use of global behaviors

Simplified the `__experimentalUseGlobalBehaviors` function by removing the 'source' parameter. Now, the function directly uses 'userConfig' for getting raw data and 'mergedConfig' for variable value.

* More removal of behaviors.

* Remove the reference to behaviors in Global styles and first iteration of updates to the lightbox UI

* Remove behaviors altogether and everywhere

* Fix linter & sniffer PHP errors

* Adjust schema properties count assertion

Previously, the test was checking whether schema properties array had exactly 10 elements
We're now checking for exactly 9 elements instead.

* Add the lightbox attribute

* Remove unnecessary space

* Add clarifying comment regarding skipped tests

* Do not remove `behaviors` attribute from Image block's block.json.
Behaviors are deprecated for 2 more releases and will be removed then.

* Revert "Do not remove `behaviors` attribute from Image block's block.json."

This reverts commit cabe31b.

* Revert deletion of behaviors.php

* Image block: UI updates for the image lightbox (#54071)

* Add initial implementation of image settings panel

* Remove unnecessary code and fix reset functionality

* Add UI to image block inspector

* Add `lightbox` to the valid `theme.json` settings

* Fix a bug with image selector and integrate with global styles

* Update `theme.json` schema

* Added the `@since`annotation

* Refactor image settings panel and screen block

Simplified the ImageSettingsPanel and ScreenBlock components. More specific changes:
 - Removed `name` and `settings` from the ImageSettingsPanel
 - Use `userSettings` instead of `settings` in the ImageSettingsPanel
 - Modified `onChangeLightbox` logic, it now takes a new setting instead of a boolean and directly passes to `onChange`
 - Updated the ScreenBlock component to account for this refactor

* Add showUI option to lightbox settings

A new option has been added to the lightbox settings in the Theme JSON reference guide and Gutenberg class. This `showUI` option allows users to toggle whether the Lightbox UI is displayed in the block editor or not. Also, updated the JSON schema accordingly to reflect these changes in theme.json files.

* Add defaults for the `lightbox` to the GB `theme.json`

* Change the falsy checks in image.js

* Add filters; add legacy support for behaviors syntax

I moved the logic to determine whether the lightbox should display
or not to two render_block_data filters.

One of these filters is inside of the index.php
so that itc can exist in WP core, the other inside of blocks.php
in order to offer legacy support for the Behaviors syntax in
the Gutenberg plugin.

Using the render_block_data instead of render_block allows us to
store a 'lightboxEnabled' value on the block, which we can use to
determine whether the lightbox should be rendered in these two
separate locations relatively cleanly without needing to touch
the markup.

I added behaviors back to the valid top-level keys so that we can
read it to offer legacy support.

Lastly, I set the lightbox.enabled attribute to NULL by default
so that we can determine whether the Behaviors syntax should override
it or not.

* Fix linter errors & add more expansive comments.

* If no value is set for the lightbox in the Global Styles, then the block editor UI should inherit the value from `theme.json`.

Likewise, if no value is set in the block attributes, the block editor UI should inherit the value from the Global Styles of the Image.

* Rename `showUI` to `allowEditing`

* Fix the `theme.json` schema

* Use the globalPath for the settings on the PHP side

* Add backwards support for enabling fade animation via the legacy syntax

* Fix PHP linter errors

* Fix error when checking lightbox['enabled'] value in global settings

* Empty commit

* Remove the default `false` value for `lightbox.enabled`attribute.

* Add deprecation notice for 'behaviors' in image block

I needed to add 'behaviors' back to the block.json attributes
in order to read them on the JavaScript side in the editor
to fire the deprecation notice.

* Add deprecation for  attribute in image block

* Remove obsolete code now that block deprecation is in place

* Add support for 'lightbox: true' syntax

* Fix lightbox 'checked' attribute being read improperly

* Add conditional display for settings panel at image block level

* Fix an error with the theme.json schema.

* Update docs with `npm run build:docs`

* Copy `class-wp-theme-json-schema.php` from core
 into `class-wp-theme-json-schema-gutenberg.php`

* Add a theme.json migration to v3 away from behaviors and to a new, simpler syntax used by the lightbox.

* Remove the `null` value from lightbox.enabled in `lib/theme.json`

* Remove `behaviors` from VALID_TOP_LEVEL_KEYS & VALID_SETTINGS

* Revise backwards compatibility for behaviors; add deprecation to block_supports

* Remove outdated comment

* Update outdated comment

* Update comment and shuffle the lines so the diff is easier on the eyes.

* Update comment to explain why we use userSettings in image-settings-panel.js

* Remove support for legacy fade configuration

* Resolve lint error

* Add clarifying comment regarding lightbox markup

* Rename the migrate function to reflect that it's not a v3 migration

* Add a `@since` in `gutenberg_should_render_lightbox` docblock

* Add support for reading top-level 'lightbox' setting in editor

By default, we read the lightbox settings underneath the 'core/image'
in theme.json; however, the 'enabled' property there is undefined by default,
which means it should be possible to declare a top-level setting for the lightbox
that overrides an undefined block-level setting.

While this appeared to be working on the PHP side, the UI wasn't accurately
reflecting this inheritance structure, so this commit fixes that.

Users should now be able to define a top-level lightbox setting as either
'lightbox: true' or 'lightbox: { enabled: true }' that will be used
if the block-level lightbox setting for 'enabled' is undefined.

* Revert "Add support for reading top-level 'lightbox' setting in editor"

This reverts commit 2f5f122.

* Add correct deprecation mentioning the Gutenberg version

* Move 'allowEditing' to top-level settings

* Fix top-level lightbox setting not being read properly

* Fix 'false' values in theme.json not being stored in settings object

* Fix error wherein 'undefined' was being passed to input component

* Fix bug wherein lightbox UI would disappear if user value was set

* Remove inheritance when determining whether to enable lightbox

Rather than trying to check if the 'enabled' has been set or not
and falling back to other levels of the theme.json inheritance
structure, I decided to just read and use the settings as defined
in theme.json. This is the expected behavior in Gutenberg from
what I understand and has less edge cases.

* Update comment

* Update whitespace in theme.json

Co-authored-by: Alex Lende <alex+github.com@lende.xyz>

* Add docblocks to clarify that `class-wp-theme-json-schema-gutenberg.php` is only put in GB as a temporary migration.

* Add comments to clarify that behaviors.php is temporarily added to GB an will be removed in a future version.

* Added integration fixtures for the lightbox

* Fix incorrect reading of global lightbox settings

* Clarify the comment getting the global lightbox settings

* Fix PHPCS parenthesis error 🤦‍♂️

* Move lightbox settings to the block level

* Remove support for shorthand

* Remove 'lightbox' from hooks.js and add `.enabled` & `.allowEditing` to class-wp-theme-json-gutenberg.php

---------

Co-authored-by: Michal Czaplinski <mmczaplinski@gmail.com>
Co-authored-by: Alex Lende <alex+github.com@lende.xyz>

* Revert "Image block: UI updates for the image lightbox (#54071)"

This reverts commit 1b3a480.

---------

Co-authored-by: Michal Czaplinski <mmczaplinski@gmail.com>
Co-authored-by: Alex Lende <alex+github.com@lende.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Feature] Interactivity API API to add frontend interactivity to blocks. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants