Forms: Increase form fields padding based on user-defined border-radius

[jcheringer]

Fixes #28776

Proposed changes:

• Increase form fields padding based on user-defined border-radius. More context on Form block - Styling settings issues: Input field radius at maximum collides with text input area #28776

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

N/A

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

No

Testing instructions:

• Create a Post and add a Form block
• Increase the border-radius from a field using the Sidebar settings
• Check if the text/placeholder isn't hidden even when using a big border-radius.

[github-actions]

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack update/form-field-padding

to get started. More details: p9dueE-5Nn-p2

[github-actions]

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.
  
• ⚠️ All commits were linted before commit.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ 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.

---

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

---

Jetpack plugin:

• Next scheduled release: March 7, 2023.
• Scheduled code freeze: February 28, 2023.

[CGastrell]

Plugin: Kind of works? placeholders and values look a bit weird, but not hidden:

Package: doesn't work at all, but you already knew this

[CGastrell]

Works on package now, published render still looking a bit weird though:
Editor:

Publish:

[digitalwaveride]

I am seeing the issue Christian mentioned, also in the editor, the moment the border-radius goes over 25.

Below it the "max radius":

In a best-case scenario, the text would start about where the "radius stops":

[jcheringer]

There's no 'magic number' here.
I don't have a way to know when I should stop increasing the padding because the amount of radius necessary to make it fully rounded depends on the field height, and that will change from theme to theme.
Is up to the user to stop increasing the border-radius as soon as they notice the field has enough radius for their purpose.
I'd say that's an issue.

cc @digitalwaveride @CGastrell

[CGastrell]

But, apparently, the padding is somewhat being set relative to the border-radius value, right? If so, we might be capping the border-radius value but not the padding?

[CGastrell]

We are assigning a fallback value for the padding, but not for the border-radius, would that help?

[jcheringer]

But, apparently, the padding is somewhat being set relative to the border-radius value, right? If so, we might be capping the border-radius value but not the padding?

We're not capping the border-radius, that's the browser's default behavior

[CGastrell]

But, apparently, the padding is somewhat being set relative to the border-radius value, right? If so, we might be capping the border-radius value but not the padding?

We're not capping the border-radius, that's the browser's default behavior

Exactly my point. We are overriding the limit of one but not the other. Then again, this is what happens when we allow so fine grained customization, people might just go for out-of-sane-range values and break it. We need to make it look good within coherent values, but if you go off the reservation, then it's up to you.

[jcheringer]

But, apparently, the padding is somewhat being set relative to the border-radius value, right? If so, we might be capping the border-radius value but not the padding?

We're not capping the border-radius, that's the browser's default behavior

Exactly my point. We are overriding the limit of one but not the other. Then again, this is what happens when we allow so fine grained customization, people might just go for out-of-sane-range values and break it. We need to make it look good within coherent values, but if you go off the reservation, then it's up to you.

Not sure I understand your point.
The code snippet you mentioned above sets the minimum padding we want if the CSS variable isn't defined.
I have no way to define the max padding allowed because that would depend on the field height, which can change from theme to theme.

[CGastrell]

ok, yeah, that wasn't clear enough. So, 2 things:

• first: there is no fallback value for --jetpack--contact-form--border-radius, meaning if it's not set, there's no value to compare to on the max() call. Would be wise to have a safe default/fallback there, like var(--jetpack--contact-form--border-radius, 0). Just so it doesn't get an invalid value.
• second: we can set a max padding by use of min() CSS function. So, after all the above evaluation took place, we use min( [some safe maximum padding value here], [all the evaluated calculus here] ). This way, when our calculation of max/var/padding/radius has gone wild, we can set it to a default "maximum" value as min() will pick the smaller value.

Example: the calculation gives you some 100px, but we all know more than (let's say) 32px padding will look bad. So:

padding-left: min(
  max( var( --jetpack--contact-form--input-padding-left, '16px' ), var( --jetpack--contact-form--border-radius ) ) // this goes over 100px
  '32px' // this gets picked as the other value has gone wild
);

[jcheringer]

I'd avoid the 'magic' 32px because we can't ensure it would fit all themes.
However, if we want to limit the padding, 32px seems reasonable.
I'll leave it up to @digitalwaveride to decide. 😬

[jcheringer]

Just to include an example.
32px doesn't work well for the textarea, we'd need to go with a higher value.

[digitalwaveride]

Thanks so much for this João!
It's much improved, especially for the multi-line text input. 🚀

[jeherve]

This tests well for me, but will need a rebase.

.