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

fix(search): add styles for custom search buttons #3247

Merged
merged 8 commits into from
Jul 2, 2019
Merged

fix(search): add styles for custom search buttons #3247

merged 8 commits into from
Jul 2, 2019

Conversation

jendowns
Copy link
Contributor

@jendowns jendowns commented Jul 1, 2019

Closes #2042

Adds some styles for custom Search buttons. Please let me know what you think 🤔 I just went with the look of the close button + what we have in our storybook, so I'm happy to adjust. 👍

The buttons basically look like this:
Screen Shot 2019-07-01 at 4 20 27 PM

Changelog

New

  • add styles for .bx--search-button

Changed

  • ended up rearranging .bx--search-close styles a little so it could share styles for .bx--search-button

Removed

  • extra/repeated focus style rule for .bx--search-close

@netlify
Copy link

netlify bot commented Jul 1, 2019

Deploy preview for the-carbon-components ready!

Built with commit a662e59

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

@netlify
Copy link

netlify bot commented Jul 1, 2019

Deploy preview for carbon-components-react ready!

Built with commit a662e59

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

@netlify
Copy link

netlify bot commented Jul 1, 2019

Deploy preview for carbon-elements ready!

Built with commit beeafed

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

@netlify
Copy link

netlify bot commented Jul 1, 2019

Deploy preview for carbon-elements ready!

Built with commit a662e59

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

@asudoh asudoh requested a review from a team July 1, 2019 22:36
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!

Copy link
Contributor

@shixiedesign shixiedesign left a comment

Choose a reason for hiding this comment

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

Hi Jen, style looks good! But just wondering if adding buttons is something we can achieve with Toolbar component? https://the-carbon-components.netlify.com/?nav=toolbar

@jendowns
Copy link
Contributor Author

jendowns commented Jul 2, 2019

Thanks @shixiedesign! I'm not familiar with that component 🤔 Looks like it is styled a little strangely when I interact with it?

Screen Shot 2019-07-02 at 10 02 00 AM

I was looking at this particular React variation of the Search here: http://react.carbondesignsystem.com/?path=/story/search--custom-buttons

Perhaps the styling could be consolidated but it seems like the Toolbar needs a little work first? 🤔 What do you think?

@shixiedesign
Copy link
Contributor

Ouch yes I'm doing another QA pass on the components so thanks for pointing that out about the tool. Will need to fix that. Thanks for the Search variant fix! I think we can merge since this is an update to existing component and think about consolidation / guidance later.

@jendowns
Copy link
Contributor Author

jendowns commented Jul 2, 2019

Thanks @shixiedesign! Just FYI I think the react version of the Tooltip has some interesting things going on as well -- for example it has a different (older styled?) close button when you enter text in the search field:

Screen Shot 2019-07-02 at 11 17 27 AM

See: http://react.carbondesignsystem.com/?path=/story/toolbar--default

@shixiedesign
Copy link
Contributor

Thanks @jendowns , I'm double checking with other designers and will make an issue for this component once I'm sure! Looks like it's lumped into data table at the moment so I'm not exactly sure where the styling went coz data table (last I checked) looked okay...

@joshblack joshblack requested a review from a team July 2, 2019 20:31
@ghost ghost requested review from aledavila and dakahn and removed request for a team July 2, 2019 20:31
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.

[Carbon X] Search button styles not present in search--x mixin
5 participants