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(tooltip): tooltip margin bug #8461

Merged
merged 15 commits into from
May 3, 2021
Merged

fix(tooltip): tooltip margin bug #8461

merged 15 commits into from
May 3, 2021

Conversation

andreancardona
Copy link
Contributor

Closes #8275

Adds small 8px margin to tooltip to match current design in Sketch kit

@andreancardona andreancardona requested a review from a team as a code owner April 20, 2021 21:22
@netlify
Copy link

netlify bot commented Apr 20, 2021 •
edited
Loading

Deploy preview for carbon-elements ready!

Built with commit dc027ee

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

@netlify
Copy link

netlify bot commented Apr 20, 2021 •
edited
Loading

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit dc027ee

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

laurenmrice
laurenmrice requested changes Apr 20, 2021
View reviewed changes
Copy link
Member

@laurenmrice laurenmrice left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The alignments are off when you change the direction prop to left , right and top. After fixing those I would also check them against the alignment prop options as well.

Everything else looks good though!

tooltip

laurenmrice
laurenmrice requested changes Apr 28, 2021
View reviewed changes
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.

  • The left and top directions with start, center and end alignments still need 8px (spacing-03) between the icon and the caret tip. The others look good !

@andreancardona
Copy link
Contributor Author

@tw15egan @laurenmrice updated :)

This was linked to issues Apr 29, 2021
Proposal: use button for toggles instead of checkbox #6576
Closed
[Toggle] label is misplaced in RTL #5944
Closed
laurenmrice
laurenmrice requested changes Apr 29, 2021
View reviewed changes
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.

  • The top direction for start,center and end alignment needs to fixed still. The rest of the directions and alignments look great!

@andreancardona
Copy link
Contributor Author

  • direction for start,center and end alignment

uff yeah - okay fixing now!

This was unlinked from issues Apr 29, 2021
[Toggle] label is misplaced in RTL #5944
Closed
Proposal: use button for toggles instead of checkbox #6576
Closed
laurenmrice
laurenmrice approved these changes May 3, 2021
View reviewed changes
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.

Looks good to me! 🎉

tw15egan
tw15egan approved these changes May 3, 2021
View reviewed changes
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 👍 ✅

@andreancardona andreancardona enabled auto-merge (squash) May 3, 2021 19:00
@andreancardona andreancardona merged commit 92900da into carbon-design-system:main May 3, 2021
@andreancardona andreancardona mentioned this pull request May 6, 2021
Closed
94 tasks
@tw15egan tw15egan mentioned this pull request May 10, 2021
Closed
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurenmrice laurenmrice laurenmrice approved these changes

@tw15egan tw15egan tw15egan approved these changes

@tay1orjones tay1orjones tay1orjones approved these changes

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.

Space between icon and tooltip in Carbon tooltip component does not match sketch kit
4 participants
@andreancardona @tay1orjones @tw15egan @laurenmrice