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(dropdown): ensure inline dropdown has transparent background #5322

Merged
merged 5 commits into from
Feb 18, 2020
Merged

fix(dropdown): ensure inline dropdown has transparent background #5322

merged 5 commits into from
Feb 18, 2020

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Feb 11, 2020 •
edited
Loading

Closes #5321

Sets the background of inline dropdowns to transparent so it will always appear correct no matter the background

Screen Shot 2020-02-11 at 12 57 32 PM

Changelog

Changed

  • Change background from ui-background to transparent

Testing / Reviewing

Ensure the inline dropdown does not have a background by manually setting a background color on the container of the inline dropdown.

@tw15egan tw15egan requested a review from a team as a code owner February 11, 2020 21:01
@ghost ghost requested review from abbeyhrt and asudoh February 11, 2020 21:01
@netlify
Copy link

netlify bot commented Feb 11, 2020 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built with commit 53d51ce

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

@netlify
Copy link

netlify bot commented Feb 11, 2020 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit 53d51ce

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

asudoh
asudoh approved these changes Feb 11, 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!

@asudoh asudoh requested review from a team and aagonzales and removed request for a team February 11, 2020 23:54
@aagonzales

This comment has been minimized.

Sign in to view
aagonzales
aagonzales requested changes Feb 13, 2020
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.

So inline dropdown actually should be transparent on enabled, it isn't supposed to have a background expect on hover/active.

(I still don't have a local build so I can't see it)

@asudoh
Copy link
Contributor

asudoh commented Feb 13, 2020

Hi @aagonzales just wanted to double-check if you can test it with https://deploy-preview-5322--carbon-components-react.netlify.com (given React deploy preview didn't fail)? If there was a different reason why you needed a local build my apologies.

@aagonzales
Copy link
Member

@asudoh It's still wrong. Inline dropdown shouldn't have a background color except on hover.It should be transparent and therefor will not need the light variant.

@asudoh
Copy link
Contributor

asudoh commented Feb 17, 2020

@aagonzales Thank you for your response! @tw15egan Does what @aagonzales pointed out happen in your local dev env?

@tw15egan tw15egan force-pushed the inline-dropdown-variant branch 2 times, most recently from 6bfdf08 to 9da8749 Compare February 17, 2020 11:41
@tw15egan
Copy link
Member Author

@aagonzales that makes way more sense, not sure why I didn't do that in the first place. I blame dad brain 🤦‍♂

@asudoh ready for re-review

aagonzales
aagonzales approved these changes Feb 17, 2020
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.

Perfect!

asudoh
asudoh approved these changes Feb 17, 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.

Super 🎉 - Thank you for your update @tw15egan!

@tw15egan tw15egan merged commit ee32294 into carbon-design-system:master Feb 18, 2020
@tw15egan tw15egan changed the title fix(dropdown): ensure inline dropdown has light variant fix(dropdown): ensure inline dropdown has transparent background Feb 18, 2020
@tw15egan
Copy link
Member Author

🤦‍♂ forgot to change commit message, should have been fix(dropdown): ensure inline dropdown has transparent background

@joshblack joshblack mentioned this pull request Feb 25, 2020
Closed
@tw15egan tw15egan mentioned this pull request Oct 6, 2020
Closed
2 tasks
@tw15egan tw15egan deleted the inline-dropdown-variant branch April 28, 2021 18:17
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

@joshblack joshblack joshblack approved these changes

@aagonzales aagonzales aagonzales approved these changes

@abbeyhrt abbeyhrt Awaiting requested review from abbeyhrt

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.

Inline Dropdown bg color in Gray 10 theme
5 participants
@tw15egan @aagonzales @asudoh @joshblack @laurenmrice