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(accordion): resolve accordion header z-index issue #5276

Merged
merged 6 commits into from Feb 7, 2020

Conversation

tw15egan
Copy link
Member

@tw15egan tw15egan commented Feb 5, 2020

Closes #5272

Refs #1674

Changes z-index value so that radio buttons are not overlayed on tooltip inside the accordion header.

Changelog

Changed

  • Bumped z-index from 0 to 1

Testing / Reviewing

See https://codesandbox.io/s/zen-hofstadter-km7to for example of the issue. Check a radio button, then hover over the filter icon. Inspect .bx--accordion__title and set z-index: 1;, and see that you can no longer see the filled radio button.

@tw15egan tw15egan requested a review from a team as a code owner February 5, 2020 20:44
@ghost ghost requested review from aledavila and joshblack February 5, 2020 20:44
@tw15egan
Copy link
Member Author

tw15egan commented Feb 5, 2020

cc @asudoh , ref'd the PR that added that z-index in case I am missing why it was added in the first place.

@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-elements ready!

Built with commit 126861a

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

@netlify
Copy link

netlify bot commented Feb 5, 2020

Deploy preview for carbon-components-react failed.

Built with commit 126861a

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

@asudoh
Copy link
Contributor

asudoh commented Feb 5, 2020

Thanks @tw15egan - As I think you saw, blame history points to #1671 - Would you be able to double-check if every bullets in there has no problem?

@tw15egan
Copy link
Member Author

tw15egan commented Feb 6, 2020

@asudoh checked the list and all still seems good 👍

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!

@abbeyhrt
Copy link
Contributor

abbeyhrt commented Feb 7, 2020

I noticed that the issue is also resolved if the z-index is removed, would that be another way to go about this?

@tw15egan
Copy link
Member Author

tw15egan commented Feb 7, 2020

@abbeyhrt I think then the accordion header text is covered on hover

@abbeyhrt
Copy link
Contributor

abbeyhrt commented Feb 7, 2020

@tw15egan ah that makes sense! Thanks!

@tw15egan
Copy link
Member Author

tw15egan commented Feb 7, 2020

@abbeyhrt Yeah I think it has to do with the use of before elements to cover up the borders when they are hovered 🤷‍♂

@tw15egan tw15egan merged commit 554c9ce into carbon-design-system:master Feb 7, 2020
@tw15egan tw15egan deleted the accordion-fix branch April 28, 2021 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip Icon on an accordion label has z-index 0, which gets behind certain elements
3 participants