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

Favor explicit usage of <FormItem> component #2549

Closed
emyarod opened this issue Dec 21, 2018 · 7 comments · May be fixed by softagram/carbon-components-react#2
Closed

Favor explicit usage of <FormItem> component #2549

emyarod opened this issue Dec 21, 2018 · 7 comments · May be fixed by softagram/carbon-components-react#2
Assignees
Labels

Comments

@emyarod
Copy link
Member

emyarod commented Dec 21, 2018

Currently many form item components contain a wrapper div within them with the .bx--form-item class. When composing forms, these wrapper divs are often duplicated or erroneously added, causing components to diverge from the spec. Examples of this can be seen in the current <NumberInput>, <ComboBox>, , <FilterableMultiSelect>, and <Slider> stories

@stale
Copy link

stale bot commented May 1, 2019

We've marked this issue as stale because there hasn't been any activity for a couple of weeks. If there's no further activity on this issue in the next three days then we'll close it. Thanks for your contributions.

@emyarod
Copy link
Member Author

emyarod commented May 2, 2019

stalebot pls

@carbon-bot carbon-bot transferred this issue from carbon-design-system/carbon-components-react May 9, 2019
@carbon-bot
Copy link
Contributor

Hi there! 👋 If you're wondering why this issue was moved, we're currently updating our repo structure so that every package is found in the same project.

This should not have any impact for you, but we wanted to give you a heads up in case you were wondering what is going on. If you have any questions, feel free to reach out to us on Slack or contact us at: carbon@us.ibm.com. Thanks!

@vpicone vpicone added the version: 11 Issues pertaining to Carbon v11 label Nov 5, 2019
@vpicone
Copy link
Contributor

vpicone commented Nov 5, 2019

I'm not sure forcing devs to wrap all of our form inputs with a FormItem wrapper is a great idea it gets really gnarley for complex forms. Using the FormItem to combine label logic though could be a good move.

As of now, the FormItem component doesn't really serve a purpose as a public since it's baked into our other form inputs. I wonder if we should just delete the story?

@emyarod
Copy link
Member Author

emyarod commented Nov 8, 2019

I'm unsure about the origins of exposing this component as a public and the discussion around the original PR is now missing because of the monorepo move. but to me it seemed like it was a way to enforce spacing between form elements. right now we already wrap our form inputs in .bx--form-item so this wrapper component doesn't have much use

IIRC the reason the idea of explicitly requiring FormItem when building forms came up is so that we wouldn't have to constantly override the default form spacing/margin in components (like Slider, Pagination which consume other components like NumberInput, Select)

@tw15egan
Copy link
Collaborator

@emyarod is there any action we want to take here or do we want to close this out?

@emyarod
Copy link
Member Author

emyarod commented Jan 28, 2021

this can be closed since the discussion went nowhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment