Truncate selector names in info bar

RESOLVED FIXED in Firefox 68

Status

enhancement
P3
normal
RESOLVED FIXED
5 months ago
2 months ago

People

(Reporter: victoria, Assigned: tgerard79, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 68

Firefox Tracking Flags

(firefox68 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

5 months ago

Some sites that use generated CSS selector names end up with super long info bars as seen in the attached screenshot (of https://mobile.twitter.com/home). I think we should truncate with ellisis so that they're never longer than 500px or so (or maybe 50% max width? A little tinkering here would be great.)

Severity: normal → enhancement
Priority: -- → P3

You can see the DOM structure of these infobars here https://searchfox.org/mozilla-central/source/devtools/server/actors/highlighters/box-model.js#85-96.
The CSS for the infobar also lives here https://searchfox.org/mozilla-central/source/devtools/server/actors/highlighters.css#142-161.

The task here is to add an appropriate max width, text-overflow, overflow and white-space properties. You can examine how we do it in codebase https://searchfox.org/mozilla-central/search?q=text-overflow%3A+ellipsis&path=devtools.

Mentor: mtigley

Comment 2

4 months ago

Hello Micah :)

I would like to work on this issue. Can you please help me in getting started?

Thanks :D

Hi Sonia, thank you for your interest in this issue! You can read through the "Getting Started" and "Contributing" docs at https://docs.firefox-dev.tools/ to get yourself started. Please feel free to reach out if you have any questions.

In the meantime, I will assign you to this bug.

Assignee: nobody → soniasingla.1812

I believe this is no longer being worked on, so I'll un-assign this for someone else to pick up. However, if that's not the case, please feel free to assign yourself again Sonia!

Assignee: soniasingla.1812 → nobody
Assignee

Comment 5

2 months ago

Hi,

I'd like to work on this issue.

Regards,
Thomas

Hi Thomas, thank you for your interest in this issue! I'll assign you now.

Please Comment 3 for getting your work environment setup for contributing to DevTools.

Assignee: nobody → tgerard79
Status: NEW → ASSIGNED
Assignee

Comment 7

2 months ago

Add a max width on infobar when inspecting elements.
The intent is to prevent very long infobars due to autogenerated classes or id.

Assignee

Comment 8

2 months ago

Thank you Micah.

I just asked you for review. I remembered the tests when the editor opened.
Could you help me with that ?
I found infobar tests in inspector/test. Shoud I use one of those or create a new file ?

(In reply to Thomas from comment #8)

Thank you Micah.

I just asked you for review. I remembered the tests when the editor opened.
Could you help me with that ?
I found infobar tests in inspector/test. Shoud I use one of those or create a new file ?

Hi Thomas,

Thank you for the patch. I tested this out and it works great! I left some comments on Phabricator.

For tests, we don't need to worry about them if we're concerned about the highlighter info-bar's max-width styles. I think this patch alone is good enough.

Assignee

Comment 10

2 months ago

(In reply to Micah Tigley [:mtigley] from comment #9)

Hi Thomas,

Thank you for the patch. I tested this out and it works great! I left some comments on Phabricator.

For tests, we don't need to worry about them if we're concerned about the highlighter info-bar's max-width styles. I think this patch alone is good enough.

Thank you Micah,

I understand what you said in Phabricator regarding following the suggestion made by Victoria.
For Victoria's information, I summarize what I said in Phabricator.
Since I set the max-width on the overall infobar, I found that 500px was too short for pretty common use cases like one autogenerated id and 3-4 classes. In thise case we could hardly see all the selectors.
For information, I use autogenerated id for accessibility in components and I need to check easily the match between id and aria-labelledby attribute for example. I take my use case as an example but I think it is pretty common. Regarding classes, 3-4 classes or more with BEM styles or the new (or somehow renewal) trend of utility classes are not uncommon too. We could do it through the DOM but it is a matter of convenience, like the infobar.

Though, I suggested we could put a 500px max width on each block (id and class) or class only.

Let me know what you think.

Thank you for explaining this, Thomas. I understand what you're saying. I'm more inclined to have the overall infobar container set to a max-width to avoid the id and class blocks getting overly long.

I will needinfo Victoria on this and see what she thinks about the infobar length. I uploaded my own screenshots comparing the infobar's max-width at both 768px and 500px. Please feel free to upload additional screenshots you feel might illustrate the problem better.

Flags: needinfo?(victoria)
Posted image 768px.png
Posted image 768px.png (obsolete) —
Posted image 500px.png
Attachment #9060154 - Attachment is obsolete: true
Reporter

Comment 15

2 months ago

Thanks for the screenshots - it looks like in addition to the max width on the full infobar, the individual selector blocks each truncate evenly? That is so nice! And the 768px looks fine to me.

Flags: needinfo?(victoria)

Comment 16

2 months ago
Pushed by mtigley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/76dd4019ac47
Set a max width on infobar text. r=mtigley

Comment 17

2 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68
You need to log in before you can comment on or make changes to this bug.