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

feat: add align to tooltip #7309

Merged

Conversation

lee-chase
Copy link
Member

@lee-chase lee-chase commented Nov 18, 2020

Closes #3837

C&CS would like to use Tooltip as a basic overflow for tag components. However, this is problematic because overflow typically happens near the right side of the page and there is no option to align the tooltip

See https://ibm-cloud-cognitive.netlify.app/?path=/story/experimental-tagset--tag-array

NOTE: Width of the container is adjusted on the controls tab.

Changelog

M packages/components/src/components/tooltip/_tooltip.scss
M packages/react/src/components/Tooltip/Tooltip-story.js
M packages/react/src/components/Tooltip/Tooltip.js
M. packages/react/src/components/Tooltip/Tooltip-test.js

Testing / Reviewing

Ran it up in storybook.

@netlify
Copy link

netlify bot commented Nov 18, 2020

Deploy preview for carbon-elements ready!

Built with commit ae0d27d

https://deploy-preview-7309--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Nov 18, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit ae0d27d

https://deploy-preview-7309--carbon-components-react.netlify.app

@netlify
Copy link

netlify bot commented Nov 18, 2020

Deploy preview for carbon-elements ready!

Built with commit 0c628f6

https://deploy-preview-7309--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Nov 18, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 0c628f6

https://deploy-preview-7309--carbon-components-react.netlify.app

@joshblack joshblack requested review from a team and laurenmrice and removed request for a team November 20, 2020 21:13
Copy link
Collaborator

@tw15egan tw15egan 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 for adding this in! 👍 ✅

@tw15egan
Copy link
Collaborator

One thing to note for v11, but Button uses TooltipDirection and TooltipAlignment. We'll probably want to conform to a consistent name, I don't really have a preference either way.

I've made a note in the v11 Prop Audit as well.

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Can we have a 16px padding before the tooltip sharkfin? This will align vertically with the text and match the padding on the opposite side.
Artboard

@lee-chase
Copy link
Member Author

Can we have a 16px padding before the tooltip sharkfin? This will align vertically with the text and match the padding on the opposite side.
Artboard

Sure can @laurenmrice also adjusted left and right to align with the text.

image
image
image
image

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

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

Bottom and Top directions
-start
-end

These need to have a 16px space between the edge of the shark fin and the container. Right now they still are not aligning vertically with the text.

1


Left and Right directions
-center

The shark fin is not showing up at the center of the container. There is not an equal amount of space above or below it right now.

@lee-chase
Copy link
Member Author

@laurenmrice Addressed the position for start and end when above and blow.

The center-left and center-right caret positions I have not changed as part of this PR. I do not know the reason they were set 9px above centre on the left and 3px below on the right. It has been like this since changes in #5304, perhaps @tw15egan could shed light on the reason for the offset.

@tw15egan
Copy link
Collaborator

tw15egan commented Dec 1, 2020

@lee-chase that offset was to get them centered in that specific PR, but if they aren't needed any more just axe em 👍

@lee-chase
Copy link
Member Author

Unfortunately, I am not familiar with the floating menu code. Could that be addressed in a separate issue as this issue has not changed it?

@laurenmrice
Copy link
Member

laurenmrice commented Dec 2, 2020

I am fine with addressing the centering problem in a separate pr 👍🏻. The position for start and end looks good now, thank you!

@kodiakhq kodiakhq bot merged commit 66ef844 into carbon-design-system:master Dec 2, 2020
@lee-chase lee-chase deleted the addAlignOptionsToTooltip branch December 7, 2020 10:40
lee-chase added a commit to lee-chase/carbon that referenced this pull request Dec 8, 2020
kodiakhq bot added a commit that referenced this pull request Dec 10, 2020
Co-authored-by: Lee Chase <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
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.

Add alignment to the Carbon's tooltip component
4 participants