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

Make multi input a form control #1667

Merged
merged 26 commits into from
Dec 9, 2022
Merged

Make multi input a form control #1667

merged 26 commits into from
Dec 9, 2022

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Dec 2, 2022

Description

The multi input in the forms framework is designed to wrap multiple inputs with the same name, only one of which may be visible at once. Why would anyone need such a thing, you ask? Imagine a form with a country dropdown. When the user selects "United States," the region input should show a list of US states. When the user selects "Canada," the region input should show a list of Canadian provinces. If any other country is selected, region becomes a free-form text field.

Currently, the multi input does not wrap its constituent inputs in FormControl - the inputs are expected to already be wrapped in FormControl. However I believe the label and validation message, i.e. the additional markup added by FormControl, should appear in the DOM only once. They should wrap the three inputs (US states, Canadian provinces, free-form text). Javascript handles showing and hiding the actual <select> and <input> elements.

To that end, this PR accomplishes the following:

  1. Adds a form_control boolean attribute to all inputs that controls whether or not FormControl should wrap the input. If form_control: false is passed, only the input itself is rendered.
  2. Modifies the multi input to wrap all its children in a single FormControl, and passes form_control: false when creating them.
  3. Adds the primer-multi-input custom element that provides an activateField method for switching between constituent fields.

Integration

Does this change require any updates to code in production?

No. Nothing is using this field in production.

Merge checklist

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

@changeset-bot
Copy link

changeset-bot bot commented Dec 2, 2022

🦋 Changeset detected

Latest commit: 5a4a3f6

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

@camertron camertron temporarily deployed to review-pr-1667 December 2, 2022 22:24 Inactive
@camertron camertron temporarily deployed to github-pages December 2, 2022 22:28 Inactive
@camertron camertron temporarily deployed to review-pr-1667 December 2, 2022 22:47 Inactive
@camertron camertron temporarily deployed to github-pages December 2, 2022 22:52 Inactive
@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Dec 5, 2022
@camertron camertron temporarily deployed to github-pages December 5, 2022 22:23 Inactive
@camertron camertron temporarily deployed to review-pr-1667 December 5, 2022 23:54 Inactive
@camertron camertron temporarily deployed to github-pages December 5, 2022 23:58 Inactive
@camertron camertron marked this pull request as ready for review December 6, 2022 00:01
@camertron camertron requested review from a team and keithamus and removed request for keithamus December 6, 2022 00:01
@camertron camertron temporarily deployed to github-pages December 6, 2022 19:29 Inactive
@camertron camertron temporarily deployed to review-pr-1667 December 6, 2022 19:40 Inactive
@camertron camertron temporarily deployed to review-pr-1667 December 6, 2022 19:57 Inactive
@camertron camertron temporarily deployed to review-pr-1667 December 7, 2022 18:38 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages December 7, 2022 18:43 — with GitHub Actions Inactive
@camertron camertron temporarily deployed to github-pages December 8, 2022 23:58 — with GitHub Actions Inactive
@camertron camertron merged commit ee7f476 into main Dec 9, 2022
@camertron camertron deleted the fix_multi_input branch December 9, 2022 00:19
@primer-css primer-css mentioned this pull request Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants