1307193 - Move swatch based tooltip editors into their individual file from Tooltip.js

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

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

Tooltip.js is a pretty large file right now and can benefit from moving some of the Swatch classes into their own individual file.

Comment 1

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

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

Comment 2

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

Attachment: 1307193.patch [1.0]

Julian, the purpose of this bug is to just move the swatch based editors modules out of Tooltip.js into their own separate files so that it is more manageable.

Comment 3

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

Attachment: 1307193.patch [2.0]

Comment 4

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44ea3c20d7ef

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8797265 [details] [diff] [review]
1307193.patch [2.0]

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

Great, thank you for doing this!
It was sad having these html tooltips living in the same file as the (unrelated) XUL tooltip.

::: devtools/client/shared/widgets/SwatchBasedEditorTooltip.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

To preserve history, you could create this file using 

hg copy devtools/client/shared/widgets/Tooltip.js devtools/client/shared/widgets/tooltip/SwatchBasedEditorTooltip.js

And put it in the tooltip/ subfolder?

::: devtools/client/shared/widgets/SwatchColorPickerTooltip.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

To preserve history, you could create this file using 

hg copy devtools/client/shared/widgets/Tooltip.js devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js

And put it in the tooltip/ subfolder?

::: devtools/client/shared/widgets/SwatchCubicBezierTooltip.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

To preserve history, you could create this file using 

hg copy devtools/client/shared/widgets/Tooltip.js devtools/client/shared/widgets/tooltip/SwatchCubicBezierTooltip.js

And put it in the tooltip/ subfolder?

::: devtools/client/shared/widgets/SwatchFilterTooltip.js
@@ +1,1 @@
> +/* This Source Code Form is subject to the terms of the Mozilla Public

To preserve history, you could create this file using 

hg copy devtools/client/shared/widgets/Tooltip.js devtools/client/shared/widgets/tooltip/SwatchFilterTooltip.js

And put it in the tooltip/ subfolder?

Comment 6

[Julian Descottes [:jdescottes]]

I was looking at the remaining call sites for the XUL tooltip and we just have:
- 2 text tooltips in the debugger
- variable view tooltip in the debugger
- text tooltip in the shader editor

Migrating the variable view tooltip to the HTML tooltip seems like a big task and I'd prefer to let debugger html decide how to port this feature.

But we could migrate the shadereditor tooltip to HTML tooltip and then move the XUL tooltip to the debugger module. This way it would no longer pollute the widgets folder and would be removed automatically when the old debugger gets removed.

What do you think?

Comment 7

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

(In reply to Julian Descottes [:jdescottes] from comment #6)
> I was looking at the remaining call sites for the XUL tooltip and we just
> have:
> - 2 text tooltips in the debugger
> - variable view tooltip in the debugger
> - text tooltip in the shader editor
> 
> Migrating the variable view tooltip to the HTML tooltip seems like a big
> task and I'd prefer to let debugger html decide how to port this feature.
> 
> But we could migrate the shadereditor tooltip to HTML tooltip and then move
> the XUL tooltip to the debugger module. This way it would no longer pollute
> the widgets folder and would be removed automatically when the old debugger
> gets removed.
> 
> What do you think?

I think this is a good idea. I would like to add a new to bug to take care of this remaining work.

Comment 8

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

Attachment: 1307193.patch [3.0]

Comment 9

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

https://hg.mozilla.org/integration/fx-team/rev/47615f092e09d8d37d883ca72e35891b242a5d4a
Bug 1307193 - Move swatch based tooltip editors into their individual file from Tooltip.js r=jdescottes

Comment 10

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

https://hg.mozilla.org/mozilla-central/rev/47615f092e09