-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Introduce responsive editing #73888
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
base: trunk
Are you sure you want to change the base?
Introduce responsive editing #73888
Changes from all commits
7dafbad
8c88af5
4f176cf
3d5cf2c
b93acc2
6b9d43e
0a8f594
46be7db
2e38185
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,32 @@ | |||||
| */ | ||||||
|
|
||||||
| /** | ||||||
| * Render nothing if the block is hidden. | ||||||
| * Add 'display' to allowed CSS properties for the style engine. | ||||||
| * | ||||||
| * @param array $styles Array of allowed CSS properties. | ||||||
| * @return array Modified array with 'display' added. | ||||||
| */ | ||||||
| function gutenberg_add_display_to_safe_style_css( $styles ) { | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: should
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might not need this at all since the style engine generates proper CSS rules, and enqueues them. In this case we're not using inline styles unless I'm missing something. That said, I'm putting together a "backend" PR behind the experiment to kick this feature off. It'll take the best bits from both PRs.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ignore me. My brain just reminded me that I wrote
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we need to add |
||||||
| $styles[] = 'display'; | ||||||
| return $styles; | ||||||
| } | ||||||
| add_filter( 'safe_style_css', 'gutenberg_add_display_to_safe_style_css' ); | ||||||
|
|
||||||
| /** | ||||||
| * Get the breakpoint media queries for responsive visibility. | ||||||
| * | ||||||
| * @return array Associative array of viewport => media query. | ||||||
| */ | ||||||
| function gutenberg_get_responsive_visibility_breakpoints() { | ||||||
| return array( | ||||||
| 'desktop' => '@media screen and (min-width: 782px)', | ||||||
| 'tablet' => '@media screen and (min-width: 600px) and (max-width: 781px)', | ||||||
| 'mobile' => '@media screen and (max-width: 599px)', | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Render nothing if the block is hidden, or add responsive visibility classes. | ||||||
| * | ||||||
| * @param string $block_content Rendered block content. | ||||||
| * @param array $block Block object. | ||||||
|
|
@@ -19,10 +44,80 @@ function gutenberg_render_block_visibility_support( $block_content, $block ) { | |||||
| return $block_content; | ||||||
| } | ||||||
|
|
||||||
| if ( isset( $block['attrs']['metadata']['blockVisibility'] ) && false === $block['attrs']['metadata']['blockVisibility'] ) { | ||||||
| if ( ! isset( $block['attrs']['metadata']['blockVisibility'] ) ) { | ||||||
| return $block_content; | ||||||
| } | ||||||
|
|
||||||
| $block_visibility = $block['attrs']['metadata']['blockVisibility']; | ||||||
|
|
||||||
| // If blockVisibility is false, hide the block completely (original behavior). | ||||||
| if ( false === $block_visibility ) { | ||||||
| return ''; | ||||||
| } | ||||||
|
|
||||||
| // If blockVisibility is an object, handle responsive visibility. | ||||||
| if ( is_array( $block_visibility ) ) { | ||||||
| $allowed_devices = array( 'desktop', 'tablet', 'mobile' ); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we allow filtering this list, together with the list of breakpoints? |
||||||
| $hidden_devices = array(); | ||||||
|
|
||||||
| foreach ( $block_visibility as $device => $is_hidden ) { | ||||||
| // Only allow whitelisted device names. | ||||||
| if ( false === $is_hidden && in_array( $device, $allowed_devices, true ) ) { | ||||||
| $hidden_devices[] = $device; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // If all devices are hidden, return empty. | ||||||
| if ( count( $hidden_devices ) === 3 && | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally, we don't hardcode this. Maybe at some point we allow adding additional (or intentionally removing) allowed devices.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then we'd make that filterable and adapt. For now it's a good way to ensure no extraneous info sneaks in. |
||||||
| in_array( 'desktop', $hidden_devices, true ) && | ||||||
| in_array( 'tablet', $hidden_devices, true ) && | ||||||
| in_array( 'mobile', $hidden_devices, true ) | ||||||
|
Comment on lines
+72
to
+74
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this separate check needed? Isn't the count check enough? |
||||||
| ) { | ||||||
| return ''; | ||||||
| } | ||||||
|
|
||||||
| // If there are responsive hiding rules, add a class and generate CSS. | ||||||
| if ( ! empty( $hidden_devices ) ) { | ||||||
| // Generate a unique class name based on the hidden devices. | ||||||
| sort( $hidden_devices ); | ||||||
| $visibility_class = 'wp-block-visibility-' . implode( '-', $hidden_devices ); | ||||||
|
|
||||||
| // Generate CSS rules using the style engine. | ||||||
| $breakpoints = gutenberg_get_responsive_visibility_breakpoints(); | ||||||
| $css_rules = array(); | ||||||
|
|
||||||
| foreach ( $hidden_devices as $device ) { | ||||||
| if ( isset( $breakpoints[ $device ] ) ) { | ||||||
| $css_rules[] = array( | ||||||
| 'selector' => '.' . $visibility_class, | ||||||
| 'declarations' => array( | ||||||
| 'display' => 'none', | ||||||
| ), | ||||||
| 'rules_group' => $breakpoints[ $device ], | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if ( ! empty( $css_rules ) ) { | ||||||
| gutenberg_style_engine_get_stylesheet_from_css_rules( | ||||||
| $css_rules, | ||||||
| array( | ||||||
| 'context' => 'block-supports', | ||||||
| ) | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| // Add the visibility class to the block. | ||||||
| $processor = new WP_HTML_Tag_Processor( $block_content ); | ||||||
| if ( $processor->next_tag() ) { | ||||||
| $existing_class = $processor->get_attribute( 'class' ); | ||||||
| $new_class = $existing_class ? $existing_class . ' ' . $visibility_class : $visibility_class; | ||||||
| $processor->set_attribute( 'class', $new_class ); | ||||||
| $block_content = $processor->get_updated_html(); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| return $block_content; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,3 +62,7 @@ | |
| justify-content: center; | ||
| } | ||
| } | ||
|
|
||
| .block-editor-block-inspector__visibility-notice { | ||
| margin: 0 $grid-unit-20 $grid-unit-20; | ||
| } | ||
|
Comment on lines
+66
to
+68
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I wish this were not necessary. Any other way to apply this spacing? Maybe an extra |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,7 @@ | ||
| export { default as BlockVisibilityMenuItem } from './menu-item'; | ||
| export { default as BlockVisibilityToolbar } from './toolbar'; | ||
| export { | ||
| isHiddenForViewport, | ||
| hasAnyVisibilitySettings, | ||
| VIEWPORT_LABELS, | ||
| } from './utils'; |
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.
Should type be consistent with the filter type (
string[])?