1172413 - Remove the line separating the tooltip contents and the tooltip arrow

Status: VERIFIED FIXED

(DevTools :: General, defect)

Description

[Sebastian Zartner [:sebo]]

Attachment: Tooltip with arrow split line

The arrow of tooltips shown for images, web fonts, JavaScript variables, etc. pointing to the referenced item is currently separated by a line from the tooltip content, which makes the tooltip look a bit ugly.

So it would be better to restyle the tooltip to get rid of that split line, so the tooltip content and the arrow look like one unit.

Sebastian

Comment 1

[Sebastian Zartner [:sebo]]

Attachment: Tooltip without arrow split line

This is how it should look like.

Sebastian

Comment 2

[steven.c.das]

Hi Sebastian,

I'd be happy to take on this bug; feel free to please assign it to me as you get a minute!

Many thanks,

Steven

Comment 3

[Sebastian Zartner [:sebo]]

It's now assigned to you. Thanks for taking this bug!

Sebastian

Comment 4

[Sebastian Zartner [:sebo]]

Steven, the bug is assigned to you for three months now. Is there any progress on it?

Sebastian

Comment 5

[steven.c.das]

Hi Sebastian,

Thanks for checking in.

It may be best for me to relinquish this bug:  I will admit that it got completely away from me after the startup I had been working for collapsed and I needed to enter full-time job search mode, and it doesn't look like the free time situation has improved much since then.

Apologies for the delay:  I should have gotten you this update much sooner.  Let me know if I can help in any other way!

Best,

Steven

Comment 6

[Sebastian Zartner [:sebo]]

Thanks for the fast reply! Don't worry, time is always a problem.

Sebastian

Comment 7

[Nikhil Gupta]

Can you please tell the CSS file which contains the relevant code and needs to be modified?

Comment 8

[Sebastian Zartner [:sebo]]

It looks like it is styled via the classes 'devtools-tooltip' and 'theme-tooltip-panel' located here:
http://mxr.mozilla.org/mozilla-central/source/devtools/client/themes/common.css
http://mxr.mozilla.org/mozilla-central/source/devtools/client/themes/dark-theme.css
http://mxr.mozilla.org/mozilla-central/source/devtools/client/themes/light-theme.css

And the tooltip logic is located at http://mxr.mozilla.org/mozilla-central/source/devtools/client/shared/widgets/Tooltip.js.

Though for any additional help, you should ask Patrick Brosset, which implemented huge parts of the related code.

Sebastian

Comment 9

[Nikhil Gupta]

Attachment: Bug1172413_removetooltipline.diff

Changed the margin to make the arrow tip overlap the border. Although it's not perfect (due to transparency), it serves the purpose quite well. Tell me if further improvement is required.

Comment 10

[Patrick Brosset <:pbro>]

Comment on attachment 8665506 [details] [diff] [review]
Bug1172413_removetooltipline.diff

Review of attachment 8665506 [details] [diff] [review]:
-----------------------------------------------------------------

To make things a bit more tricky, this problem only occurs on Windows (at least that's what I've seen). On mac, the tooltip arrow is positioned exactly as it should.
To make things even *more* tricky, the display is different on higher density screens (like on macbook with retina screens).
So, I tried your changes on a mac retina, and I'm afraid they don't work as they should. The arrow is too high there (see this screenshot: https://dl.dropboxusercontent.com/u/714210/tooltip-mac-retina.png )

@Brian: what's the correct way these day to have OS-specific CSS? Didn't we have a class in the DOM we could use for conditional rules?

Comment 11

[Brian Grinstead [:bgrins]]

Comment on attachment 8665506 [details] [diff] [review]
Bug1172413_removetooltipline.diff

Review of attachment 8665506 [details] [diff] [review]:
-----------------------------------------------------------------

If it's truly Windows-only you can use the preprocessor:

%ifdef XP_WIN

There's bug 1183280 on file to remove this preprocessor usage so this will get scooped up at that time.  I'd do this personally:

.theme-tooltip-panel .panel-arrow {
  --arrow-margin: -4px;
%ifdef XP_WIN
  --arrow-margin: -7px;
%endif
}

Put that above the changed rules and then change them to all use `margin-bottom: var(--arrow-margin);` instead of `margin-bottom: -4px`, etc.  That will make getting rid of the preprocessor usage easy if we add a class on the root indicating the OS.

Comment 12

[Nikhil Gupta]

Attachment: bug1172413_tooltiparrow.diff

Comment 13

[Patrick Brosset <:pbro>]

Comment on attachment 8671742 [details] [diff] [review]
bug1172413_tooltiparrow.diff

Review of attachment 8671742 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

Comment 14

[Patrick Brosset <:pbro>]

(In reply to Nikhil Gupta from comment #9)
> Although it's not perfect (due to transparency)
Yeah, that's a problem we've had from the beginning with these tooltips. It's more visible with the light theme than with the dark theme btw.
It's due to the fact that the panel-arrowcontent is just a rectangular element with a border all around, and the panel-arrowcontainer is shifted up so that the arrow triangle shape covers a bit of the border. And due to transparency, that part of the border is somewhat visible.

Comment 15

[Patrick Brosset <:pbro>]

Attachment: Bug_1172413_-_Make_the_arrow_overlap_with_the_box_.diff

This bug fell off my radar. Here's a rebased patch (I took the opportunity to remove the preprocessor since we have a platform attribute on :root now).
I will land this asap.

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3d4f379b47fc

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d4f379b47fc

Comment 18

[Sebastian Zartner [:sebo]]

Works for me. Thank you for the fix!

Sebastian