1247331 - Import snapshot button should be an icon

Status: VERIFIED FIXED

(DevTools :: Memory, defect, P2)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Maybe one of those box-with-arrow-going-in things?

       |
    +- | -+
    |  V  |
    +-----+

Maybe sideways instead of down?

    +---+
    | <---
    +---+

Thoughts?

Comment 1

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

I see the "down" icon a lot; let's go with that even if sideways makes sense for us in particular.

Comment 2

[Tim Nguyen :ntim]

I would rather use an up icon since it's more commonly used for uploading (importing action) while the down icon is used more for downloading (exporting action).

Comment 3

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

Attachment: upload-expanded.svg

Comment 4

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

Attachment: import-snapshot.patch

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #2)
> I would rather use an up icon since it's more commonly used for uploading
> (importing action) while the down icon is used more for downloading
> (exporting action).

So, a quick google search revealed that "upload icon" was an upward pointing arrow pretty much unanimously.

"Import icon" had arrows pointing in all four directions.

So I went with what Tim suggested, since it seemed like it had /slightly/ less ambiguity. Kinda.

Comment 5

[Tim Nguyen :ntim]

Comment on attachment 8770365 [details] [diff] [review]
import-snapshot.patch

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

The icon looks great! I am seeing the bottom stroke of the import icon a bit blurry, it seems like it's not perfectly pixel snapped. I'll upload a new version as it should be trivial to fix (the bottom edge is apart only from 0.08 pixel from the pixel grid).

Love the new diff icon, perfectly pixel snapped, and matches better the new icon style. Thanks for updating it.

Past that, it seems like the Style Editor and the Canvas tools have an import button, can you change these to use the new icon ?

Comment 6

[Tim Nguyen :ntim]

Attachment: import pixel snapped.svg

Comment 7

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

Attachment: upload-icon.patch

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #5)
> I'll upload a new
> version as it should be trivial to fix (the bottom edge is apart only from
> 0.08 pixel from the pixel grid).
Added!

> Past that, it seems like the Style Editor and the Canvas tools have an
> import button, can you change these to use the new icon ?
Added these as well. Wasn't sure if /where/ I added the rules in the CSS makes sense—since you're more familiar with the CSS in general, can you advise if there's a better place for those rules to live?

Comment 8

[Tim Nguyen :ntim]

Comment on attachment 8770676 [details] [diff] [review]
upload-icon.patch

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

Looks good to me! I'll land this with my comment applied, and bug 1260523 with it as well.

::: devtools/client/themes/styleeditor.css
@@ +12,5 @@
> +}
> +
> +.style-editor-importButton .toolbarbutton-text {
> +  display: none;
> +}

it seems like both of these rules are not needed (.toolbarbutton-icon and .toolbarbutton-text)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/006a4f4e6852
Make import snapshot an icon,r=ntim

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/006a4f4e6852

Comment 11

[Nazir Ahmed Sabbir [:NaSb]]

I've managed this issue on this bug in Nightly 47.0a1 (2016-02-10) ; (Build ID: 20160210071115) from Linux, 64 Bit.

This Bug is now verified as fixed on Latest Firefox Nightly 50.0a1 (2016-07-21) 

Build ID: 20160721030216 
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0