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(tooltip): set trigger button margin only when descendent of label #4672

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Nov 13, 2019

Closes #4671

This PR moves the left margin rule for non-icon-only tooltip trigger buttons to a new selector that only targets tool trigger buttons that are descendants of tooltip labels

Changelog

Changed

  • change selector for tooltip trigger button left margin from .bx--tooltip__trigger:not(.bx--btn--icon-only) to .bx--tooltip__label .bx--tooltip__trigger

Testing / Reviewing

Ensure overflow menu trigger button has the correct size and no margins

Ensure labeled interactive tooltip trigger button has space between it and its label

Ensure all components that consume tooltip appear as expected

@netlify
Copy link

netlify bot commented Nov 13, 2019

Deploy preview for the-carbon-components ready!

Built with commit e80a3d9

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

@netlify
Copy link

netlify bot commented Nov 13, 2019

Deploy preview for carbon-components-react ready!

Built with commit e80a3d9

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

@netlify
Copy link

netlify bot commented Nov 13, 2019

Deploy preview for carbon-elements ready!

Built with commit e80a3d9

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

@asudoh asudoh requested a review from a team November 13, 2019 21:37
@ghost ghost requested review from designertyler and removed request for a team November 13, 2019 21:37
@designertyler
Copy link
Contributor

I'm not sure if this is related, but there's a white line or gap that's showing up the tooltip. Everything else looked fine

image

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 👍 unless what @designertyler pointed out was a regression. Thanks @emyarod!

@emyarod
Copy link
Member Author

emyarod commented Nov 14, 2019

that's something we discussed in a tooltip ticket or PR a while ago (would have to go back to find related discussion) but unless there's now a consistent way to reproduce that issue, I think it's related to screen resolution/retina screens? not sure if that's a true bug or not but in any case I don't think it's a regression introduced in this PR

@asudoh asudoh merged commit 1a2d21b into carbon-design-system:master Nov 14, 2019
@emyarod emyarod deleted the 4671-tooltip-trigger-button-left-margin branch November 14, 2019 22:18
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.

Overflow menu trigger button has extra left margin
3 participants