Create `FormControl` component #287

justinanastos · 2020-12-10T17:47:52Z

Create a new modular FormControl component.

Reviewing

I don't really need a review here. I'm super open to any and all feedback; but it's more important to me to get this in people's hands ASAP. I'm going to merge this pretty quickly to help people start using it.

Release Notes

`FormControl`

There is a new component to handle forms; FormControl. It's a layout component that will automatically layout it's children.

New components include: FormLabel, FormDescription, FormInput, FormHelperText, FormStartAdornment, ForEndAdornment, and FormErrorMessage. All of these components can be rendered anywhere on their own; but if they are rendered inside of a FormControl then they will automatically communicate with FormControl to add accessibility props and lay them out correctly.

There is a new hook, useFormControlContext intended to be used by any user-component that needs access to information passed by FormControl. Input and Select both consume useFormControlContext, for example.

`Select`

Backtrack and rename initialValue prop to defaultValue to be inline with React's built-ins
Add listAs prop
Use FormControlContext to accept error state

`Input`

New component to apply custom styles to <input />.

📦 Published PR as canary version: 8.7.3-canary.287.7275.0

✨ Test out this PR locally via:

npm install @apollo/space-kit@8.7.3-canary.287.7275.0
# or 
yarn add @apollo/space-kit@8.7.3-canary.287.7275.0

jglovier

Awesome!! Just one one request about the info icon...

src/FormHelperText/index.tsx

jglovier

Gah, I'm sorry, I forgot to specify they should be text-grey instead of text-blue. 🙈

Approving anyway so feel free to merge with that tweak. 🙇

cheapsteak · 2020-12-15T03:12:21Z

src/FormControl/index.tsx

+                  {startAdornment && (
+                    <div
+                      css={{
+                        position: "absolute",
+                        display: "inline-flex",
+                        left: 12,
+                        top: "50%",
+                        transform: "translateY(-50%)",
+                      }}
+                    >
+                      {startAdornment}
+                    </div>
+                  )}


(Please do merge at will / don't let this slow down delivery)

The styles around these wrappers for adornments don't look like they can be easily/safely overridden

Could FormStartAdornment and FormEndAdornment export styled divs that carried these styles by default?

Yeah you're right that the adornment layout can't be overridden here. That's kind of unintentional and intentional at the same time. I wanted the components themselves to not have any layout styles in them; just what's needed to render them in isolation and then FormControl would handle the layout. I provided escape hatches for some of the layout, but obviously not this part.

I think this will need to be re-worked, especially where we don't even make an attempt to measure the size of the adornments to pad the Input and they just overlap the Select.

I want to come back to this when we have a use case for it if that's cool.

Yup of course, merge at will

If you don't mind, I am curious to fill out my mental model so I can better know what kind of feedback is useful to provide -- is the motivation for punting on making this portion of the component open for extension due to a desire to defer coding time? (My mental model might be missing some pieces to better weigh the tradeoffs of implementing this now)

From my perspective of consuming components, since making changes for external dependencies is not quite frictionless, if/when a use case for this arises, I would prefer if the convenient thing to do also happened to be the right thing to do.

If these adornment components' styles are not open to extension/override, then the convenient thing for a consumer to do becomes to wrap itself with negative margins to work around not having access to the wrapper, which is more convenient from a time-and-effort perspective, but the resulting code would feel less than great

I would be curious to understand what costs or risks that ^ is trading off against

@cheapsteak I was thinking in terms of "what's the use case and how should we prepare for that?" Going with a straightforward approach of just applying these css properties in FormStartAdornment and FormEndAdornment and allowing a className override seems fair and frictionless so I added it; thanks for the pushback!

src/FormStartAdornment/index.tsx

justinanastos · 2020-12-15T15:56:55Z

Gah, I'm sorry, I forgot to specify they should be text-grey instead of text-blue. 🙈

Approving anyway so feel free to merge with that tweak. 🙇

@jglovier grey it is!

jglovier · 2020-12-15T17:05:58Z

…ering

apollo-bot2 · 2021-01-05T22:47:19Z

🚀 PR was released in v8.8.0 🚀

justinanastos force-pushed the justin/form-element branch 3 times, most recently from a6118af to 58a056e Compare December 10, 2020 19:53

justinanastos added the minor Increment the minor version when merged label Dec 10, 2020

justinanastos force-pushed the justin/form-element branch 2 times, most recently from 101e41d to 7832574 Compare December 10, 2020 20:15

justinanastos changed the title ~~Create FormElement component~~ Create FormControl component Dec 10, 2020

justinanastos force-pushed the justin/form-element branch 2 times, most recently from 4b24f0a to 7ebc0fa Compare December 14, 2020 20:20

justinanastos requested review from Jephuff, jgzuke, timbotnik, cheapsteak and AlexanderMann December 14, 2020 20:28

justinanastos marked this pull request as ready for review December 14, 2020 20:29

justinanastos force-pushed the justin/form-element branch 2 times, most recently from 2407a0a to 014a8d9 Compare December 14, 2020 20:39

jglovier reviewed Dec 14, 2020

View reviewed changes

src/FormHelperText/index.tsx Outdated Show resolved Hide resolved

src/FormHelperText/index.tsx Outdated Show resolved Hide resolved

justinanastos force-pushed the justin/form-element branch 2 times, most recently from 9e2a365 to 08d99c6 Compare December 14, 2020 21:20

justinanastos requested a review from jglovier December 14, 2020 21:23

justinanastos force-pushed the justin/form-element branch from 08d99c6 to e8faf4d Compare December 14, 2020 21:23

jglovier approved these changes Dec 15, 2020

View reviewed changes

cheapsteak reviewed Dec 15, 2020

View reviewed changes

src/FormStartAdornment/index.tsx Outdated Show resolved Hide resolved

justinanastos force-pushed the justin/form-element branch from e8faf4d to ba3ec29 Compare December 15, 2020 15:57

justinanastos force-pushed the justin/form-element branch 2 times, most recently from c1cb30f to a1d9f64 Compare December 15, 2020 20:00

justinanastos force-pushed the justin/form-element branch 4 times, most recently from 0f1ea1b to 6b84ce4 Compare December 16, 2020 16:45

justinanastos added 7 commits January 5, 2021 12:55

feat(Select): add renderTriggerNode prop for custom select value rend…

c3ad0dc

…ering

feat(Select): Add listAs prop

8188a7b

add deprecation warning to TextField

5d90036

add testing-library/react-hooks and upgrade testling-library

f8dd0fd

Create FormControl and Input components

4724cf8

Update Select for FormControl

7e28a09

Create Form file to export all form components

bfd93d0

justinanastos force-pushed the justin/form-element branch from 6b84ce4 to bfd93d0 Compare January 5, 2021 17:56

justinanastos merged commit 4de9a45 into main Jan 5, 2021

justinanastos deleted the justin/form-element branch January 5, 2021 22:42

apollo-bot2 added the released This issue/pull request has been released. label Jan 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create `FormControl` component #287

Create `FormControl` component #287

justinanastos commented Dec 10, 2020 •

edited by apollo-bot2

Loading

jglovier left a comment

jglovier left a comment

cheapsteak Dec 15, 2020 •

edited

Loading

justinanastos Dec 15, 2020

cheapsteak Dec 15, 2020 •

edited

Loading

cheapsteak Dec 15, 2020 •

edited

Loading

justinanastos Dec 15, 2020

justinanastos commented Dec 15, 2020

jglovier commented Dec 15, 2020

apollo-bot2 commented Jan 5, 2021

Create FormControl component #287

Create FormControl component #287

Conversation

justinanastos commented Dec 10, 2020 • edited by apollo-bot2 Loading

Reviewing

Release Notes

FormControl

Select

Input

jglovier left a comment

Choose a reason for hiding this comment

jglovier left a comment

Choose a reason for hiding this comment

cheapsteak Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

justinanastos Dec 15, 2020

Choose a reason for hiding this comment

cheapsteak Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

cheapsteak Dec 15, 2020 • edited Loading

Choose a reason for hiding this comment

justinanastos Dec 15, 2020

Choose a reason for hiding this comment

justinanastos commented Dec 15, 2020

jglovier commented Dec 15, 2020

apollo-bot2 commented Jan 5, 2021

Create `FormControl` component #287

Create `FormControl` component #287

justinanastos commented Dec 10, 2020 •

edited by apollo-bot2

Loading

`FormControl`

`Select`

`Input`

cheapsteak Dec 15, 2020 •

edited

Loading

cheapsteak Dec 15, 2020 •

edited

Loading

cheapsteak Dec 15, 2020 •

edited

Loading