1521917 - Truncate selector names in info bar

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Victoria Wang [:victoria]]

Attachment: Screen Shot 2019-01-22 at 12.47.46 PM.png

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.)

Comment 1

[Gabriel Luong [:gl] (ΦωΦ)]

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.

Comment 2

[Sonia Singla :soniasingla]

Hello Micah :)

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

Thanks :D

Comment 3

[Micah [:mtigley] (she/her)]

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.

Comment 4

[Micah [:mtigley] (she/her)]

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!

Comment 5

[Thomas]

Hi,

I'd like to work on this issue.

Regards,
Thomas

Comment 6

[Micah [:mtigley] (she/her)]

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.

Comment 7

[Thomas]

Attachment: Bug 1521917 - Set a max width on infobar text. r=mtigley

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

Comment 8

[Thomas]

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 ?

Comment 9

[Micah [:mtigley] (she/her)]

(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.

Comment 10

[Thomas]

(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.

Comment 11

[Micah [:mtigley] (she/her)]

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.

Comment 12

[Micah [:mtigley] (she/her)]

Attachment: 768px.png

Comment 13

[Micah [:mtigley] (she/her)]

Attachment: 768px.png

Comment 14

[Micah [:mtigley] (she/her)]

Attachment: 500px.png

Comment 15

[Victoria Wang [:victoria]]

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.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76dd4019ac47
Set a max width on infobar text. r=mtigley

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/76dd4019ac47