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

[PoC] Forms: Try using inner blocks for fields to aid styling via Global Styles #41281

Closed
wants to merge 18 commits into from

Conversation

aaronrobertshaw
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw commented Jan 23, 2025

⚠️ ⚠️ This is an exploration for discussion purposes only. It is not intended to be merged! ⚠️ ⚠️

At this stage, it isn't even really ready for discussion and is mostly just a collection of quick hacks and copy pasta 😅

Proposed changes:

Update form fields to use inner blocks for labels and inputs. This aims to allow each of those inner blocks to opt into appropriate block supports and benefit from styling via Global Styles and theme.json.

Exploration Phase 1

What this exploration aims to answer

These are the UX questions I’m expecting the spike Aaron is doing answer:

  • Impact on customizing individual labels & fields UX?
  • Impact on theme inheritence, how easy to its for designer to style forms, and overwrite with their own global styles or CSS?
  • How would “I want to change appearance of all input fields in this form” look?
  • How would “I want to change all forms appearance on my site” look?

And then we can decide how we feel about the solution and is it worth proceeding.

What Doesn't This Draft PR Do?

  1. Update all fields to use inner blocks
    • At this stage, this exploration only refactors the text field blocks
    • It might make less sense for something like a checkbox or radio button field to use inner blocks
  2. Render the new blocks on the frontend.
    • The shortcode rendering needs updating to use the inner block attributes for content, classes, and styles.
  3. Sync styles across fields.
    • At this stage, the old controls remain at the field container level for the new field blocks
    • A couple of new HoCs are probably needed to provide the same syncing of styles to fields that opt into that behaviour e.g. withSharedInputAttributes and withSharedLabelAttributes
  4. Deprecate old form blocks
    • The original field blocks will likely need to remain registered so they can be deprecated and migrated to the new inner field blocks
    • The original field blocks can be flagged as deprecated and hidden from the inserters

Next steps

If we choose to invest further time into leveraging inner blocks for form fields, the next steps should include:

  1. Investigate deprecations to ensure we maintain backward compatibility with existing forms
  2. Make sure the inner blocks and their styles still play nice with the shortcode rendering of the blocks on the frontend
  3. Work out what updates if any are required for non-text fields like checkboxes, selects etc. to be stylable via global styles
  4. Make sure that the block variations play nice with translated default attribute text values
  5. Clean up all the hacks 🧹

Quick Demo

Screen.Recording.2025-01-23.at.8.03.34.pm.mp4

What this exploration aims to answer

Some of the questions this exploration aims to answer include:

  • Impact on customizing individual labels & fields UX?
  • Impact on theme inheritence, how easy to its for designer to style forms, and overwrite with their own global styles or CSS?
  • How would “I want to change appearance of all input fields in this form” look?
  • How would “I want to change all forms appearance on my site” look?
  • Can all the existing field types be migrated to use inner blocks via block migrations?
  • Will the current shortcode rendering of forms extend to support fields using inner blocks, global styles, and block supports?
  • Can the existing syncing of styles between labels/fields be achieved if split between inner blocks?
  • Can global styles and block supports on inner blocks work alongside the form block's use of custom CSS properties?
  • Are there any blockers to using inner blocks?
  • What new blocks would be required?
  • How much work and effort is required to safely migrate form fields to inner blocks?
  • Any other risks to weigh up?

What Doesn't This Draft PR Do?

  1. Implement Option and Options inner blocks
    • This includes using these blocks for the Single/Multi choice form fields
    • These blocks will be needed to globally style option label separately to normal form field labels
  2. Rendering of option field inner block styles on the frontend.
    • Updating the shortcode rendering for options will be a little more involved, as options can no longer be represented as an array of strings, we'll need to pass around the option string, its classes and styles
  3. Choice field deprecations are incomplete as they'd require the new blocks.
    • There are also some challenges here as this will require deprecating the current option blocks to use inner blocks themselves while also deprecating the parent to wrap the migrated options into a new "Options" wrapper block.

Essentially, most field types have been migrated to inner blocks (albeit in a hacky way). Time ran out on the exploration before new options related blocks could be implemented and choice fields properly migrated. So while the list above notes choice fields with inner blocks aren't complete or working, they should definitely be doable.

Next steps

If we choose to invest further time into leveraging inner blocks for form fields, the next steps should include:

  • Decide on committing to fully implementing inner block based fields
  • Decide on an overall approach e.g. experiment/feature flag, separate feature branch etc
  • Breakdown the work involved into atomic tasks that can be shared around
  • Leverage this PR as a guide to accelerate the tasks defined in the previous step

Testing instructions:

TBA

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] In Progress [Package] Forms labels Jan 23, 2025
@aaronrobertshaw aaronrobertshaw self-assigned this Jan 23, 2025
Copy link
Contributor

github-actions bot commented Jan 23, 2025

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the try/splitting-form-field-blocks branch.

  • To test on Simple, run the following command on your sandbox:

    bin/jetpack-downloader test jetpack try/splitting-form-field-blocks
    

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions bot added [Block] Contact Form Form block (also see Contact Form label) [Feature] Contact Form labels Jan 23, 2025
Copy link
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  • ✅ Add testing instructions.
  • 🔴 Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

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 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


🔴 Action required: We would recommend that you add a section to the PR description to specify whether this PR includes any changes to data or privacy, like so:

## Does this pull request change what data or activity we track or use?

My PR adds *x* and *y*.

Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Choose a review path based on your changes:
    • A. Team Review: add the "[Status] Needs Team Review" label
      • For most changes, including minor cross-team impacts.
      • Example: Updating a team-specific component or a small change to a shared library.
    • B. Crew Review: add the "[Status] Needs Review" label
      • For significant changes to core functionality.
      • Example: Major updates to a shared library or complex features.
    • C. Both: Start with Team, then request Crew
      • For complex changes or when you need extra confidence.
      • Example: Refactor affecting multiple systems.
  3. Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Code Coverage Summary

No summary data is avilable for parent commit bdf6479, cannot calculate coverage changes. 😴

If that commit is a feature branch rather than a trunk commit, this is expected. Otherwise, you might try re-running the Tests / Publish coverage data job on this PR once the corresponding job on the trunk commit has succeeded.

Full summary · PHP report · JS report

@aaronrobertshaw aaronrobertshaw force-pushed the try/splitting-form-field-blocks branch from 0ca93b3 to 2840207 Compare February 3, 2025 09:52
@aaronrobertshaw aaronrobertshaw force-pushed the try/splitting-form-field-blocks branch from be7c411 to 529d1f7 Compare February 5, 2025 01:18
@aaronrobertshaw aaronrobertshaw force-pushed the try/splitting-form-field-blocks branch from 7ef2355 to b1e5716 Compare February 5, 2025 03:59
@aaronrobertshaw aaronrobertshaw force-pushed the try/splitting-form-field-blocks branch from b1e5716 to 273a949 Compare February 5, 2025 06:50
@aaronrobertshaw aaronrobertshaw force-pushed the try/splitting-form-field-blocks branch from a089e1d to 22728c4 Compare February 6, 2025 06:14
Important: Note that this commit does not include updates that will
apply global styles and block support styles to the individual option
fields. This will require a larger refactor that might also change based
on how inner blocks for the choice fields shake out.
@aaronrobertshaw
Copy link
Contributor Author

This PR was meant as a vehicle for exploring blockers, backward compatibility requirements, necessary compromises, and UX implications of inner blocks on form fields.

It has served its purpose so I'll close it now to avoid further confusion and expectations for this PR to be merged.

Some next steps from here will be:

  • Decide on committing to fully implementing inner block based fields
  • Decide on an overall approach e.g. experiment/feature flag, separate feature branch etc
  • Breakdown the work involved into atomic tasks that can be shared around
  • Leverage this PR as a guide to accelerate the tasks defined in the previous step

@ntsekouras
Copy link
Member

Thank you for this exploration @aaronrobertshaw! I know this is something really challenging but I'm curious about the main blockers that led you to close this PR instead of pushing forwards?

@aaronrobertshaw
Copy link
Contributor Author

Thanks for the question @ntsekouras 👍

I'm in the process of writing up findings but to answer your question specifically:

I know this is something really challenging but I'm curious about the main blockers that led you to close this PR instead of pushing forwards?

I closed the PR to manage expectations not due to blockers. On the contrary, switching to inner block based form fields in a backward compatible manner is definitely achievable.

This PR has been referenced multiple times in a way that suggests to others following less closely that it is:

a) imminent
b) intended to be a proper implementation
c) a silver bullet to lots of styling issues

When a complete implementation gets a green light, there is nothing preventing this PR being resurrected.

@aaronrobertshaw aaronrobertshaw changed the title [WIP] Forms: Try using inner blocks for fields to aid styling via Global Styles [PoC] Forms: Try using inner blocks for fields to aid styling via Global Styles Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Contact Form Form block (also see Contact Form label) [Feature] Contact Form [Package] Forms [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants