Closed Bug 1408205 Opened 4 years ago Closed 4 years ago

Send to Device icon in the Page Action Menu needs to be updated

Categories

(Firefox :: Theme, defect, P1)

58 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox56 --- unaffected
firefox57 --- wontfix
firefox58 --- verified

People

(Reporter: bbell, Assigned: mikedeboer)

References

Details

(Whiteboard: [reserve-photon-structure])

Attachments

(4 files)

Attached image example.png
The Icon for 'Send to Device' needs an update. See Example.
Whiteboard: [photon-structure]
Attached image send-to-device.svg
Icon needed.
Whiteboard: [photon-structure] → [photon-structure] [triage]
Assignee: nobody → mdeboer
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [photon-structure] [triage] → [reserve-photon-structure]
Comment on attachment 8920165 [details]
Bug 1408205 - Change the icon for the 'Send To Device' page-action.

https://reviewboard.mozilla.org/r/191162/#review196484

::: browser/themes/shared/icons/send-to-device.svg:4
(Diff revision 1)
> +<!-- 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" width="16" height="16" viewBox="0 0 16 16">

I think you should be able to remove the viewBox attribute here.
Attachment #8920165 - Flags: review+
Attached image Send to device in RTL
This should probably point the other way in RTL?
Comment on attachment 8920165 [details]
Bug 1408205 - Change the icon for the 'Send To Device' page-action.

https://reviewboard.mozilla.org/r/191162/#review196562

::: browser/themes/shared/urlbar-searchbar.inc.css:128
(Diff revision 1)
>  }
>  
>  .pageAction-sendToDevice-device[clientType=mobile] {
> -  list-style-image: url("chrome://browser/skin/device-mobile.svg");
> +  list-style-image: url("chrome://browser/skin/send-to-device.svg");
>  }
>  

nit: (or follow-up) this new icon is directional and should probably point to the left in RTL with a :-moz-locale-dir(rtl) rule.
Attachment #8920165 - Flags: review?(sfoster) → review+
Comment on attachment 8920165 [details]
Bug 1408205 - Change the icon for the 'Send To Device' page-action.

https://reviewboard.mozilla.org/r/191162/#review196484

> I think you should be able to remove the viewBox attribute here.

UX has stated that it's preferable to keep this attribute, because it makes (pre)viewing outside of Firefox easier.
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ec97e588099
Change the icon for the 'Send To Device' page-action. r=jaws,sfoster
(In reply to Sam Foster [:sfoster] from comment #5)
> nit: (or follow-up) this new icon is directional and should probably point
> to the left in RTL with a :-moz-locale-dir(rtl) rule.

Thanks for mentioning this! Fixed in the commit.
https://hg.mozilla.org/mozilla-central/rev/6ec97e588099
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Verified Windows 10 64bit, Mac OS X 10.13, Ubuntu 16.04 64bit using Nightly 58.0a1 (64-bit) using today's build of Nightly.
Component: Menus → Theme
Marking as Verified for 58 since 57 is marked as Wontfix.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.