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

fix(text-input): rename sizing props #5167

Merged
merged 4 commits into from
Jan 24, 2020
Merged

fix(text-input): rename sizing props #5167

merged 4 commits into from
Jan 24, 2020

Conversation

tw15egan
Copy link
Member

Matches Text Input size prop naming to Dropdown size prop naming

Screen Shot 2020-01-23 at 3 55 01 PM

Changelog

Changed

  • changes prop small to sm
  • changes prop large to xl

Testing / Reviewing

Change knob for size and ensure text input changes to correct size

@tw15egan tw15egan requested a review from a team as a code owner January 23, 2020 23:57
@ghost ghost requested review from abbeyhrt and aledavila January 23, 2020 23:57
asudoh
asudoh approved these changes Jan 23, 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!

@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for the-carbon-components ready!

Built with commit 21da02d

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

@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for carbon-elements ready!

Built with commit 21da02d

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

@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for carbon-components-react failed.

Built with commit 21da02d

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

This was referenced Jan 24, 2020
Closed
Merged
@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for the-carbon-components ready!

Built with commit 727f7ae

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

@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for carbon-components-react failed.

Built with commit 727f7ae

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

@netlify
Copy link

netlify bot commented Jan 24, 2020

Deploy preview for carbon-elements ready!

Built with commit 727f7ae

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

@netlify
Copy link

netlify bot commented Jan 24, 2020 •
edited
Loading

Deploy preview for the-carbon-components ready!

Built with commit 9650c85

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

@tw15egan tw15egan mentioned this pull request Jan 24, 2020
Closed
@netlify
Copy link

netlify bot commented Jan 24, 2020 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit 9650c85

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

@netlify
Copy link

netlify bot commented Jan 24, 2020 •
edited
Loading

Deploy preview for carbon-components-react failed.

Built with commit 9650c85

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

vpicone
vpicone suggested changes Jan 24, 2020
View reviewed changes
Copy link
Contributor

@vpicone vpicone left a comment

Choose a reason for hiding this comment

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

LGTM, just this one tweak. Thanks for hopping on this!

packages/react/src/components/TextInput/TextInput.js Outdated Show resolved Hide resolved
@vpicone
Copy link
Contributor

vpicone commented Jan 24, 2020 •
edited
Loading

It's wild to me that dropdown/listbox goes sm, default (large), xl. The naming from your original PR makes more sense, but definitely think it's better to be consistent for future fixes or updates.

@tw15egan @vpicone
Update packages/react/src/components/TextInput/TextInput.js
9650c85 
Co-Authored-By: Vince Picone <vpicone@gmail.com>
@tw15egan
Copy link
Member Author

@vpicone Makes sense, updated and should be good to go

@asudoh asudoh merged commit 8c577c5 into carbon-design-system:master Jan 24, 2020
@joshblack joshblack mentioned this pull request Jan 31, 2020
Closed
@joshblack joshblack mentioned this pull request Feb 25, 2020
Closed
@tw15egan tw15egan deleted the text-input-name-change 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

@asudoh asudoh asudoh approved these changes

@vpicone vpicone vpicone approved these changes

@abbeyhrt abbeyhrt Awaiting requested review from abbeyhrt

@aledavila aledavila Awaiting requested review from aledavila

Assignees
No one assigned
Labels
status: one more review 👀 status: ready for review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tw15egan @vpicone @asudoh