Closed Bug 1348718 Opened 8 years ago Closed 8 years ago

Tweak images of ActionBar in CustomTabsActivity

Categories

(Firefox for Android Graveyard :: General, enhancement)

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

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: walkingice, Assigned: walkingice)

References

Details

Attachments

(3 files)

Bug 1348718 - Use cross icon as close button
59 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1348718 - Build option menu button in same way
59 bytes, text/x-review-board-request
sebastian
: review+
Details
Bug 1348718 - Remove useless method
59 bytes, text/x-review-board-request
sebastian
: review+
Details 
According to Material Design and requirements from TDC designers, some images of ActionBar in CustomTabsActivity need to be adjust

* Home button: change from Arrow-image to Cross(Close)-image
* Image size of Action-button and Menu-option-button
Comment on attachment 8855780 [details]
Bug 1348718 - Use cross icon as close button

https://reviewboard.mozilla.org/r/127676/#review130398

::: mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java:152
(Diff revision 1)
>          return mTextPrimaryColor;
>      }
>  
> +    private void initIndicator() {
> +        mActionBar.setDisplayHomeAsUpEnabled(true);
> +        Drawable indicator = mActionBar.getThemedContext().getDrawable(R.drawable.ic_close_light);

nit: final

::: mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java:154
(Diff revision 1)
>  
> +    private void initIndicator() {
> +        mActionBar.setDisplayHomeAsUpEnabled(true);
> +        Drawable indicator = mActionBar.getThemedContext().getDrawable(R.drawable.ic_close_light);
> +        DrawableCompat.setTint(indicator, mTextPrimaryColor);
> +        mActionBar.setHomeAsUpIndicator(indicator);

I saw that there's a custom tabs API for setting a custom icon: setCloseButtonIcon(). Are we going to support this? Do we have a bug filed for that already?
Attachment #8855780 - Flags: review?(s.kaspari) → review+
Comment on attachment 8855781 [details]
Bug 1348718 - Build option menu button in same way

https://reviewboard.mozilla.org/r/127678/#review130400

https://www.youtube.com/watch?v=9jK-NcRmVcw :)

::: mobile/android/base/java/org/mozilla/gecko/customtabs/ActionBarPresenter.java:133
(Diff revision 1)
> +        int size = res.getDimensionPixelSize(R.dimen.custom_tab_action_button_size);
> +        int padding = res.getDimensionPixelSize(R.dimen.custom_tab_action_button_padding);

final final :)

::: mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java:218
(Diff revision 1)
>      public boolean onCreatePanelMenu(final int id, final Menu menu) {
> -        insertActionButton(menu, startIntent, actionBarPresenter.getTextPrimaryColor());
>  
> -        popupMenu = createCustomPopupMenu();
> +        // if 3rd-party app asks to add an action button
> +        if (IntentUtil.hasActionButton(startIntent)) {
> +            Bitmap bitmap = IntentUtil.getActionButtonIcon(startIntent);

final?
Attachment #8855781 - Flags: review?(s.kaspari) → review+
Comment on attachment 8855782 [details]
Bug 1348718 - Remove useless method

https://reviewboard.mozilla.org/r/127680/#review130402
Attachment #8855782 - Flags: review?(s.kaspari) → review+
Just saw that there's a test failure:
> :app:compileAutomationDebugUnitTestJavaWithJavac/home/worker/workspace/build/src/mobile/android/tests/background/junit4/src/org/mozilla/gecko/customtabs/TestCustomTabsActivity.java:124: error: cannot find symbol
Comment on attachment 8855780 [details]
Bug 1348718 - Use cross icon as close button

https://reviewboard.mozilla.org/r/127676/#review130398

> I saw that there's a custom tabs API for setting a custom icon: setCloseButtonIcon(). Are we going to support this? Do we have a bug filed for that already?

Cannot believe I missed this method. Already file a bug for this(Bug 1354766), thanks!
Keywords: checkin-needed
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/500fcd4b01fb
Use cross icon as close button r=sebastian
https://hg.mozilla.org/integration/autoland/rev/0aca46064d80
Build option menu button in same way r=sebastian
https://hg.mozilla.org/integration/autoland/rev/aa6ffa5c5b2c
Remove useless method r=sebastian
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/500fcd4b01fb
https://hg.mozilla.org/mozilla-central/rev/0aca46064d80
https://hg.mozilla.org/mozilla-central/rev/aa6ffa5c5b2c
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
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: