Offset parent indicator can be confusing and could use a tooltip

RESOLVED FIXED in Firefox 67

Status

enhancement
P3
normal
RESOLVED FIXED
2 years ago
3 months ago

People

(Reporter: jdescottes, Assigned: danielleleb12, Mentored)

Tracking

({good-first-bug})

unspecified
Firefox 67

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(1 attachment)

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.
Assignee: nobody → gl
Status: NEW → ASSIGNED
Priority: -- → P3

Updated

Last year
Product: Firefox → DevTools
Assignee: gl → nobody
Status: ASSIGNED → NEW
Mentor: gl
Keywords: good-first-bug
Assignee

Comment 1

4 months ago

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?

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!

Assignee: nobody → danielleleb12
Assignee

Comment 3

4 months ago

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: Moduleresource://devtools/client/inspector/layout/components/ComputedProperty.jsis 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!

(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: Moduleresource://devtools/client/inspector/layout/components/ComputedProperty.jsis 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.

Assignee

Comment 5

3 months ago

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?

Assignee

Comment 6

3 months ago

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 8

3 months ago
Pushed by gabriel.luong@gmail.com:
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

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

Assignee

Comment 10

3 months ago

Awesome - thanks again for all of the help Gabriel!

Comment 11

3 months ago
bugherder
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.