1265985 - Create toolbox icons for Firebug theme

Status: RESOLVED FIXED

(DevTools :: General, enhancement)

Description

[Sebastian Zartner [:sebo]]

The Firebug theme from bug 1244054 misses some theme-specific icons for the toolbox buttons.
This bug targets to add them.

Sebastian

Comment 1

[Sebastian Zartner [:sebo]]

Attachment: Split console icon

This icon was initially used by Firebug 2.

Sebastian

Comment 2

[Sebastian Zartner [:sebo]]

Attachment: Responsive Design Mode icon

Comment 3

[Sebastian Zartner [:sebo]]

Attachment: iframe Selector icon

Comment 4

[Sebastian Zartner [:sebo]]

These three icons are the ones shown by default.

I'll try to create the others before 48.0 reaches DevEdition, but those can already be integrated in the meantime.
Honza, can you do this?

Sebastian

Comment 5

[Sebastian Zartner [:sebo]]

Attachment: Eyedropper icon

Comment 6

[Sebastian Zartner [:sebo]]

Attachment: Highlight Painted Areas icon

Comment 7

[Sebastian Zartner [:sebo]]

Attachment: Scratchpad icon

Comment 8

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1265985-icons.patch

Attaching a patch that uses new icons in the Firebug theme

Missing icons:
* theme/images/firebug/command-screenshot.svg
* theme/images/firebug/command-rulers.svg
* theme/images/firebug/command-measure.svg

This one in only for the browser toolbox:
* theme/images/firebug/command-noautohide.svg


@Sebastian: command-eyedropper.svg is almost invisible in the background. I'll attach a screenshot.

@Helen: do you think we can commit just some icons or wait till all are available? Note that hitting 48 would be great.

@ntim: I created the black list for Firebug icons (you've suggested) and removed the 'filter: none' from toolbars.css variables. How does it look to you now?

Honza

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: eyedropper-screenshot.png

Comment 10

[Sebastian Zartner [:sebo]]

Attachment: Eyedropper icon

Made the borders a bit darker and added a filling. Let me know if it's not enough.

Sebastian

Comment 11

[Sebastian Zartner [:sebo]]

(In reply to Jan Honza Odvarko [:Honza] from comment #8)
> Missing icons:
> * theme/images/firebug/command-screenshot.svg
> * theme/images/firebug/command-rulers.svg
> * theme/images/firebug/command-measure.svg
> 
> This one in only for the browser toolbox:
> * theme/images/firebug/command-noautohide.svg

I'll try to get those today, so we still get them in time for 48.

Sebastian

Comment 12

[Sebastian Zartner [:sebo]]

Attachment: Screenshot icon

Comment 13

[Sebastian Zartner [:sebo]]

Attachment: Measure icon

Comment 14

[Sebastian Zartner [:sebo]]

Attachment: Rulers icon

Comment 15

[Sebastian Zartner [:sebo]]

Attachment: Disable popup auto-hide icon

Comment 16

[Sebastian Zartner [:sebo]]

Those should be all icons, so I'm removing the needinfo request for Helen.

Sebastian

Comment 17

[Helen V. Holmes (:helenvholmes) (:✨)]

(In reply to Sebastian Zartner [:sebo] from comment #16)
> Those should be all icons, so I'm removing the needinfo request for Helen.
> 
> Sebastian

Thanks Sebastian!

Comment 18

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1265985-icons.patch

Great, here is a new patch.
Helen, what do you think?

Honza

Comment 19

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: command-buttons.png

A screenshot of the UI in action.

Honza

Comment 20

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Thanks Helen!

Honza

Comment 21

[Tim Nguyen :ntim]

Comment on attachment 8743859 [details] [diff] [review]
bug1265985-icons.patch

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

Can you run all SVGs through SVGO please (and make sure to re-add the license header after that, since SVGO removes it) ?

Thanks !

::: devtools/client/inspector/inspector-panel.js
@@ +176,5 @@
> +      label: "test",
> +      callback: function () {
> +        Cmds.showTroubleShooting();
> +      }
> +    }];

Why is it needed ?

Also, this needs to be indented properly

::: devtools/client/themes/toolbars.css
@@ +73,3 @@
>    --tool-options-image: url(images/firebug/tool-options.svg);
>    --close-button-image: url(chrome://devtools/skin/images/firebug/close.svg);
> +  --icon-filter: invert(1);

Thanks for taking care of this here !

Comment 22

[Sebastian Zartner [:sebo]]

Attachment: Eyedropper icon

Comment 23

[Sebastian Zartner [:sebo]]

Attachment: Highlight painted areas icon

Comment 24

[Sebastian Zartner [:sebo]]

Attachment: iframe Selector icon

Comment 25

[Sebastian Zartner [:sebo]]

Attachment: Measure icon

Comment 26

[Sebastian Zartner [:sebo]]

Attachment: Disable popup auto-hide icon

Comment 27

[Sebastian Zartner [:sebo]]

Attachment: Responsive Design Mode icon

Comment 28

[Sebastian Zartner [:sebo]]

Attachment: Rulers icon

Comment 29

[Sebastian Zartner [:sebo]]

Attachment: Scratchpad icon

Comment 30

[Sebastian Zartner [:sebo]]

Attachment: Screenshot icon

Comment 31

[Sebastian Zartner [:sebo]]

Attachment: Split console icon

Comment 32

[Sebastian Zartner [:sebo]]

(In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #21)
> Comment on attachment 8743859 [details] [diff] [review]
> bug1265985-icons.patch
> 
> Review of attachment 8743859 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Can you run all SVGs through SVGO please (and make sure to re-add the
> license header after that, since SVGO removes it) ?

Done and attached the new files now.

Sebastian

Comment 33

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Attachment: bug1265985-icons.patch

(In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #21)
> Why is it needed ?
Ah, good catch, removed.

@Sebastian: thanks for the icon update. Also, not big deal, but if you use the same file names for images as in the tree, it's easier to deal with it.

New patch attached.
Honza

Comment 34

[Tim Nguyen :ntim]

Comment on attachment 8744194 [details] [diff] [review]
bug1265985-icons.patch

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

I have a new version of this patch that addresses both comments.

::: devtools/client/themes/firebug-theme.css
@@ +16,5 @@
> +
> +.theme-firebug #toolbox-dock-buttons > toolbarbutton > image,
> +.theme-firebug .devtools-closebutton > image,
> +.theme-firebug .devtools-option-toolbarbutton > image,
> +.theme-firebug .devtools-toolbarbutton > image,

This line should be removed, otherwise a lot of icons will have low contrast (clear/perf/network monitor icons/...).

::: devtools/client/themes/images/firebug/command-console.svg
@@ +1,4 @@
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="16" height="16"><defs><linearGradient id="a"><stop offset="0" stop-color="#325de6"/><stop offset="1" stop-color="#8ba3f1"/></linearGradient><linearGradient id="b"><stop offset="0" stop-color="#1a47d6"/><stop offset="1" stop-color="#6786ed"/></linearGradient><linearGradient x1="7.771" y1="13.61" x2="7.771" y2="2.16" id="e" xlink:href="#a" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.70047 0 0 .8 8.145 1037.962)"/><linearGradient x1="7.21" y1="14.919" x2="7.21" y2="1.081" id="f" xlink:href="#b" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.70047 0 0 .8 8.145 1037.962)"/><linearGradient id="c"><stop offset="0" stop-color="#282828"/><stop offset="1" stop-color="#505050"/></linearGradient><linearGradient id="d"><stop offset="0"/><stop offset="1" stop-color="#3c3c3c"/></linearGradient><linearGradient x1="7.771" y1="15.451" x2="7.771" y2="3.941" id="g" xlink:href="#c" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.9204 0 0 .56 15.804 1039.472)"/><linearGradient x1="7.21" y1="16.411" x2="7.21" y2="3.037" id="h" xlink:href="#d" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.9204 0 0 .56 15.804 1039.472)"/></defs><g stroke-linejoin="round"><path d="M1.14 1039.162l4.56 5.167-4.56 5.233-.84-.8 3.166-4.433-3.166-4.367z" fill="url(#e)" stroke="url(#f)" stroke-width=".6" transform="translate(0 -1036.362)"/><path d="M5.688 1040.05v1.125h10.125v-1.125H5.687zm2.5 3.75v1.125h7.624v-1.125H8.189zm-2.5 3.75v1.125h10.125v-1.125H5.687z" style="marker:none" color="#000" fill="url(#g)" stroke="url(#h)" stroke-width=".4" overflow="visible" transform="translate(0 -1036.362)"/></g></svg>
\ No newline at end of file

The SVGs are probably *too* optimised here. Anyway, I have a new version of this patch that prettifies up those SVGs (and sets the indent to 2 spaces on other SVGs)

Comment 35

[Tim Nguyen :ntim]

Attachment: bug1265985-icons.patch

Here's the patch with both comments addressed.

Comment 36

[Tim Nguyen :ntim]

Attachment: bug1265985-icons.patch

Now properly inverts icons

Comment 37

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8744263 [details] [diff] [review]
bug1265985-icons.patch

Looks good to me now.

You can land it, thanks!

Honza

Comment 38

[Pulsebot]

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

Comment 39

[Sebastian Zartner [:sebo]]

(In reply to Jan Honza Odvarko [:Honza] from comment #33)
> Created attachment 8744194 [details] [diff] [review]
> bug1265985-icons.patch
> 
> (In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #21)
> > Why is it needed ?
> Ah, good catch, removed.
> 
> @Sebastian: thanks for the icon update. Also, not big deal, but if you use
> the same file names for images as in the tree, it's easier to deal with it.

I'll keep that in mind next time. (There are still icons throughout the UI, which may be replaced by custom ones.)

(In reply to Tim Nguyen [:ntim] (busy, email me instead) from comment #34)
> ::: devtools/client/themes/images/firebug/command-console.svg
> @@ +1,4 @@
> > +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> > +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> > +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> > +<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="16" height="16"><defs><linearGradient id="a"><stop offset="0" stop-color="#325de6"/><stop offset="1" stop-color="#8ba3f1"/></linearGradient><linearGradient id="b"><stop offset="0" stop-color="#1a47d6"/><stop offset="1" stop-color="#6786ed"/></linearGradient><linearGradient x1="7.771" y1="13.61" x2="7.771" y2="2.16" id="e" xlink:href="#a" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.70047 0 0 .8 8.145 1037.962)"/><linearGradient x1="7.21" y1="14.919" x2="7.21" y2="1.081" id="f" xlink:href="#b" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.70047 0 0 .8 8.145 1037.962)"/><linearGradient id="c"><stop offset="0" stop-color="#282828"/><stop offset="1" stop-color="#505050"/></linearGradient><linearGradient id="d"><stop offset="0"/><stop offset="1" stop-color="#3c3c3c"/></linearGradient><linearGradient x1="7.771" y1="15.451" x2="7.771" y2="3.941" id="g" xlink:href="#c" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.9204 0 0 .56 15.804 1039.472)"/><linearGradient x1="7.21" y1="16.411" x2="7.21" y2="3.037" id="h" xlink:href="#d" gradientUnits="userSpaceOnUse" gradientTransform="matrix(-.9204 0 0 .56 15.804 1039.472)"/></defs><g stroke-linejoin="round"><path d="M1.14 1039.162l4.56 5.167-4.56 5.233-.84-.8 3.166-4.433-3.166-4.367z" fill="url(#e)" stroke="url(#f)" stroke-width=".6" transform="translate(0 -1036.362)"/><path d="M5.688 1040.05v1.125h10.125v-1.125H5.687zm2.5 3.75v1.125h7.624v-1.125H8.189zm-2.5 3.75v1.125h10.125v-1.125H5.687z" style="marker:none" color="#000" fill="url(#g)" stroke="url(#h)" stroke-width=".4" overflow="visible" transform="translate(0 -1036.362)"/></g></svg>
> \ No newline at end of file
> 
> The SVGs are probably *too* optimised here. Anyway, I have a new version of
> this patch that prettifies up those SVGs (and sets the indent to 2 spaces on
> other SVGs)

It was the first time I used SVGO, so bear with me. Thanks for updating the patch accordingly!

I'd say that was a very good teamwork! :-)

Sebastian

Comment 43

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0361b7030d36