Closed Bug 1366669 Opened 8 years ago Closed 7 years ago

(photon) update icons

Categories

(Firefox for Android Graveyard :: General, enhancement)

Product:
Firefox for Android Graveyard
Component:
General
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox56 fixed, firefox57 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 56
Tracking Flags:
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: wesley_huang, Assigned: jwu)

References

Details

Attachments

(3 files)

Bug 1366669 - Part 1: Rename images and move to Australis folder.
(deleted), text/x-review-board-request
walkingice
: review+
Details
Bug 1366669 - Part 2: Update icons for Photon.
(deleted), text/x-review-board-request
walkingice
: review+
Details
Bug 1366669 - Part 3: Check correct resource folder according to current skin.
(deleted), text/x-review-board-request
nalexander
: review+
walkingice
: review+
Details
No description provided.
Blocks: fennec-photon-misc_ui
Comment on attachment 8886076 [details] Bug 1366669 - Part 3: Check correct resource folder according to current skin. https://reviewboard.mozilla.org/r/156876/#review162184 I have some nits, and one minor request, but this doesn't need re-review. ::: commit-message-e2e47:1 (Diff revision 1) > +Bug 1366669 - Part 3: Check correct resource folder accroding to current skin. r?nalexander,walkingice nit: s/accroding/according/ ::: commit-message-e2e47:3 (Diff revision 1) > +Bug 1366669 - Part 3: Check correct resource folder accroding to current skin. r?nalexander,walkingice > + > +Check images in photon/res or australis/res based on `--enable-photon` is set or not in mozconfig. nit: based on whether `--enable-photon` is set ... (missing whether) ::: mobile/android/base/locales/Makefile.in:98 (Diff revision 1) > # L10NBASEDIR is not defined for en-US. > l10n-srcdir := $(if $(filter en-US,$(AB_CD)),,$(or $(realpath $(L10NBASEDIR)),$(abspath $(L10NBASEDIR)))/$(AB_CD)/mobile/chrome) > > $(eval $(call generated_file_template,suggestedsites,suggestedsites.json)) > > +ifdef MOZ_ANDROID_PHOTON We're not consistent (yet), but prefer to use lower-case names for local variables and upper-case names for global variables. Also, in this case you can use `$(if)`, I think -- can you try: ``` skin := $(if $(MOZ_ANDROID_PHOTON),photon,australis) ```
Attachment #8886076 - Flags: review?(nalexander) → review+
Comment on attachment 8886074 [details] Bug 1366669 - Part 1: Rename images and move to Australis folder. https://reviewboard.mozilla.org/r/156872/#review162386 ::: mobile/android/app/src/main/res/drawable/close_edit_mode_selector.xml:10 (Diff revision 2) > > <selector xmlns:android="http://schemas.android.com/apk/res/android" > xmlns:gecko="http://schemas.android.com/apk/res-auto"> > > <item gecko:state_dark="true" > - android:drawable="@drawable/close_edit_mode_dark"/> > + android:drawable="@drawable/ic_cancel_nm"/> I guess we can just remove list line since lightweight theme is not awared. ::: mobile/android/app/src/main/res/drawable/close_edit_mode_selector.xml:15 (Diff revision 2) > - android:drawable="@drawable/close_edit_mode_dark"/> > + android:drawable="@drawable/ic_cancel_nm"/> > > <item gecko:state_private="true" > - android:drawable="@drawable/close_edit_mode_light"/> > + android:drawable="@drawable/ic_cancel_pm"/> > > - <item android:drawable="@drawable/close_edit_mode_light"/> > + <item android:drawable="@drawable/ic_cancel_pm"/> by default using *pm*?
Attachment #8886074 - Flags: review?(walkingice0204) → review+
Comment on attachment 8886075 [details] Bug 1366669 - Part 2: Update icons for Photon. https://reviewboard.mozilla.org/r/156874/#review162392
Attachment #8886075 - Flags: review?(walkingice0204) → review+
Comment on attachment 8886076 [details] Bug 1366669 - Part 3: Check correct resource folder according to current skin. https://reviewboard.mozilla.org/r/156876/#review162394
Attachment #8886076 - Flags: review?(walkingice0204) → review+
Assignee: nobody → topwu.tw
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/41cdb9589102 Part 1: Rename images and move to Australis folder. r=walkingice https://hg.mozilla.org/integration/autoland/rev/d97403d6b52d Part 2: Update icons for Photon. r=walkingice https://hg.mozilla.org/integration/autoland/rev/2b56008c121a Part 3: Check correct resource folder according to current skin. r=nalexander,walkingice
Keywords: checkin-needed
Depends on: 1381501
Blocks: 1387349
Verified as fixed on Nightly 57. Icons have been updated following the design.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: