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

refactor(react): remove default placeholder from Combobox, TextArea, TextInput, MultiSelect, FilterableMultiselect #9510

Merged
merged 11 commits into from
Jan 27, 2022
Merged

refactor(react): remove default placeholder from Combobox, TextArea, TextInput, MultiSelect, FilterableMultiselect #9510

merged 11 commits into from
Jan 27, 2022

Conversation

dakahn
Copy link
Contributor

@dakahn dakahn commented Aug 20, 2021 •
edited
Loading

partially closes #5022

Changes

  • Removes default placeholder for
  1. Combobox
  2. TextArea
  3. TextInput
  4. MultiSelect
  5. FilterableMultiselect

Notes

Three inputs with a default placeholder were excluded from this PR (this PR now only partially closes 5022) because the removing default placeholder text without further design consideration would have resulted in worse usability potentially. These components are:

  • Search
  • Datepicker
  • Timepicker

Any other instances of Placeholder in the codebase were not required or used in example storybook stories as far as i could tell 👍🏾

@dakahn dakahn requested a review from a team as a code owner August 20, 2021 00:20
@dakahn dakahn requested review from sstrubberg and jnm2377 August 20, 2021 00:20
@netlify
Copy link

netlify bot commented Aug 20, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-react-next ready!

🔨 Explore the source changes: d62e8ac

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-react-next/deploys/61f2ff7399584300085fbcc8

😎 Browse the preview: https://deploy-preview-9510--carbon-react-next.netlify.app

@netlify
Copy link

netlify bot commented Aug 20, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: d62e8ac

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/61f2ff734dbbde00083cbbf9

😎 Browse the preview: https://deploy-preview-9510--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Aug 20, 2021 •
edited
Loading

✔️ Deploy Preview for carbon-components-react ready!

🔨 Explore the source changes: d62e8ac

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/61f2ff73b4daf400073c004b

😎 Browse the preview: https://deploy-preview-9510--carbon-components-react.netlify.app/

sstrubberg
sstrubberg approved these changes Aug 20, 2021
View reviewed changes
Copy link
Member

@sstrubberg sstrubberg left a comment

Choose a reason for hiding this comment

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

LGTM!

@dakahn dakahn requested a review from a team as a code owner August 31, 2021 15:46
joshblack
joshblack suggested changes Sep 8, 2021
View reviewed changes
Copy link
Contributor

@joshblack joshblack left a comment •
edited by dakahn
Loading

Choose a reason for hiding this comment

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

Looking great! I think in this PR we saw: ComboBox, TextArea, and TextInput covered. Was curious if we were also going to cover:

  • TimePicker
  • ControlledPasswordInput
  • PasswordInput
  • Search
  • FilterableMultiSelect
  • DatePickerInput
  • TableToolbarSearch

Which I think also have a placeholder prop: https://carbon-react-explorer.vercel.app/props/placeholder

I think there are several with placeholderText too that I wasn't sure if they applied either but wanted to confirm: https://carbon-react-explorer.vercel.app/props/placeHolderText

packages/react/src/components/TextInput/TextInput.js Outdated Show resolved Hide resolved
@joshblack joshblack mentioned this pull request Jan 4, 2022
Closed
46 tasks
@dakahn dakahn requested a review from joshblack January 27, 2022 00:40
@dakahn dakahn changed the title 5022 remove default placeholder 5022 remove default placeholder from Combobox, TextArea, TextInput, MultiSelect, FilterableMultiselect Jan 27, 2022
@joshblack joshblack changed the title 5022 remove default placeholder from Combobox, TextArea, TextInput, MultiSelect, FilterableMultiselect refactor(react): remove default placeholder from Combobox, TextArea, TextInput, MultiSelect, FilterableMultiselect Jan 27, 2022
@joshblack joshblack requested review from a team and johnbister and removed request for a team January 27, 2022 17:20
aagonzales
aagonzales approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member

@aagonzales aagonzales left a comment

Choose a reason for hiding this comment

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

LGTM!

@dakahn dakahn enabled auto-merge (squash) January 27, 2022 20:24
@dakahn dakahn merged commit df28984 into carbon-design-system:main Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnm2377 jnm2377 jnm2377 approved these changes

@sstrubberg sstrubberg sstrubberg approved these changes

@aagonzales aagonzales aagonzales approved these changes

@joshblack joshblack joshblack approved these changes

@tay1orjones tay1orjones tay1orjones approved these changes

@johnbister johnbister Awaiting requested review from johnbister johnbister was automatically assigned from carbon-design-system/design

Assignees

@dakahn dakahn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Removing placeholder text from Carbon Design System
6 participants
@dakahn @tay1orjones @joshblack @aagonzales @sstrubberg @jnm2377