-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Block Image: Lightbox - Hide animation selector if behavior is Default or None #51748
Block Image: Lightbox - Hide animation selector if behavior is Default or None #51748
Conversation
Size Change: +12.2 kB (+1%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 07c22ca. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5356751213
|
da2b0a2
to
4e27cff
Compare
4e27cff
to
3d083f2
Compare
I also added the zoom-out icon on the enlarged image. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@c4rl0sbr4v0 I tested with the following config and got the result that when 'Default' is selected, the dropdowns values switch to Lightbox
and Zoom
in the UI rather than staying on Default
with the animation dropdown hidden.
"behaviors": {
"blocks": {
"core/image": {
"lightbox": {
"enabled": true
}
}
}
}
behaviors-animation-dropdown.mp4
True, it seems that the last refactor broke the fix. Will take a look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and works as expected 👍
I feel like the logic in the behaviors.js
file can probably be refactored to more clearly express the intent, but we can clean this up in additional PRs as the behaviors functionality gets better fleshed out.
…t or None (WordPress#51748) * Hide animation selector if default or none * Small refactor, fix on translation * Add zoom out icon * Fix selector, but still dont like the solution * Moved to own function * Add useMemo
What?
Before this PR, if you don't define an animation in theme.json, but you have lightbox enabled defined in the same file, it won't work.
Also, the animation selector should not be displayed if the behavior selected is
None
orDefault
Testing Instructions
Edit your theme.json file to be like this
Check that the lightbox is working with zoom as default.
Go to post/page edition. Add an Image. Select behavior as
None
orDefault
. The Animation selector should dissapear.