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.

Sign up for GitHub

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

Jump to bottom

feat(text-input): add height variations to text input #5076

Merged
merged 8 commits into from
Jan 21, 2020
Merged

feat(text-input): add height variations to text input #5076

merged 8 commits into from
Jan 21, 2020

Conversation

tw15egan
Copy link
Collaborator

@tw15egan tw15egan commented Jan 16, 2020 •
edited
Loading

Closes #5071

Adds height variations to TextInput component.

  • small
  • `` (default)
  • large

Changelog

New

  • New size prop on TextInput, with a default of ``
  • .#{$prefix}--text-input--large and .#{$prefix}--text-input--small classes
  • Size knob in storybook

Testing / Reviewing

Change the size of the TextInput via the knob in the storybook

@tw15egan tw15egan requested a review from a team as a code owner January 16, 2020 22:37
@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for the-carbon-components ready!

Built with commit 673a8a7

https://deploy-preview-5076--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for carbon-components-react ready!

Built with commit 023b69c

https://deploy-preview-5076--carbon-components-react.netlify.com

asudoh
asudoh approved these changes Jan 16, 2020
View reviewed changes
Copy link
Contributor

@asudoh asudoh left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - Thanks @tw15egan for your quick change!

@asudoh asudoh requested review from a team and jeanservaas and removed request for a team January 16, 2020 22:56
@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for carbon-elements ready!

Built with commit 673a8a7

https://deploy-preview-5076--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Jan 16, 2020 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit c1fe65a

https://deploy-preview-5076--carbon-elements.netlify.com

@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for the-carbon-components ready!

Built with commit 85f1205

https://deploy-preview-5076--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for carbon-components-react failed.

Built with commit 85f1205

https://app.netlify.com/sites/carbon-components-react/deploys/5e20ea96a2c2ac0008052e94

@netlify
Copy link

netlify bot commented Jan 16, 2020 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit c1fe65a

https://deploy-preview-5076--the-carbon-components.netlify.com

@netlify
Copy link

netlify bot commented Jan 16, 2020

Deploy preview for carbon-components-react failed.

Built with commit 69f9508

https://app.netlify.com/sites/carbon-components-react/deploys/5e20eef85a8aca00094e0251

@netlify
Copy link

netlify bot commented Jan 17, 2020 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit c1fe65a

https://deploy-preview-5076--carbon-components-react.netlify.com

emyarod
emyarod approved these changes Jan 17, 2020
View reviewed changes
Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me, should these variants exist for the password input as well?

@tw15egan
Copy link
Collaborator Author

tw15egan commented Jan 17, 2020 •
edited
Loading

@emyarod added in support for the password fields as well

jeanservaas
jeanservaas approved these changes Jan 17, 2020
View reviewed changes
Copy link
Contributor

@jeanservaas jeanservaas left a comment

Choose a reason for hiding this comment

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

Hey @asudoh this change looks good and I will approve.

but @laurenmrice and i were wondering what the effort level would be to change the names of these field height sizes... "Extra large" "Large" and "Small" does not really make sense in the scheme of our system. "Large" "Default" and "Small" or something like that would make more sense...

also we noticed that other components like Data table are using a different naming convention for field heights: "tall" "short" and "compact." It would be nice to streamline all of this stuff.

@tw15egan
Copy link
Collaborator Author

tw15egan commented Jan 17, 2020 •
edited
Loading

@jeanservaas I can change these to be small and large, with the default being default before we merge this in. The dropdown updates can be made in a separate PR

@asudoh asudoh merged commit 7011d0b into carbon-design-system:master Jan 21, 2020
@joshblack joshblack mentioned this pull request Jan 31, 2020
Closed
@tw15egan tw15egan mentioned this pull request Feb 11, 2020
Merged
@tw15egan tw15egan mentioned this pull request Feb 18, 2020
Closed
This was referenced Feb 25, 2020
Closed
Closed
@aagonzales aagonzales mentioned this pull request Feb 27, 2020
Closed
24 tasks
@tw15egan tw15egan deleted the text-input-height-variations branch April 28, 2021 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeanservaas jeanservaas jeanservaas approved these changes

@emyarod emyarod emyarod approved these changes

@asudoh asudoh asudoh approved these changes

@joshblack joshblack Awaiting requested review from joshblack

Assignees
No one assigned
Labels
status: ready to merge 🎉
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[text input] add 48px and 32px size options
4 participants
@tw15egan @asudoh @emyarod @jeanservaas