Skip to content

Let FormControl accept any input - #1968

Merged
mperrotti merged 10 commits into
mainfrom
mp/form-control-any-input
Mar 16, 2022
Merged

Let FormControl accept any input#1968
mperrotti merged 10 commits into
mainfrom
mp/form-control-any-input

Conversation

@mperrotti

@mperrottimperrotti commented Mar 16, 2022

Copy link
Copy Markdown
Contributor

Instead of rendering unexpected* FormControl children before the other children, we render them in the same spot we'd normally render an input.

* any component that is not a Primer input component or not in the FormControl namespace

Closes#1964

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

@mperrotti
mperrotti requested review from a team and pksjceMarch 16, 2022 14:12
@changeset-bot

changeset-botBot commented Mar 16, 2022•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: aa500e9

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

This PR includes changesets to release 1 package
NameType
@primer/reactPatch

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

github-actionsBot commented Mar 16, 2022•
edited
Loading

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
dist/browser.esm.js63.19 KB (-0.34% 🔽)
dist/browser.umd.js63.54 KB (-0.34% 🔽)

dusave
dusave approved these changes Mar 16, 2022
View reviewed changes

@dusavedusave left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👏

@siddharthkp
siddharthkp self-requested a review March 16, 2022 14:28
siddharthkp
siddharthkp approved these changes Mar 16, 2022
View reviewed changes

@siddharthkpsiddharthkp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor changes requested in docs, the code looks good!

Approving in advance :)

Comment threaddocs/content/FormControl.mdx
Comment threaddocs/content/FormControl.mdx
Comment threaddocs/content/FormControl.mdx
Comment threaddocs/content/FormControl.mdx Outdated
Comment threaddocs/content/FormControl.mdx Outdated
Comment threadsrc/FormControl/_FormControlCaption.tsx
mperrotti
mperrotti commented Mar 16, 2022
View reviewed changes
Comment threaddocs/content/FormControl.mdx
Comment threaddocs/content/FormControl.mdx Outdated
Comment threadsrc/FormControl/FormControl.tsx
flexDirection="column"
width="100%"
sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 2}} : {'> * + *': {marginTop: 2}}), ...sx}}
sx={{...(isLabelHidden ? {'> *:not(label) + *': {marginTop: 1}} : {'> * + *': {marginTop: 1}}), ...sx}}

@mperrottimperrottiMar 16, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This has nothing to do with accepting different input components.
I looked at this with fresh eyes and thought the elements were spaced too far apart.

@mperrotti
mperrotti merged commit 1b01485 into mainMar 16, 2022
@mperrotti
mperrotti deleted the mp/form-control-any-input branch March 16, 2022 20:40
@primer-cssprimer-css mentioned this pull request Mar 16, 2022
Merged
pksjce pushed a commit that referenced this pull request Mar 17, 2022
@mperrotti@siddharthkp@pksjce
Let FormControl accept any input (#1968)
20f09b5
* renders any non-FormControl components passed into FormControl in the spot we'd normally render a Primer input component
* adds changeset
* appease the linter
* Update docs/content/FormControl.mdx
Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
* addresses PR feedback
Co-authored-by: Siddharth Kshetrapal <siddharthkp@github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siddharthkpsiddharthkpsiddharthkp approved these changes
@pksjcepksjceAwaiting requested review from pksjce
+1 more reviewer
@dusavedusavedusave approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@siddharthkpsiddharthkp

@mperrottimperrotti

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

FormControl does not support custom components

3 participants

@mperrotti@siddharthkp@dusave