1316797 - Refactor HighlightersOverlay and TooltipsOverlay into separate modules

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

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

The purpose of this bug is to refactor style-inspector-overlays.js and separate out HighlightersOverlay and TooltipsOverlay into their own individual file/module. This will keep files smaller in size and easier to manage and edit. In addition, this will make it easier to add new features in HighlightersOverlay to take care of handling multiple grid highlighters on the page.

Comment 1

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95ce085c8590

Comment 2

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

Attachment: 1316797.patch [1.0]

Comment 3

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

Attachment: 1316797.patch [2.0]

Comment 4

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b1752d1f1d6

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8809744 [details] [diff] [review]
1316797.patch [2.0]

Review of attachment 8809744 [details] [diff] [review]:
-----------------------------------------------------------------

Nice refactor Gabriel! 
Works for me with my comments addressed and if try is green.

::: devtools/client/inspector/shared/highlighters-overlay.js
@@ +9,5 @@
>  // The style-inspector overlays are:
>  // - tooltips that appear when hovering over property values
>  // - editor tooltips that appear when clicking color swatches, etc.
>  // - in-content highlighters that appear when hovering over property values
>  // - etc.

Update this comment and remove references to the tooltip overlays.

@@ +47,5 @@
>    /**
>     * Add the highlighters overlay to the view. This will start tracking mouse
>     * movements and display highlighters when needed
>     */
> +  addToView() {

Not against the shorthand style, but it's inconsistent with the rest of the files in this folder. Unless there is a reason to use it only here, I would switch back to the previous style.

::: devtools/client/inspector/shared/tooltips-overlay.js
@@ +8,5 @@
>  
>  // The style-inspector overlays are:
>  // - tooltips that appear when hovering over property values
>  // - editor tooltips that appear when clicking color swatches, etc.
>  // - in-content highlighters that appear when hovering over property values

Update this comment and remove references to the highlighter overlays.

Comment 6

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

Attachment: 1316797.patch [3.0]

Fixed comments and reverted shorthands.

Comment 7

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/3530eb420c7f2ec8510304e7e30aee8fe07c23e7
Bug 1316797 - Refactor HighlightersOverlay and TooltipsOverlay into separate modules r=jdescottes

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3530eb420c7f