Closed Bug 1147661 Opened 5 years ago Closed 5 years ago

Update "Send to devices" icons

Categories

(Firefox for Android :: Overlays, defect)

x86
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 39
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: antlam, Assigned: mcomella)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 1 obsolete file)

Attached file device_mob_PC.zip
Updating device icons from the old ones. They're really quite light ATM.
Attached image Screenshot
Attachment #8583916 - Flags: feedback?(alam)
/r/6149 - Bug 1147661 - Add new device assets. r=liuche
/r/6151 - Bug 1147661 - Use new device icons in share overlay. r=liuche

Pull down these commits:

hg pull review -r 056298dee4b10da39bdd05dee44d9da4f327074e
Attachment #8583917 - Flags: review?(liuche)
Comment on attachment 8583916 [details]
Screenshot

Makes me happy :D
Attachment #8583916 - Flags: feedback?(alam) → feedback+
https://reviewboard.mozilla.org/r/6151/#review5145

::: mobile/android/base/overlays/ui/SendTabDeviceListArrayAdapter.java
(Diff revision 1)
> -            return R.drawable.sync_mobile_inactive;
> +            return R.drawable.device_mobile;

What about other uses of synce_mobile_inactive?
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/RemoteTabsExpandableListAdapter.java#157

If it's because the contrast between active/inactive  isn't enough, that's also fine (but if you do end up removing all the references, a reminder to remove the resource too)
https://reviewboard.mozilla.org/r/6149/#review5143

Nit: maybe rename these to device_desktop instead of pc
Assignee: nobody → michael.l.comella
Comment on attachment 8583917 [details]
MozReview Request: bz://1147661/mcomella

<finkle.gif>
Attachment #8583917 - Flags: review?(liuche) → review+
https://reviewboard.mozilla.org/r/6151/#review5173

> What about other uses of synce_mobile_inactive?
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/RemoteTabsExpandableListAdapter.java#157
> 
> If it's because the contrast between active/inactive  isn't enough, that's also fine (but if you do end up removing all the references, a reminder to remove the resource too)

The other uses are in other widgets that want don't want to change (e.g. the remote tabs panel).
Comment on attachment 8583917 [details]
MozReview Request: bz://1147661/mcomella

Approval Request Comment
[Feature/regressing bug #]: bug 1130203
[User impact if declined]:
  Users will see the old device icons which are a different color than the ones we'd like (light grey as opposed to dark grey, which matches the font color), i.e. polish. 

[Describe test coverage new/current, TreeHerder]: None
[Risks and why]: 
  Low - we add new assets and replace the previous usage in the code with the new names. Worst case - we use the wrong assets.

[String/UUID change made/needed]: None
Attachment #8583917 - Flags: approval-mozilla-aurora?
Verified as fixed using:
Device: Nexus 5 (Android 5.0)
Build: Firefox for Android 39.0a1 (2015-03-27)
Attachment #8583917 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8583917 [details]
MozReview Request: bz://1147661/mcomella

This missed the uplift to Aurora. It'll need to go through the Beta approval process.
Attachment #8583917 - Flags: approval-mozilla-aurora+
Comment on attachment 8583917 [details]
MozReview Request: bz://1147661/mcomella

Approval Request Comment 11
Attachment #8583917 - Flags: approval-mozilla-beta?
Attachment #8583917 - Flags: approval-mozilla-aurora?
Attachment #8583917 - Flags: approval-mozilla-aurora?
Attachment #8583917 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Should be in 38 beta 2
Verified as fixed on Beta 38.0b3 on Sony Xperia (4.4.4)
Status: RESOLVED → VERIFIED
Attachment #8583917 - Attachment is obsolete: true
Attachment #8619875 - Flags: review+
Attachment #8619876 - Flags: review+
You need to log in before you can comment on or make changes to this bug.