770818 - [inspector] highlighter v3

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Paul Rouget [:paul]]

We should only show the veil when:
- the user is hovering the page to select a node
- the user is selecting nodes in the breadcrumbs
- the user is hovering the infobar
- the user is using the Markup View

That might mean that we also need to change the "look" of the selection (the dashed rectangle around the selected node).

Comment 2

[Dils]

Attachment: UX Spec for 770818

Comment 3

[Dils]

See the attached JPG for recommended UI changes.

Paul and I  discussed a few other, more aggressive changes to the UI, but since the UX team is working on a new UI structure for all the dev tools as a whole, I didn't want to recommend features that could end up being changed after a big development effort.

Just so we can keep track, here are some future UX ideas I want to explore for the Inspector:

#1: When the user is hover over the selected element, a zoomed in view of the element can be seen in the layout view with the measurements overlaid on top

#2 Add a button for a zoom tool to the Node Info UI

#3 Add zoom tool to the Inspector toolbar

#4 When the user is hovering over the various parts of the element in layout view, such as margin, border, etc the corresponding areas on the live element on the page are highlighted while the rest of the page is dimmed.

Comment 4

[Paul Rouget [:paul]]

(In reply to Dils from comment #3)
> See the attached JPG for recommended UI changes.

I don't have any comment on this. Looks right to me.

> #1: When the user is hover over the selected element, a zoomed in view of
> the element can be seen in the layout view with the measurements overlaid on
> top
> 
> #2 Add a button for a zoom tool to the Node Info UI
> 
> #3 Add zoom tool to the Inspector toolbar
> 
> #4 When the user is hovering over the various parts of the element in layout
> view, such as margin, border, etc the corresponding areas on the live
> element on the page are highlighted while the rest of the page is dimmed.

We need specific bugs for that. Also, let's not discuss this on bugzilla (we prefer ml discussion for feature definition).

See bug 731652 and bug 663778 tool.

Comment 5

[Paul Rouget [:paul]]

Brian, any good reason to keep the veil?

Comment 6

[Dils]

I mean the veil looks great, but I don't see needing both the veil and the dashed border. I can see it working well for idea #4 listed above...

Comment 7

[Paul Rouget [:paul]]

Brian, please test this build (we need to wait a bit before the builds the show up):
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-61dc79b156a8

Changes:
- no veil
- removed the gear menu
- outline is "strong" while highlighter unlocked. outline is faded when locked.
- outline is hidden when the user overs the sidebar.
- infobar bar is moved up and faded out when the user overs the sidebar.

Fading out the infobar feels weird. It works if there's no content behind it, but if there is some text or any image, it looks messy: http://i.imgur.com/9NEjZ.png

Comment 8

[Dils]

Thoughts on the changes:

- no veil
  * I think this works well, I'd say keep this setting

- removed the gear menu
  * Works for me. Lets pay attention to whether users complain about the veil disappearing

- outline is "strong" while highlighter unlocked. outline is faded when locked.
  * Works for me as well
- outline is hidden when the user overs the sidebar.
  * I think works too

- infobar bar is moved up and faded out when the user overs the sidebar.
  * I think we should either slide the infobar away or use fading. Personally I really like the sliding effect you built. I'd suggesting keeping the sliding action and not fading the infobar

Comment 9

[Paul Rouget [:paul]]

(In reply to Dils from comment #8)
> - removed the gear menu
>   * Works for me. Lets pay attention to whether users complain about the
>     veil disappearing

Ok. I'd like to have another approval for that.
Kevin, what do you think?

> - infobar bar is moved up and faded out when the user overs the sidebar.
>   * I think we should either slide the infobar away or use fading.
>     Personally I really like the sliding effect you built. I'd suggesting
>     keeping the sliding action and not fading the infobar

Understood.

Comment 10

[Paul Rouget [:paul]]

Just talked to Kevin:
1) we get rid of the veil
2) we get rid of the gear-menu
3) we totally hide the infobar while editing the style of the page

About 3), people want to have a "clean" page while editing code. Not having a way to remove the infobar (because we remove the gear-menu) might my a real problem.

I'll bake something, and make a build available, and we'll see how it feels.

Comment 11

[Kevin Dangoor]

(In reply to Paul Rouget [:paul] from comment #9)
> (In reply to Dils from comment #8)
> > - removed the gear menu
> >   * Works for me. Lets pay attention to whether users complain about the
> >     veil disappearing
> 
> Ok. I'd like to have another approval for that.
> Kevin, what do you think?

I tried the build out. I very much like the removal of the gear menu, and I like the behavior when I hover over the style panel. Feedback that we've gotten (which partly led to the gear menu in the first place) is that designers want to see their designs without any distractions while they're trying to figure out what changes to make, so I think the infobar should go away as well when styling.

Paul and I discussed this a bit on IRC. Paul said he could try using the animation in this build to fade out the infobar entirely rather than just fading it to some low opacity.

The veil is visually distinctive and has certainly gotten attention for our tools. However, it does get in the way for designers. Paul also tried fading out the veil, but described that variation as "annoying". The veil is a bit too heavy to fade in and out frequently.

Eliminating modes while keeping things useful for everyone seems like a win. bug 663778 will be a useful addition to this, because then the node will have a bit more highlighting to it than just a small dashed line.

tldr; my opinion: overall good, make the infobar fade away completely rather than mostly

Comment 12

[Paul Rouget [:paul]]

Attachment: v0.1

build to come

Comment 13

[Paul Rouget [:paul]]

Mac build will be available here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-681e9b18fde0

Screencast:
http://www.youtube.com/watch?v=aePy2NpFtyI

Comment 14

[Paul Rouget [:paul]]

Todo:
- tests

Comment 15

[Paul Rouget [:paul]]

Attachment: v1

with tests

Comment 16

[Paul Rouget [:paul]]

Todo:
- some code has been deleted. I need to make sure nothing is left in locales/ & themes/
- try

Comment 17

[Paul Rouget [:paul]]

Attachment: v1.1

Comment 18

[Paul Rouget [:paul]]

This patch goes beyond just removing the veil:

- removed the option menu in the toolbar
- removed the veil
- hide the infobar + outline the mouse is overring the rule view
- only one reflow when moving the selection (performance++)
- ignore page events while inspecting (performance+++++)
- inversed outline contrast: before it was low contrast while inspecting and high contrast once locked. Now it's the other way around.

Comment 19

[Paul Rouget [:paul]]

Comment on attachment 652050 [details] [diff] [review]
v1.1

Rob, can you try this patch and tell me what you think about this new way to highlight nodes.

And a quick code review won't hurt :)

Comment 20

[Paul Rouget [:paul]]

https://tbpl.mozilla.org/?tree=Try&rev=6171571117e3

Comment 21

[Dão Gottwald [:dao]]

Comment on attachment 652050 [details] [diff] [review]
v1.1

> <!-- LOCALIZATION NOTE: These are for the menu in the Inspector Toolbar -->
> <!ENTITY inspectorToggleVeil.label          "Dim The Page">
> <!ENTITY inspectorToggleVeil.accesskey      "D">
> <!ENTITY inspectorToggleInfobar.label       "Show Node Info">
> <!ENTITY inspectorToggleInfobar.accesskey   "S">

What's the plan for these strings?

Comment 22

[Paul Rouget [:paul]]

(In reply to Dão Gottwald [:dao] from comment #21)
> Comment on attachment 652050 [details] [diff] [review]
> v1.1
> 
> > <!-- LOCALIZATION NOTE: These are for the menu in the Inspector Toolbar -->
> > <!ENTITY inspectorToggleVeil.label          "Dim The Page">
> > <!ENTITY inspectorToggleVeil.accesskey      "D">
> > <!ENTITY inspectorToggleInfobar.label       "Show Node Info">
> > <!ENTITY inspectorToggleInfobar.accesskey   "S">
> 
> What's the plan for these strings?

I knew I'd miss something :) Thank you Dao.

Comment 24

[Paul Rouget [:paul]]

Attachment: v1.2

Comment 25

[Paul Rouget [:paul]]

Attachment: v1.2

rebased

Comment 26

[Paul Rouget [:paul]]

Just realized there's an orange with WinXP.

Comment 27

[Paul Rouget [:paul]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=beeeef68ee29

Comment 28

[Paul Rouget [:paul]]

Apparently, it's fixed.
Ready to land!

Comment 29

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/cbaa56e4bebb

Comment 30

[Paul Rouget [:paul]]

Orange.

I'll see if there's an obvious fix. Otherwise, I'll back this out.

Comment 31

[Paul Rouget [:paul]]

Backed out:  https://hg.mozilla.org/integration/fx-team/rev/aeb96f8edcb8

Comment 32

[Paul Rouget [:paul]]

Can't reproduce here. I'll use try to experiment some ideas.

First round: https://tbpl.mozilla.org/?tree=Try&rev=99e162ee6721

Comment 34

[Paul Rouget [:paul]]

Apparently, it worked. Another try with all the platforms: https://tbpl.mozilla.org/?tree=Try&rev=8e4bfc8f0c7f

Comment 35

[Paul Rouget [:paul]]

It's green. Let's reland that.

Comment 36

[Paul Rouget [:paul]]

https://hg.mozilla.org/integration/fx-team/rev/c4d19217db8d

Comment 37

[Paul Rouget [:paul]]

https://hg.mozilla.org/mozilla-central/rev/c4d19217db8d