1568847 - Make whitespace text node in the markup view more explicit

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

We help users visualize whitespace text nodes that impact the layout, but our UI for this is quite cryptic. This feature was added in Bug 1304685. Such nodes are currently displayed in the markup view as a badge-like element that just contains a circle. The circle was initially chosen because it was similar to how text-editors show whitespaces.

See answers to https://twitter.com/FirefoxDevTools/status/1153936100681015297 , users seem to find the feature useful, but had no idea it was there.

The goal of this bug is to make the feature more discoverable.

Comment 1

[Julian Descottes [:jdescottes]]

A suggestion from Victoria was to use a badge that says [space].
This is similar to one of the designs we suggested initially, a badge saying [whitespace].

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: old-mockup.png

This is from Bug 1304685, hence the old inspector UI.

Comment 3

[Victoria Wang [:victoria]]

Regarding the tooltip: Currently, it shows the whitespace characters used, such as the return symbol for carriage returns and circles to indicate spaces. We can expand on that for more clarity by spelling it out like so (probably with some truncation after a certain point)

Whitespace-only text node: return, space, space, space, space, space ...

A more thorough solution would be to covert the title attr tooltip into an Inactive CSS-style tooltip, with a Learn More link to this MDN page. I did some quick editing today on this page, and could do more if needed. (E.g. Do we want the first section to focus more on the CSS/HTML layout bug implications?)

Comment 4

[Florens Verschelde :fvsch]

Attachment: markup-whitespace-badge-tests.png

Badge

A badge of sorts that says "whitespace" could be useful. In practice in modern layouts with a lot of Flexbox I'm not seeing the "significant whitespace" nodes often (I had trouble finding examples to look at for this bug!), so when we do display it we can afford to make it bigger.

I hacked together a few options:

• plain text or with a "badge" look (colors are not exact)
• with the word "whitespace", or the more geeky \w, or a text representation of the actual content

The content representation could be the same we're using right now in the tooltip, or something a bit more code-like with \n for newlines, \t for tabs and either a blank space or a small bullet or middle-dot character for spaces; we could truncate it to the first 6-10 characters.

I'm not sure I like the badge look. One worry is that we're currently using clickable badges that are attached to the end of opening tags (<element attribute="foo"> [badge]), so users may read the badge look as meaning that the whitespace belongs to the previous node?

Tooltip

Spelling out the individual characters feels a bit strange. The current representation is alright, though it could be improved if we could style it (in a custom tooltip).

A more thorough solution would be to convert the title attr tooltip into an Inactive CSS-style tooltip, with a Learn More link to this MDN page

Love the idea.

Do we want the first section to focus more on the CSS/HTML layout bug implications?

Probably, this page feels strange when coming from the Inspector since it very quickly gives you a bunch of JS code that can be used for… er something, not sure what. Maybe that page needs an overhaul that is more HTML-and-CSS-friendly, and less "I'm writing DOM whitespace manipulation code".

The article from Patrick that is linked is actually very close to what would be useful in this use case. Maybe we should update it and publish it on MDN, and link to that?

Comment 5

[Victoria Wang [:victoria]]

Badge

Thanks for the mockups! There is indeed a bit of weirdness with using badges here — they haven't represented a specific bit of content before. I do love that it makes them more prominent in your bottom row examples.

I think I'd avoid displaying \w by itself because I suspect a lot of HTML/CSS-only devs won't be familiar with that. Representing the actual whitespace in the UI makes sense though, especially if we can do it more reliably! (Julian found one example in which that part shows in the tooltip as blank). Maybe something like this:

[whitespace: ••••\t\r]

Tooltip

If we're showing the whitespace representation from the tooltip right in the badge, then the tooltip could remain a simple title attr. It could even just say Learn More, with the badge potentially linking to the MDN page (though we'd want some kind of info icon if it's going to directly link anywhere).

MDN page

Yeah, the page used to not have anything on layout - I had just added those parts. I'll pull in the MDN folks on whether we could revamp the page or create a new page.

Comment 6

[Victoria Wang [:victoria]]

Hi Chris! We're wondering about this page, Whitespace in the DOM.

Before yesterday, the page was mainly based on the use case of writing DOM manipulation code. However, we recently got a lot of Twitter love for our whitespace node marker in DevTools, which is much more about HTML/CSS layout issues due to unexpected effects of whitespace. Thus, I started to add a bit of info on this to the page, including a link to this article by Patrick.

Because we want to start linking to it from inside DevTools as part of our new whitespace node design, we feel it would be best to have an MDN page that is more focused on the layout issues up front, with more of the info from Patrick. We also think the HTML/CSS use case is more common. Would it make sense to update the existing page for this, or create a new page about 'Whitespace in HTML/CSS Layouts'?

Comment 7

[Chris Mills [:cmills]]

Hi Victoria,

I think improving this page is a really good idea — it is already useful content, but I do agree that it should have some more practical stuff in there. Patrick's article is interesting and useful; the issue discussed at https://patrickbrosset.com/articles/2016-10-21-when-does-white-space-matter-in-HTML.html#ecde comes up all the time when you are trying to create a horizontal nav or rows of a photo gallery, for example, and it does really confuse beginners. Although it is less of a problem when you use modern responsive layout techiques like grid or flexbox.

I'd advise to keep it all in one article rather than do two separare ones, as it is all closely-related.

The only question then is where we put that article. It more logically feels like a CSS/layout thing than an API thing, so we could consider moving it under the CSS section of MDN, and perhaps renaming it slightly to something like "Solving layout problems due to excess whitespace"?

Comment 8

[Victoria Wang [:victoria]]

Thanks for the info!

As far as whether this article should be in the CSS Layout category - I'm needinfoing Patrick for his opinion.

As far as refreshing the article to have more practical details about web layouts, such as information from Patrick's article, I wanted to check if anyone on your team is available to work on this :). I'd love to take this on, but I'm overloaded at the moment.

Comment 9

[Patrick Brosset <:pbro>]

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #7)

The only question then is where we put that article. It more logically feels like a CSS/layout thing than an API thing, so we could consider moving it under the CSS section of MDN, and perhaps renaming it slightly to something like "Solving layout problems due to excess whitespace"?

I would definitely move this under CSS on MDN. This is all really about how CSS handles whitespace on the web, so it makes sense to move it to that category.

Comment 10

[Chris Mills [:cmills]]

As far as refreshing the article to have more practical details about web layouts, such as information from Patrick's article, I wanted to check if anyone on your team is available to work on this :). I'd love to take this on, but I'm overloaded at the moment.

OK. Can you file an issue at https://github.com/mdn/sprints/issues, explaining what should be done? Then we'll prioritise it as part of our sprint process.

Comment 11

[Victoria Wang [:victoria]]

Should we define an initial goal for this bug as just changing the label of the icon to "whitespace"? Could be a good first bug :). We can experiment with different representations of whitespace later.

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #10)

OK. Can you file an issue at https://github.com/mdn/sprints/issues, explaining what should be done? Then we'll prioritise it as part of our sprint process.

Thanks, will do!

Comment 12

[Victoria Wang [:victoria]]

Clarifying next steps for this bug :)

• Change badge text from • to whitespace
• Match styling of scroll badge
• Horizontally align badge with its peer nodes by removing padding from .editor.text .whitespace and removing margin from .editor.text .whitespace::before

Comment 13

[Jean]

Hey all, I'd like to grab this one (seems like a good bug to get familiar with the contribution workflow). I'm following the documentation at https://docs.firefox-dev.tools/ (just got a local Firefox build running); is there anything I should be looking at?

Comment 14

[Patrick Brosset <:pbro>]

Hi Jean, thanks for wanting to work on this. I'll assign the bug to you now.
That website is the right one, great to hear you have a working local build! You're all set to work on this then.
The list of things to do is in comment 12. And the files you will be wanting to modify are:

• the React component where the whitespace symbol is generated: devtools/client/inspector/markup/components/TextNode.js
• and the CSS file for it: devtools/client/themes/markup.css

I highly suggest using the Browser Toolbox to help you debug/inspect. This tool is like devtools for the entire Firefox. So you can inspect devtools with devtools, and therefore check if your style changes work correctly, and fine tune them without having to re-build.

Also, because you will only be changing static files, no need for a full build whenever you make a change. Once you've made the change, you can run ./mach build faster and then ./mach run.

Comment 15

[Jean]

Attachment: Bug 1568847 - Change whitespace node's text to "whitespace" and restyle the node. r=jdescottes!

Change the displayed text of whitespace nodes in the inspector to "whitespace, and add a corresponding value to the inspector localized file. Also restyled the badge to match the scroll badge.

Comment 16

[Patrick Brosset <:pbro>]

Awesome, thanks for the patch Jean! I see it was reviewed by Julian and Flod already. Are you ok taking it from there, addressing the comments and posting a new version? If you need any help, please do ask.
I'm looking forward to seeing this land!

Comment 17

[Julian Descottes [:jdescottes]]

(Jean updated the patch and I will now land it, so clearing the ni? here)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d44c6f765cb
Change whitespace node's text to "whitespace" and restyle the node. r=jdescottes,flod

Comment 19

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6d44c6f765cb

Comment 20

[Victoria Wang [:victoria]]

Thanks all for the great work!

I think this patch is still missing one part - fixing the alignment of the badge so that it's flush with other tags.

• Horizontally align badge with its peer nodes by removing padding from .editor.text .whitespace and removing margin from .editor.text .whitespace::before

Should I reopen this bug or create a new one?

Comment 21

[Julian Descottes [:jdescottes]]

(In reply to Victoria Wang [:victoria] from comment #20)

Thanks all for the great work!

I think this patch is still missing one part - fixing the alignment of the badge so that it's flush with other tags.

• Horizontally align badge with its peer nodes by removing padding from .editor.text .whitespace and removing margin from .editor.text .whitespace::before

Should I reopen this bug or create a new one?

Please open another bug, blocking this one. Reopening bugs or using leave-open makes things very hard to track.

Comment 22

[Victoria Wang [:victoria]]

Sounds good, filed: New whitespace badge design - small tweak to alignment