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

Contact Form: add date format to date picker #34743

Merged
merged 3 commits into from
Dec 22, 2023

Conversation

monsieur-z
Copy link
Contributor

@monsieur-z monsieur-z commented Dec 20, 2023

Fixes #34501

Proposed changes:

The jQuery UI Datepicker widget used by the Date Picker field of the Contact Form can't be used with a keyboard. This PR adds a date format to the field so keyboard users know how to fill out the form without opening the picker.

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

Project thread: pf5801-87-p2

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

No.

Testing instructions:

Prerequisites

  • Spin up a test site with this branch
  • If you run this locally, make sure to build the package: cd projects/packages/forms && npm run build

Editor

  • Create a new post
  • Add the Contact Form block
  • Add a Date Picker field
Screenshot 2023-12-20 at 4 50 52 PM
  • Notice the default format added in the label
Screenshot 2023-12-20 at 4 51 32 PM
  • Make sure the field is selected
  • Notice the Date Format setting in the sidebar
Screenshot 2023-12-20 at 4 52 22 PM
  • Update the value, save the draft, and refresh the page
  • Check the value was saved

Site

  • Open the preview
  • Check that the date field label mentions the date format
  • Enter an invalid date and notice the error message
Screenshot 2023-12-20 at 4 58 00 PM
  • Enter a valid date and verify there's no error message

@monsieur-z monsieur-z self-assigned this Dec 20, 2023
@monsieur-z monsieur-z requested a review from a team December 20, 2023 21:59
@monsieur-z monsieur-z added [Feature] Contact Form [Focus] Accessibility Improving usability for all users (a11y) [Status] In Progress [Block] Contact Form Form block (also see Contact Form label) labels Dec 20, 2023
Copy link
Contributor

github-actions bot commented Dec 20, 2023

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 add/contact-form-date-format branch.

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

    bin/jetpack-downloader test jetpack add/contact-form-date-format
    

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

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 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 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 reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

Copy link
Contributor

@fgiannar fgiannar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, Kevin!

I tested this with a JN site on latest stable, added a contact form with a date field and then switched to this branch to view/edit the existing form.

Upon viewing I got the following warning:
Warning: Undefined array key "formTitle" in [REDACTED]/wp-content/plugins/jetpack-dev/jetpack_vendor/automattic/jetpack-forms/src/contact-form/class-contact-form.php on line 357

Also, very minor but when editing the form the placeholder I had in place seemed to lose its padding:

Screenshot 2023-12-21 at 13 50 43

@fgiannar fgiannar added the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Dec 21, 2023
@monsieur-z
Copy link
Contributor Author

monsieur-z commented Dec 21, 2023

Thanks @fgiannar!

Upon viewing I got the following warning:

Good catch! I think this one was introduced by this PR: #34667. I'll create a new PR to suppress the warning.

Also, very minor but when editing the form the placeholder I had in place seemed to lose its padding:

Nice finding. I could reproduce the issue (with all text inputs), but not consistently. The padding is determined by reading the padding value of a "probe" component (see this line) and I suppose this style probing is sometimes done before all CSS is loaded.

That should probably be handled separately too.

@monsieur-z monsieur-z requested review from fgiannar and a team December 21, 2023 16:06
@monsieur-z monsieur-z removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Dec 21, 2023
Copy link
Contributor

@fgiannar fgiannar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested once more on both JN and Simple sites and apart from the original findings that will be addressed in follow-up PRs, everything works as expected 👍

@fgiannar fgiannar added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Team Review labels Dec 22, 2023
@monsieur-z monsieur-z merged commit dc54305 into trunk Dec 22, 2023
54 of 55 checks passed
@monsieur-z monsieur-z deleted the add/contact-form-date-format branch December 22, 2023 14:21
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Dec 22, 2023
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 [Focus] Accessibility Improving usability for all users (a11y) [Package] Forms [Tests] Includes Tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contact Form: datepicker is not accessible to keyboard users
2 participants