artsy / palette

Artsy's design system
https://palette-storybook.artsy.net/
MIT License
214 stars 45 forks source link

required prop for Input doesn't actually set required on the input #709

Open dzucconi opened 4 years ago

dzucconi commented 4 years ago

Passing <Input required /> visually denotes it as being required — I'd expect it to actually pass the prop down to the underlying input, which it doesn't

Just leaving myself this note here.

iskounen commented 4 years ago

Hi. I had the same expectation, but am currently working on a form where this behavior makes sense. The form in question can be saved as a draft or published, and being able to mark its fields as required without setting it on the input has turned out to be useful.

@oxaudo suggested adding a separate visuallyRequired prop for the use case that I described since there may be several uses of the required prop that depend on it not being passed down to the underlying input.

dzucconi commented 4 years ago

@iskounen To me it just sounds like this component is semantically confusing. It's not just an input, it's a field complete with label. To me that's something else entirely. Inputs on their own should be unsurprising and accept the complete range of native props.

dzucconi commented 4 years ago

Of course, I think this is a problem in terms of how to move forward 🤔

iskounen commented 4 years ago

I see where you're coming from, and I agree.

I hadn't considered your point about the label before, but it would be useful to have that be separate like it is in Bootstrap.