Closed Bug 1348718 Opened 4 years ago Closed 4 years ago

Tweak images of ActionBar in CustomTabsActivity

Categories

(Firefox for Android :: General, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed

People

(Reporter: walkingice, Assigned: walkingice)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

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
You need to log in before you can comment on or make changes to this bug.