Closed Bug 1493677 Opened 6 years ago Closed 6 years ago

Link box-model values to their source CSS rules

Categories

(DevTools :: Inspector: Rules, enhancement, P1)

enhancement

Tracking

(firefox67 fixed)

RESOLVED FIXED
Firefox 67
Tracking Status
firefox67 --- fixed

People

(Reporter: pbro, Assigned: mtigley)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(3 files)

This is a feature request from the Mozilla WebCompat team. In the box-model diagram in the inspector's sidebar, the various margin, border, padding and content size computed values are displayed. Now, it would be very useful if it was possible for users to jump from those values to where they were defined in CSS. We used to have a tooltip on hover of these values that would give the CSS selector of the rule, but that doesn't seem to exist anymore for some reason.
Priority: -- → P1
Depends on: 1501962
Assignee: nobody → mtigley
Status: NEW → ASSIGNED

We might need to think about how we want to select those values from the box-model diagram.

Would a single click on the property value suffice? This would also trigger the value to become editable in the box diagram, which I don't think we want.

We could also use modifier keys. Holding down the shift key and clicking the property value would scroll to it in the rules view.

Another thing is that a property value may not directly link to a source CSS rule. So there wouldn't be a property to jump to. I wonder if we should be providing some visual indication if this is the case.

Flags: needinfo?(pbrosset)
Flags: needinfo?(mbalfanz)

That's a really good question Micah.
I guess displaying some sort of arrow or link icon next to the values that do link to a property would work, but then we'd have space issues (left/right values typically don't have a lot of space to get displayed already).
We could also have a tooltip again, but one that can be interacted with (like the debugger's variable hover tooltip), so that once it appears, users can not only read what's in there (possibly the CSS selector to the rule), but also click on it to jump to the rule-view.

Let me transfer the needinfo to Victoria.

Flags: needinfo?(pbrosset) → needinfo?(victoria)

I think an icon is not an option as we have just to little space.

I do like the modifier key suggestion. This would allow us to reuse the same pattern in different places (box model properties, computed, flexbox etc., not a focus of this bug)

I could imagine a combination, e.g. an icon that shows up when you hold shift + hover as an indicator that "jump to source" is supported. This might help us with situations where there's no property to jump to.

Another thing is that a property value may not directly link to a source CSS rule.

I guess this also means that we need to take into account of the user has browser styles enabled or not.

Flags: needinfo?(mbalfanz)

It could be neat to continue the pattern of hovering over a value to highlight UI elsewhere - maybe it could work like this:

  • if a box model value such as border is set,
  • and user hovers over the gray border line+label+value inside the box model
    • Change the color of the border line in the box model to show it's interactable, and
    • Highlight the "border:1px solid black" in rules.
  • Click on the gray border area to scroll to and focus on the input field in Rules

What happens if the value is a computed value that's different from any one of the CSS declarations? Does it attempt to jump to a related Rule anyway?

If there's simply no matching Rule, I guess we could show a tooltip instead.

This makes the workflow no fewer steps than directly modifying the number in the box model, and is probably more useful than changing it in the box model. Though I could see a reason to still keep it there, if people like it.

I'd be interested in avoiding modifier keys if possible.

Flags: needinfo?(victoria)

I like the approach of highlighting the related properties in the rule view when hovering.

I also understand why you would want to avoid modifier keys, but here are drawbacks I see when looking at interactions with the areas:

  • The pattern is box model specific and not easily applicable to other tools. Modifiers are more universal. Maybe we could think about combinations?!
  • The border area is fairly small and easy to miss compared to margin and padding.
  • Clicking the border area to scroll to and focus on a single input field is only possible if the CSS was written using shorthands. But esp. for margins and paddings a very common use case is to override single values in a different rule. So, we would need 4 clickable areas to compensate for that. That's 12 clickable areas, plus 2 more if we think about width/height.

My TL;DR: Let's either try to be pragmatic and acknowledge who our target audience for this is, or let's try to be a little bit more universal as the functionality would be super helpful in many other places, too.

What happens if the value is a computed value that's different from any one of the CSS declarations? Does it attempt to jump to a related Rule anyway?

To me, this is a must. The box model converts everything to px, so "jumping to source" has to recognize origin rules.

Yes, I see what you mean about the issues here. As far as universality, I was actually hoping the simple hover+highlight behavior could be the main behavior across DevTools, but I haven't thought this through completely, so I'm sure there are areas where this may not work.

I see this idea as similar to the persistent picker highlight we've discussed before - just more direct connection between DevTools and the page.

I know the border design is kind of thin but we could widen the click box a bit, and I think the "border" label itself should be pretty clickable.

Attached video box-model-values.mp4

I've implemented a version where hovering over the box-model values would highlight AND scroll to the corresponding property in rules. If there isn't a property to scroll to then we show a tooltip.

Watching the video, I do find it a bit difficult to follow the scrolling when jumping to a property. It feels a little sudden. I think (as has already been suggested in comment 4) it might be better to click the editable box area to trigger scrolling?

What do you think about this Victoria?

Flags: needinfo?(victoria)

This looks very promising from the prototype. We should consider scrolling to the top of rule that defines the declaration we are hovering. In the video, it jumps to the padding-left declaration which is at the bottom of the rule. If we modified it to scroll to the rule selector or container and highlight the padding-left, we could see the entire context of the rule in which the declaration is defined in.

A little bit of history, we used to show the rule source information in the title attribute if there is an associated rule to one of the box model properties. I overlooked re-implementing this when I was converting the box model into react/redux. This was actually 2 years ago today while you were also a UCOSP student https://bugzilla.mozilla.org/show_bug.cgi?id=1333561. I bring this up since we are showing both the HTMLTooltip and title, which might be something to consider for UX. You can see how we used to put source information in the title in https://hg.mozilla.org/integration/mozilla-inbound/file/934733d09f10/devtools/client/inspector/components/deprecated-box-model.js#l770. You would have to go install Firefox 53 to see the old title.

Ahhhh, this is so cool!! I love the smooth scroll, the hover colors, and the faster visibility of the HTMLTooltip. I actually dislike the OS title for this type of important use case because it's so slow. Not sure if we need title tooltips for the "padding-left" etc.

Agree with Gabe about scrolling to the top of the rule. This could be tricky if the rule is bigger than the scroll area though - we'd want to always have the relevant line showing.

One thing - the orange highlight color is pretty garish - could you try Yellow 50 40% opacity?
Also it would be nice to lighten up the blue hover color in the content box.

Flags: needinfo?(victoria)

Thanks for the demo, Micah! This looks really good already :)

I agree with the previously mentioned points.

Coming back to your question about hover vs click: I agree with you that hovering feels a bit sudden. I would expect something on the side to just move when I work in the layout panel, without obvious interaction. I think we could give clicking a try and see how it feels?!

I imagine that a click would: scroll to the rule and automatically enter the "edit mode" for the value. If there is no direct rule, we would need to create the property in inline styles and have the "edit mode" on the value there (similar to what currently happens).

This would mean that we lose the direct editing in the box model, but we would gain editing in the right context.

I have 2 worries about the simple click:

  • the user will need to switch context / move eyes from one panel to the other. That might make the whole process slower / less efficient
  • we are changing a behavior that users are already used to, and we don't know yet if they will like it. Please make sure to put this behind a pref :-)

Thanks for the feedback everyone! It's great to see this feature is heading in the right direction :)

Martin, you have a good point about the simple click changing up existing behavior, which might be unfavorable for users. I agree moving forward we should hide clicking to scroll behind a pref until it's decided whether hovering/clicking behavior is best.

This patch allows hovering over box-model values and scrolling to their source CSS rule while pressing down the shift-key,

Another piece of UI this issue is introducing is a tooltip that displays a preview of the rule when hovering over a box-model value. It's an extension of the tooltip shown in attachment 9041659 [details] and the idea is to have this shown in other areas of the inspector where we would want to jump to a CSS rule (i.e: the Flexbox Inspector). To clarify, this tooltip appears when the cursor hovers over an editable box model value with a source CSS rule(and the shift key is not pressed). The footer of the tooltip would instead say: "Shift-hover to jump to rule".

I've attached an initial idea for what should be displayed in the tooltip if there is a CSS source rule to jump to. In this design, we're showing the rule and its properties. But I realize this might get messy if the rule is long and/or it contains several declarations.

Something that Victoria has brought up about this initial design is if the tooltip needs to be condensed a bit. Maybe we don't need to show the properties. Instead, we could show something like:

rule (in one font)
declaration (in another font/color)

I'll needinfo Martin and Victoria for more input on this.

Flags: needinfo?(mbalfanz)

Thanks Micah, this is great progress already!

Re: Tooltip content: I agree that the tooltip has a lot of content currently, and it can get more than in your example. The information that would make most sense to me would be around the value the developer is hovering. In your example, the tooltip for the left padding should show how the padding is defined in source (padding:0). This is esp. powerful if the unit is different and allows a quick way to compare calculated px from the box model vs. specified value, e.g. em. The surrounding declarations are imho less important and could be removed. This might also go for the selector of the rule.

For the interaction, I'm not sure I fully understand what you describe.

The footer of the tooltip would instead say: "Shift-hover to jump to rule".

So, to get to the tooltip I would hover the value, and to jump to rule I would just press the shift key? Or do you mean "Shift-click to jump to rule"?

Flags: needinfo?(mbalfanz)

In the interest of keeping this bug small and actionable, let's move the tooltip discussion to another bug. And let's focus here on jumping to the rule-view from the box-model, with a modifier key.
This is nice because it doesn't break current workflows, doesn't necessitate other discussions, allows Micah to land her work, and puts us one step closer to the final solution.
Also this blocks the WebCompat meta bug. So the fact that holding shift might make this undiscoverable isn't a problem here. This is a tool first and foremost for the WebCompat team, who we can very easily tell that this is how it works.
I've asked Micah to land her patch accordingly. So I'll file another bug for the tooltip discussion.

Blocks: 1528288

Those sound like great MVP decisions :D

Pushed by mtigley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d18e4e5cf3fa Link box-model values to their source CSS rules. r=gl
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Depends on: 1530621
Depends on: 1530711
Depends on: 1530823
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: