In flex inspector, the flex-item graphic has its "basis/final" text smooshed against the image

VERIFIED FIXED in Firefox 66

Status

P3
normal
VERIFIED FIXED
4 months ago
a month ago

People

(Reporter: dholbert, Assigned: akshithashetty84, Mentored, NeedInfo)

Tracking

(Blocks: 1 bug, {good-first-bug})

unspecified
Firefox 66
good-first-bug

Firefox Tracking Flags

(firefox66 verified)

Details

(Whiteboard: [lang=css])

Attachments

(4 attachments)

(Reporter)

Description

4 months ago
STR:
 1. Visit data:text/html,<div style="display:flex"><span>abc
 2. Right-click "abc" and inspect
 3. Click the "flex" badge, and then click the span to show its flex item info

EXPECTED RESULTS:
The "basis/final" text label should be alongside the "|" marker that it's labeling, and there should be a little space between it and the graphic, for readability.

ACTUAL RESULTS:
- The "basis/final" text is a few pixels lower than its | marker.
- The text is smooshed against the graphic, so that the bottoms of the letters are bumping directly into the similarly-colored border of the graphic below them, which makes them harder to read.

I ran into this on Win10 nightly: 65.0a1 (2018-12-06) (64-bit).  Haven't yet checked whether other platforms are affected.
Blocks: 1409965
No longer blocks: 1478396
Thanks for filing. I believe this can be solved by adding line-height: 1; to the CSS rule .flex-outline-point::before in /devtools/client/themes/layout.css
At least on mac it looks good this way.
I would be great to test how this looks on windows and linux too.

I'm marking this as a good first bug because I think it is.
If you are interested in fixing it, please make sure you go through our contribution guide first: http://docs.firefox-dev.tools/
And then don't hesitate to ask questions here! I can help.
Mentor: pbrosset
Keywords: good-first-bug
Priority: -- → P3
Whiteboard: [lang=css]
(Reporter)

Comment 2

4 months ago
Sorry, I'd meant to attach a screenshot when filing -- here's how this looks in current Nightly on Win10.
(Reporter)

Comment 3

4 months ago
Here's how this looks in Win10 if I make Patrick's suggested "line-height" tweak (using devtools to edit on the fly -- I didn't actually write the patch).

It's still not quite perfect (intuitively I'd expect the text and the marker to be baseline-aligned, and they're not quite), but it looks much better, and does seem good enough (no smooshing) to call this bug fixed.
(Assignee)

Comment 4

2 months ago

I would like to work on this one. Could you please assign it to me?

Thanks for your interest in helping fix this issue. I'll assign the bug to you now.
As you can see, some instructions were given in comment 1. Please do get started and don't hesitate to come back here and ask any questions you might have. I'll do my best to answer them and help you land a fix.

Assignee: nobody → akshithashetty84
Status: NEW → ASSIGNED
(Assignee)

Comment 6

2 months ago

Thanks,
Will start working on it and get back here.

(Assignee)

Comment 7

2 months ago

Hi,
I have added the line-height property as suggested on linux and attached a screenshot of the same. It looks good to me. Could you please review it ?

Hi Akshitha, your fix seems to look good on linux, thank you. But I don't see your code changes here.
Could you send a patch for review as described in our documentation please: https://docs.firefox-dev.tools/contributing/code-reviews-setup.html

Flags: needinfo?(akshithashetty84)

Comment 10

2 months ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33ca660f7670
Resolved the smooshing of the - basis/final - text against the image in flex inspector's flex-item graphic . r=pbro

Comment 11

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox66: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Flags: qe-verify+
QA Contact: cristian.comorasu

I reproduced this issue using Fx 65.0a1 (2018-02-19), on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 66.0b9, on Windows 10 x64, macOS 10.13 and Ubuntu 14.04 LTS.

Status: RESOLVED → VERIFIED
status-firefox66: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.