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

chore(Toolbar/ToobarSearch): add component name to deprecation warning #4229

Merged
merged 3 commits into from
Oct 4, 2019
Merged

chore(Toolbar/ToobarSearch): add component name to deprecation warning #4229

merged 3 commits into from
Oct 4, 2019

Conversation

dakahn
Copy link
Contributor

@dakahn dakahn commented Oct 4, 2019

We didn't have the component name in our deprecation warning. 🤦‍♂

This also adds a deprecation warning to the Story. 👍

@dakahn dakahn requested review from joshblack and a team October 4, 2019 18:11
@ghost ghost requested review from jnm2377 and removed request for a team October 4, 2019 18:11
@netlify
Copy link

netlify bot commented Oct 4, 2019

Deploy preview for carbon-elements ready!

Built with commit de06fc0

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

@netlify
Copy link

netlify bot commented Oct 4, 2019

Deploy preview for the-carbon-components ready!

Built with commit de06fc0

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

@netlify
Copy link

netlify bot commented Oct 4, 2019

Deploy preview for carbon-components-react ready!

Built with commit de06fc0

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

@joshblack joshblack requested a review from a team October 4, 2019 18:48
@ghost ghost requested review from aagonzales and removed request for a team October 4, 2019 18:48
@joshblack
Copy link
Contributor

@aagonzales for context, I was wondering if this component was supposed to be in our v10 release, or not. Specifically: http://react.carbondesignsystem.com/?path=/story/toolbar--default

This PR is to address deprecating it as we assumed it wasn't supposed to be in this release.

@joshblack
Copy link
Contributor

@dakahn should we also list this component as deprecated in our storybook?

@joshblack
Copy link
Contributor

@aagonzales confirmed!

Copy link
Contributor

@joshblack joshblack left a comment

Choose a reason for hiding this comment

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

Could we also update the story to include [Deprecated] Toolbar?

@dakahn dakahn requested a review from a team as a code owner October 4, 2019 20:28
@ghost ghost requested review from aledavila and joshblack October 4, 2019 20:28
@joshblack
Copy link
Contributor

fyi @tay1orjones

@dakahn
Copy link
Contributor Author

dakahn commented Oct 4, 2019

For reference later -- we first attempted to use the built in Storybook decorators in the title of Toolbar story like
storiesOf('Deprecated|Toolbar', module).add(...

But that stuck all the other components into an "Other" category which is weird and not really what we want here. If we ever have multiple deprecated components we could think about prepending each components story like Components|ComponentName or sumthin 👍

@joshblack joshblack merged commit dd2c135 into carbon-design-system:master Oct 4, 2019
AlexanderLyon pushed a commit to AlexanderLyon/carbon that referenced this pull request Oct 8, 2019
…arbon-design-system#4229)

* chore(Toolbar/ToobarSearch): add component name to deprecation warning

* docs(Toolbar): add deprecation decorator to Toolbar story

* docs(Toolbar): un-add deprecation decorator
joshblack pushed a commit that referenced this pull request Oct 9, 2019
* feat(search): add styles for 40px search input

* chore(react): add component name to deprecation warning for toolbar (#4229)

* chore(Toolbar/ToobarSearch): add component name to deprecation warning

* docs(Toolbar): add deprecation decorator to Toolbar story

* docs(Toolbar): un-add deprecation decorator

* feat(search): add medium configuration

* feat(search): add medium description to README

* feat(search): add clear icon for medium size
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants