Skip to content

Markdown rich diff tooltip appears underneath diff settings bar#1308

Closed
ghost wants to merge 1 commit into
refined-github:masterfrom
mngatewood:diff-tooltip
Closed

Markdown rich diff tooltip appears underneath diff settings bar#1308
ghost wants to merge 1 commit into
refined-github:masterfrom
mngatewood:diff-tooltip

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented May 8, 2018

Tooltips now appear above buttons on hover.

screen shot 2018-05-08 at 14 24 58

screen shot 2018-05-08 at 14 29 03

I ran npm test and got the following output https://user-images.githubusercontent.com/19957343/39781814-7cb09b82-52cd-11e8-93c6-c4fc504443c3.png

Fixes #1295

@mngatewood
Copy link
Copy Markdown

Note that we tested this in Chrome, Firefox, Opera. Thanks.

@fregante
Copy link
Copy Markdown
Member

fregante commented May 9, 2018

Interesting. This has the side effect of bringing the whole button above the file bar (and not just the tooltip, whose tooltip z-index is relative to the button itself so it can’t be moved on its own). Scroll 20px down in your screenshot to notice the issue. Perhaps it’s not a big deal since it happens only on hover.

Then: the selector is very specific but we have the same issue on other file views, everywhere we have a file header. Can this be a bit more generic and not just for pull requests?

@fregante fregante added the bug label May 9, 2018
@ghost
Copy link
Copy Markdown
Author

ghost commented May 9, 2018

@bfred-it Good catch. We didn’t notice that buggy behavior until after several scroll-throughs. Could we then make the tooltip appear below the buttons, or is the solution we proposed in the PR an acceptable alternative?

Regarding specificity of the selector, we can only produce this behavior when there are two headers, as in the case of the PR, with the diff setting toolbar and the file header. If you happen to know of this issue in any other place, can you point us to that direction as well? Thanks.

@fregante
Copy link
Copy Markdown
Member

we can only produce this behavior when there are two headers

Ah you're correct

is the solution we proposed in the PR an acceptable alternative?

It is for me, but let's see what others say

@fregante
Copy link
Copy Markdown
Member

The effect is not what I expected but worse. If we're gonna add a CSS solution I'd prefer forcing the tooltips to point downwards instead.

cast

Copy link
Copy Markdown
Member

@fregante fregante left a comment

Choose a reason for hiding this comment

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

⬆️

@fregante fregante changed the title Markdown rich diff tooltip appears underneath diff settings bar #1295 Markdown rich diff tooltip appears underneath diff settings bar May 21, 2018
@fregante
Copy link
Copy Markdown
Member

Closing this for now. If you want to push the alternative solution feel free to open a new PR

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

Labels

Development

Successfully merging this pull request may close these issues.

2 participants