1398725 - Offset parent indicator can be confusing and could use a tooltip

Status: RESOLVED FIXED

(DevTools :: Inspector, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

It's very nice to have the offset parent indicator in the layout view, but it can be disturbing to see that if you've never heard about offset parent elements. 

And just querying "offset" on internet will not really help the user. With a simple tooltip such as "Offset parent of the selected element" it would already be more accessible.

Comment 1

[danielleleb12]

I'd be happy to try to create a patch - would it be through adding to the ComputedProperty class, and the BoxModelProperties.js getReferenceElement function? Or is there somewhere else I should start?

Comment 2

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

So, this should be for https://searchfox.org/mozilla-central/source/devtools/client/inspector/layout/components/ComputedProperty.js#59. We will want to add a title tooltip to that element which will represent the "offset" element.

Since we are adding a tooltip, we will also need to make sure the new tooltip label is localized. To do this, you will need to add a new entry to boxmodel.properties (https://searchfox.org/mozilla-central/source/devtools/client/locales/en-US/boxmodel.properties). You will need to add a new comment and key (something like boxmode.offsetParent.title). Afterwards, you can get that string by importing our localization helpers with that boxmodel.properties file. See https://searchfox.org/mozilla-central/source/devtools/client/inspector/boxmodel/components/BoxModelInfo.js#14-15.

As a bonus task, looking at ComputedProperty is only used in the box model. I think we should probably copy the component over to the boxmodel/components folder. If you choose to do this, you will need to add/remove the file entry in the folder's moz.build file. This tells us what files should be packaged/built. To copy the file, use "hg copy <path to file> <new path to file>". We want to use this command so we can preserve the version control history.

Thanks again for the help on these bugs!

Comment 3

[danielleleb12]

Hey :gl,

I'm working on moving the ComputerProperty into the boxmodel/components folder - I ran the command, but the file still appears to be in the layout/components folder. But then I get this error:

JavaScript error: resource://devtools/shared/base-loader.js, line 195: Error: Module resource://devtools/client/inspector/layout/components/ComputedProperty.js is not found at resource://devtools/client/inspector/layout/components/ComputedProperty.js

Am I supposed to do something else in addition to the hg copy command posted above?

Thanks in advance for the help!

Comment 4

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

(In reply to danielleleb12 from comment #3)

Hey :gl,

I'm working on moving the ComputerProperty into the boxmodel/components folder - I ran the command, but the file still appears to be in the layout/components folder. But then I get this error:

JavaScript error: resource://devtools/shared/base-loader.js, line 195: Error: Module resource://devtools/client/inspector/layout/components/ComputedProperty.js is not found at resource://devtools/client/inspector/layout/components/ComputedProperty.js

Am I supposed to do something else in addition to the hg copy command posted above?

Thanks in advance for the help!

Removing the ComputedProperty.js from the moz.build file in that directory should do the trick.

Comment 5

[danielleleb12]

I've removed the ComputedProperty.js from that directory's moz.build file and put it into the component directory's moz.build file. The ComputedProperty.js is still showing in the project tree as being in the layout/components folder - is that how it should still look after running the hg copy command?

Comment 6

[danielleleb12]

Never mind, the IRC channel helped me to fix the issue - BoxModelProperties.js also had a line requiring the ComputedProperty.js at its old path. That issue is fixed now, I'll push the diff. Thanks!

Comment 7

[danielleleb12]

Attachment: Bug 1398725 - Adds tooltip to offset parent indicator and moves ComputedProperty.js to boxmodel/components directory.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb76b90f829c
Adds tooltip to offset parent indicator and moves ComputedProperty.js to boxmodel/components directory. r=gl

Comment 9

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

Thanks you for the patch Danielle! I have went ahead with landing it.

Comment 10

[danielleleb12]

Awesome - thanks again for all of the help Gabriel!

Comment 11

[Stefan Hindli [:stefan_hindli]]

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