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

1401 Tooltip Bottom and Bottom Left Arrow Fix #1404

Closed
wants to merge 1 commit into from

Conversation

austingardner
Copy link
Contributor

@austingardner austingardner commented Apr 12, 2021

Description

This fixes the direction of the arrow for the tooltip--bottom and tooltip--bottom-left variants. Previously the arrow was the same as in the top and top-left variants, because the top: 7px was being applied and the top: auto was not being applied.

This was done by adding the span.tooltip__pointer--bottom-left and span.tooltip__pointer--bottom selectors.

References

Closes #1401

Screenshots

Screen Shot 2021-04-08 at 2 45 44 PM

Screen Shot 2021-04-08 at 2 45 53 PM

Screen Shot 2021-04-12 at 2 08 35 PM

Screen Shot 2021-04-12 at 2 28 49 PM

Screen Shot 2021-04-12 at 2 29 00 PM

Screen Shot 2021-04-12 at 2 29 07 PM

@austingardner austingardner changed the title 1401 tooltip experiment 1401 Tooltip Wrong Direction Apr 12, 2021
@austingardner austingardner changed the title 1401 Tooltip Wrong Direction 1401 Tooltip Bottom and Bottom Left Arrow Fix Apr 12, 2021
@ianmcburnie
Copy link
Contributor

I'm not sure why the buttons in bottom right and top right examples are over to the right hand side of the page..?

.pointer-bottom-left();
}

.tooltip__pointer--bottom {
.tooltip__pointer--bottom,
span.tooltip__pointer--bottom {
Copy link
Contributor

@ianmcburnie ianmcburnie Apr 13, 2021

Choose a reason for hiding this comment

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

Hmm, it doesn't seem right to be hammering it with more specificity (and on just two of the pointer modifiers, not all of them). I'll take a look and try and see what changed between v10 and v11. I did a big refactor to try and normalise ds4 and ds6 so I probably broke something then (#1168)

Copy link
Contributor

@ianmcburnie ianmcburnie Apr 13, 2021

Choose a reason for hiding this comment

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

I've been trying to figure out where this override comes from:

Screen Shot 2021-04-13 at 8 28 34 AM

The cascade order of the source looks good, so I suspected some overly aggressive minification, and yup when I disable minification things appears as expected:

Screen Shot 2021-04-13 at 9 05 45 AM

@austingardner Can you see if you can play around with less minification settings to make it less aggressive?

Relevant line is: https://github.com/eBay/skin/blob/master/package.json#L30 EDIT: https://github.com/eBay/skin/blob/master/gulpfile.js#L59

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I can look into that!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, this is the line I meant to point to: https://github.com/eBay/skin/blob/master/gulpfile.js#L59

@ianmcburnie
Copy link
Contributor

So I think we can close this based on: less/less-plugin-clean-css#33 , right?

@ianmcburnie ianmcburnie deleted the 1401-tooltip-experiment branch April 16, 2021 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants