forms immediate server side validation #1672

neall · 2022-12-05T21:34:54Z

Description

This adds optional client-side functionality to Forms framework text fields so they can validate against the server as the user types.

I had to touch a lot of lines in form_control.html.erb, so I wrapped that in some more tests. There's still a lot of logic in that file but I moved some of it into other places where it is easier to read.

In particular that file is where we render the error-message-related elements. We normally don't render them if there is no error on the field, but if we are using auto-check and updating the client we need to render those elements hidden.

Integration

This won't require any changes to existing forms; nothing changes unless the developer specifies a URL for auto_check_src on their text field.

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews

changeset-bot · 2022-12-05T21:34:57Z

🦋 Changeset detected

Latest commit: 3fd3217

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2022-12-05T21:42:28Z

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

camertron

Yasss! 🎉 Just a few minor things.

test/lib/primer/forms/deep_thought.rb

test/lib/primer/forms/forms_test.rb

lesliecdubs · 2022-12-06T05:37:26Z

Oh, this is neat! Was there an issue tied to this effort? Curious if we involved Primer accessibility design in the decisions about how the client-side validations appear in the form.

lib/primer/forms/primer_text_field.ts

camertron · 2022-12-06T18:16:54Z

@lesliecdubs good questions! There's no tracking issue to my knowledge. This work came from one of the settings page forms @neall's been working on. It so happens I also need it for the billing upgrade form.

Curious if we involved Primer accessibility design in the decisions about how the client-side validations appear in the form.

We have not explicitly sought their input, however the markup and behavior adheres to the existing form interface guidelines. I believe they have been a11y reviewed, although I may be wrong about that.

In any case, we do need to have someone from a11y look at this. We probably need to announce the validation message somehow.

camertron

🎉

This sets up a place to test form text fields that will automatically validate against the server. It makes tests around the existing text field functionality without the auto-server-validation. I pulled out the DeepMind fake model from an existing test file in order to share it with this new test file.

This will allow us to validate a text field against the server with JavaScript using custom elements.

When we show the validation elements, only include the data-target attributes when we need them.

We decided that `text-field` was a bit too generic a name for the global custom element namespace on a page.

Prevent PrimerTextField event handlers from hanging around in weird cases. Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

We were only deploying lib assets under `lib/postcss_mixins`, but now we have JavaScript coming from the lib directory as well.

The underlying functionality is tested somewhere else, and these example forms are not used in the built gem.

Terser seems to be eliminating debugger statements during minification and we couldn't figure out why. For now, if you want to use the debugger in development you can comment out the line that adds the Terser plugin to Rollup.

neall requested review from a team and keithamus December 5, 2022 21:34

neall had a problem deploying to review-pr-1672 December 5, 2022 21:35 Failure

github-actions bot added the javascript Pull requests that update Javascript code label Dec 5, 2022

neall temporarily deployed to github-pages December 5, 2022 21:39 Inactive

camertron requested changes Dec 5, 2022

View reviewed changes

test/lib/primer/forms/deep_thought.rb Outdated Show resolved Hide resolved

test/lib/primer/forms/forms_test.rb Outdated Show resolved Hide resolved

camertron temporarily deployed to review-pr-1672 December 6, 2022 00:01 Inactive

camertron temporarily deployed to github-pages December 6, 2022 00:05 Inactive

keithamus reviewed Dec 6, 2022

View reviewed changes

lib/primer/forms/primer_text_field.ts Outdated Show resolved Hide resolved

keithamus reviewed Dec 6, 2022

View reviewed changes

lib/primer/forms/primer_text_field.ts Outdated Show resolved Hide resolved

neall force-pushed the neall/forms-immediate-server-side-validation branch from 53228a3 to 683c2b4 Compare December 6, 2022 21:14

neall had a problem deploying to review-pr-1672 December 6, 2022 21:14 Error

neall force-pushed the neall/forms-immediate-server-side-validation branch from f9431ec to 21c6f3c Compare December 6, 2022 21:19

neall had a problem deploying to review-pr-1672 December 6, 2022 21:19 Failure

neall force-pushed the neall/forms-immediate-server-side-validation branch from 81efead to 025d7c0 Compare December 7, 2022 14:00

neall had a problem deploying to review-pr-1672 December 7, 2022 14:00 — with GitHub Actions Failure

neall temporarily deployed to github-pages December 7, 2022 14:06 — with GitHub Actions Inactive

neall force-pushed the neall/forms-immediate-server-side-validation branch from 05b5421 to 5bad56d Compare December 7, 2022 14:14

neall had a problem deploying to review-pr-1672 December 7, 2022 14:14 — with GitHub Actions Failure

neall temporarily deployed to github-pages December 7, 2022 14:18 — with GitHub Actions Inactive

neall force-pushed the neall/forms-immediate-server-side-validation branch from 44ee351 to e3a9f01 Compare December 7, 2022 14:23

neall had a problem deploying to review-pr-1672 December 7, 2022 14:23 — with GitHub Actions Failure

neall temporarily deployed to github-pages December 7, 2022 14:27 — with GitHub Actions Inactive

neall had a problem deploying to review-pr-1672 December 7, 2022 15:15 — with GitHub Actions Failure

neall temporarily deployed to github-pages December 7, 2022 15:15 — with GitHub Actions Inactive

neall temporarily deployed to review-pr-1672 December 7, 2022 18:38 — with GitHub Actions Inactive

neall temporarily deployed to github-pages December 7, 2022 18:42 — with GitHub Actions Inactive

neall temporarily deployed to review-pr-1672 December 7, 2022 19:38 — with GitHub Actions Inactive

neall temporarily deployed to github-pages December 7, 2022 19:43 — with GitHub Actions Inactive

camertron approved these changes Dec 7, 2022

View reviewed changes

camertron requested a review from keithamus December 7, 2022 19:52

neall changed the title ~~Neall/forms immediate server side validation~~ forms immediate server side validation Dec 7, 2022

neall and others added 13 commits December 8, 2022 10:21

Add JavaScript to form text_fields

e7570ba

This will allow us to validate a text field against the server with JavaScript using custom elements.

Only have text-field data attributes when needed

3893bb0

When we show the validation elements, only include the data-target attributes when we need them.

Include forms js in bundle

6875def

Include correct authenticity token in auto-check requests

e0c11d7

Watch lib/primer/forms for ts changes; ensure components are full-width

cabfedc

Generating component snapshots

e9dd173

Rename from <text-field> to <primer-text-field>

a75bba1

We decided that `text-field` was a bit too generic a name for the global custom element namespace on a page.

Add AbortController to primer-text-field

740b46a

Prevent PrimerTextField event handlers from hanging around in weird cases. Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

describe branch in changeset message

e3e6976

Deploy assets from the entire demo lib dir

6b0c5e9

We were only deploying lib assets under `lib/postcss_mixins`, but now we have JavaScript coming from the lib directory as well.

Exclude example form from coverage requirements

d5466b7

The underlying functionality is tested somewhere else, and these example forms are not used in the built gem.

Put a note in rollup config about debuggers

33acbe2

Terser seems to be eliminating debugger statements during minification and we couldn't figure out why. For now, if you want to use the debugger in development you can comment out the line that adds the Terser plugin to Rollup.

neall force-pushed the neall/forms-immediate-server-side-validation branch from c352bf6 to 33acbe2 Compare December 8, 2022 15:23

neall temporarily deployed to review-pr-1672 December 8, 2022 15:23 — with GitHub Actions Inactive

neall temporarily deployed to github-pages December 8, 2022 15:27 — with GitHub Actions Inactive

camertron added 2 commits December 8, 2022 12:01

Fix routes

80c0227

Merging upstream changes

3fd3217

camertron deployed to review-pr-1672 December 8, 2022 20:06 — with GitHub Actions View deployment

camertron temporarily deployed to github-pages December 8, 2022 20:11 — with GitHub Actions Inactive

camertron merged commit 1a7dadd into main Dec 8, 2022

camertron deleted the neall/forms-immediate-server-side-validation branch December 8, 2022 21:25

primer-css mentioned this pull request Dec 8, 2022

Release Tracking #1686

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

forms immediate server side validation #1672

forms immediate server side validation #1672

neall commented Dec 5, 2022

changeset-bot bot commented Dec 5, 2022 •

edited

Loading

github-actions bot commented Dec 5, 2022 •

edited

Loading

camertron left a comment

lesliecdubs commented Dec 6, 2022

camertron commented Dec 6, 2022

camertron left a comment

forms immediate server side validation #1672

forms immediate server side validation #1672

Conversation

neall commented Dec 5, 2022

Description

Integration

Merge checklist

changeset-bot bot commented Dec 5, 2022 • edited Loading

🦋 Changeset detected

github-actions bot commented Dec 5, 2022 • edited Loading

⚠️ Visual differences found

camertron left a comment

Choose a reason for hiding this comment

lesliecdubs commented Dec 6, 2022

camertron commented Dec 6, 2022

camertron left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Dec 5, 2022 •

edited

Loading

github-actions bot commented Dec 5, 2022 •

edited

Loading