Forms: disable device visibility on fields/inputs, honor per-viewport hiding on labels#49973
Forms: disable device visibility on fields/inputs, honor per-viewport hiding on labels#49973CGastrell wants to merge 9 commits into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 3 files.
|
Device/viewport visibility ("Hide on…") is not honored on the field wrapper
at render time — the field render pipeline bypasses core's render_block
visibility filter — so the control did nothing. Disable the 'visibility'
block support on field and input blocks on both the JS (shared/settings,
input) and PHP (register_block_type_args filter) registrations so the
non-functional option no longer appears. Labels keep visibility support.
See FORMS-694.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aa49d16 to
5d61ca2
Compare
The per-viewport "Hide on…" option is the part that isn't honored (and on a required field can't be made safe); "hide everywhere" does work for fields, but the GB control bundles both under one supports boolean, so we disable it wholesale on fields/inputs as an interim. Comment/changelog wording only — no functional change. Labels untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ktop)
Labels kept blockVisibility support but only full-hide was wired; the
per-viewport selection was ignored because the field renderer discards the
label's own rendered output (where Gutenberg would have added the
wp-block-hidden-* classes). Read blockVisibility.viewport in the block->shortcode
bridge and add the same wp-block-hidden-{mobile,tablet,desktop} classes to the
label the field renderer emits; the matching media-query CSS is already
registered by core's render_block visibility filter (label keeps support).
Also apply label classes to the 'below' style-variation label for parity.
Verified on the frontend: hidden-on-mobile label gets the class and the
@media (width <= 480px){.wp-block-hidden-mobile{display:none}} rule is emitted.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Nice work on this, the interim approach makes sense. I double-checked the part everyone was unsure about:
|
The standard jetpack/input was disabled, but the field-specific input blocks (input-range/slider, input-rating, input-image-option, phone-input, dropzone/file) define their own supports and don't share defaultSettings, so the control still showed on them. Add visibility:false to each (JS) and broaden the PHP filter to match all jetpack/input* plus phone-input and dropzone. Verified in-editor: every field-* and input block now hides the control; only label keeps it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LiamSarsfield
left a comment
There was a problem hiding this comment.
The input-block gap from last round. phone-input, range/rating/image, and dropzone are covered now in both JS and PHP, and the visibility key checks out against WP 7.0 core. 🚀
Some nice to haves that may be worth addressing:
Tests. Only the jetpack/input supports line is locked down today, so reverting the per-viewport label mapping, the full-hide case, or any widened input would stay green. That's the gap that needed a follow-up commit this round. A @dataProvider test on disable_field_visibility_support() across the name matrix (a field, a field-option-*, the prefix inputs, phone-input/dropzone, a non-forms block), plus a couple of Contact_Form_Plugin_Test cases for blockVisibility = false and viewport.mobile = false, covers it.
Grouped fields skip full-hide on their label. render_legend_as_label() never checks labelhiddenbyblockvisibility, so a fully-hidden label still renders on radio, checkbox-multiple, image-select, and rating, while render_label() honors it. It predates this PR, but since this is the change that makes labels honor visibility, finishing it here (or a fast-follow) would round it out. The per-viewport classes already reach the legend, so only the full-hide branch is missing.
Per team decision, the choice/option blocks (option, options, fieldset-image-options, and the deprecated field-option-radio/checkbox) are inputs in effect — they flatten through the same field-shortcode path, so visibility is inert on them. Add visibility:false (JS) and extend the PHP filter (drop the field-option-* exclusion; add the option containers). Label is now the only forms block keeping the control. Verified in-editor: every field/input/option block hides the control; only label shows it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…y tests - render_legend_as_label() now respects labelhiddenbyblockvisibility, so a fully-hidden label is dropped on radio/checkbox-multiple/image-select/rating fields too (it already honored per-viewport hide via label_classes). Mirrors render_label(); addresses Liam's review. - Add a dataProvider matrix test for disable_field_visibility_support() (field, field-option, all input variants, option containers, label, non-forms block). - Add label blockVisibility tests for block_attributes_to_shortcode_attributes: full-hide sets the flag, per-viewport adds wp-block-hidden-* classes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Proposed changes
Interim measure for FORMS-694 while we decide whether to fully support per-field device visibility.
What the GB control actually does. The block "visibility" support exposes two modes behind a single
supports.visibilityboolean: Hide everywhere (writesblockVisibility = false) and per-viewport "Hide on mobile/tablet/desktop" (writes{ viewport: … }).How they behave on our (dynamic) form blocks:
render_blockclass injection, sowp-block-hidden-*+ the media-query CSS never reach the field. Worse, it can't be made safe on a required field: both server (validate()) and client (isFormValid) validation are viewport-blind, so a required field hidden on mobile stays required but unfillable → dead form.required-safe), but the single boolean bundles it with the broken per-viewport mode and we can't split them.So this disables
visibilityon every field, input, and choice/option block (the only one that keeps it is the label — see below).Labels keep their support — and now honor per-viewport hiding too. Label hiding never touches input/validation/submission (it's pure presentation), so the viewport pitfall doesn't apply. Full-hide was already wired via
labelhiddenbyblockvisibility; this adds the per-viewport case: the block→shortcode bridge readsblockVisibility.viewportand adds the samewp-block-hidden-{mobile,tablet,desktop}classes Gutenberg would to the label the field renderer emits. The matching media-query CSS is already registered by core'srender_blockvisibility filter (the label keepsvisibilitysupport, so the filter runs on it — it just couldn't attach the class because that output is discarded). Applied across the default, outlined, animated, and below label styles.Applied consistently on both registrations:
shared/settings/index.js(all field blocks), plus per-blockvisibility: falseon the input variants (input,input-range,input-rating,input-image-option,phone-input,dropzone) and choice/option blocks (option,options,fieldset-image-options, deprecatedfield-option-*).register_block_type_argsfilter matching alljetpack/field-*,jetpack/input*,phone-input,dropzone, and the option containers, mirroring the JS.Related
Does this pull request change what data or activity we track or use?
No.
Testing instructions
<label>getswp-block-hidden-mobileand the@media (width <= 480px){…display:none}rule is emitted, so it hides on mobile. Full-hide on a label still removes it entirely.wp.blocks.hasBlockSupport('jetpack/field-name','visibility',true) === falsefor fields/input andtruefor labels/options; serverWP_Block_Type_Registryreports the same. PHPContact_Form_Block_Testpasses.Deferred (separate decision, FORMS-694)
Fully supporting per-field device visibility would require honoring viewport hiding inside the forms render pipeline and a viewport flag on the submission so required/validation can be skipped for the hidden viewport on both client and server — plus token handling. Likely not worth it; tracked in FORMS-694.