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

Popover - ::before and ::after carets are misaligned when using --right-bottom #462

Closed
iancanderson opened this issue Mar 23, 2018 · 2 comments

Comments

@iancanderson
Copy link

When using .Popover-message--right-bottom, the background caret (gray) is not aligned correctly with the foreground (white) caret. See screenshot:

screen shot 2018-03-23 at 10 25 00 am

I think this is because the ::before and ::after pseudo elements have different values for their bottom property, specifically when using --right-bottom orientation: https://github.com/primer/primer/blob/e9b76069771492c5e94656f917ba023b8321694a/modules/primer-popover/lib/popover.scss#L158-L173

Is there a historical reason for the difference in these problems, or should they just be set to the same value?

For example, for the top-oriented carets, their top value is the same:
https://github.com/primer/primer/blob/e9b76069771492c5e94656f917ba023b8321694a/modules/primer-popover/lib/popover.scss#L149-L156

@iancanderson
Copy link
Author

I actually just found that for --right-bottom, I needed to set ::before bottom to 1 pixel less than ::after bottom in order to get them to line up correctly

With equal bottom values

screen shot 2018-03-23 at 10 42 08 am

With ::before up 1 pixel higher than ::after

screen shot 2018-03-23 at 10 41 48 am

@shawnbot
Copy link
Contributor

Thanks for this, @iancanderson! Your solution with ::before 1px higher is the right one: we want the drop shadow to start above the white caret. This particular bit of CSS is under review in github/design-systems#406, but I think it'd be good to fix the popover in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants