Skip to content

Commit

Permalink
Add a shadow to the follow accelerator button.
Browse files Browse the repository at this point in the history
Due to optical illusions, the shadow using android:elevation did
not show up since the button was a darker color.  Instead we now use
a 9-patch based approach.


Bug: b/188184619
Change-Id: I303f08c87d8b9c0eb20a56993f5c5a99401f8d2a

be able to do the shadow.  I have followed the instructions at
and run tools/resources/optimize-png-files.sh to make the bitmaps
as small as possible.

Binary-Size: Size increase is unavoidable.  We have added bitmaps to
https: //chromium.googlesource.com/chromium/src/+/HEAD/docs/speed/binary_size/optimization_advice.md#optimizing-images
Change-Id: I303f08c87d8b9c0eb20a56993f5c5a99401f8d2a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3072888
Commit-Queue: Peter Williamson <petewil@chromium.org>
Reviewed-by: Cathy Li <chili@chromium.org>
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#914970}
  • Loading branch information
Pete Williamson authored and Chromium LUCI CQ committed Aug 24, 2021
1 parent 1a848e1 commit b1733c7
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 22 deletions.
6 changes: 6 additions & 0 deletions chrome/browser/feed/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ android_library("java") {
"java/src/org/chromium/chrome/browser/feed/VideoPreviewsType.java",
"java/src/org/chromium/chrome/browser/feed/settings/FeedAutoplaySettingsFragment.java",
"java/src/org/chromium/chrome/browser/feed/settings/RadioButtonGroupVideoPreviewsPreference.java",
"java/src/org/chromium/chrome/browser/feed/webfeed/ShadowedClickableTextBubble.java",
"java/src/org/chromium/chrome/browser/feed/webfeed/WebFeedBridge.java",
"java/src/org/chromium/chrome/browser/feed/webfeed/WebFeedDialogContents.java",
"java/src/org/chromium/chrome/browser/feed/webfeed/WebFeedDialogCoordinator.java",
Expand Down Expand Up @@ -54,6 +55,7 @@ android_library("java") {
"//third_party/androidx:androidx_annotation_annotation_java",
"//third_party/androidx:androidx_appcompat_appcompat_resources_java",
"//third_party/androidx:androidx_browser_browser_java",
"//third_party/androidx:androidx_core_core_java",
"//third_party/androidx:androidx_preference_preference_java",
"//ui/android:ui_no_recycler_view_java",
"//url:gurl_java",
Expand All @@ -65,6 +67,10 @@ android_library("java") {
android_resources("web_feed_java_resources") {
sources = [
"java/res/color/menu_footer_chip_background.xml",
"java/res/drawable-hdpi/follow_accelerator_shadow.9.png",
"java/res/drawable-mdpi/follow_accelerator_shadow.9.png",
"java/res/drawable-xhdpi/follow_accelerator_shadow.9.png",
"java/res/drawable-xxhdpi/follow_accelerator_shadow.9.png",
"java/res/drawable/web_feed_post_follow_illustration.xml",
"java/res/layout/feed_management_activity.xml",
"java/res/layout/feed_management_list_item.xml",
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,27 +1,36 @@
// Copyright 2021 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.components.browser_ui.widget.textbubble;

package org.chromium.chrome.browser.feed.webfeed;

import android.content.Context;
import android.graphics.drawable.Drawable;
import android.view.View;

import androidx.annotation.DrawableRes;
import androidx.annotation.StringRes;
import androidx.core.content.res.ResourcesCompat;

import org.chromium.components.browser_ui.widget.R;
import org.chromium.components.browser_ui.widget.textbubble.TextBubble;
import org.chromium.ui.widget.LoadingView;
import org.chromium.ui.widget.RectProvider;

/**
* Provides a text bubble with a dark shadow to support a medium dark color. We need a darker
* shadow than other images such as navigation_bubble_shadow provide to prevent the optical illusion
* of the shadow being a part of the bitmap. However, if we add the images for the new shadow to
* ClickableTextBubble, they also get used by WebLayer, which doesn't need this shadow. To prevent
* increasing the size of the chrome APK, we include the bitmaps and the class to use them here with
* the other feed code.
*
* UI component that handles showing a clickable text callout bubble.
*
* <p>This has special styling specific to clickable text bubbles:
* <ul>
* <li>No arrow
* <li>Rounder corners
* <li>Smaller padding
* //TODO(sophey): Implement shadow once 9-patches are available.
* <li>Shadow
* <li>Optional loading UI
* </ul>
Expand All @@ -34,7 +43,7 @@
* element). Example below:
*
* <pre>{@code
* ClickableTextBubble clickableTextBubble;
* ShadowedClickableTextBubble clickableTextBubble;
* OnTouchListener onTouchListener = (view, motionEvent) -> {
* performPotentiallyLongRequest();
* clickableTextBubble.showLoadingUI(loadingViewContentDescriptionId);
Expand All @@ -49,11 +58,13 @@
* }
* }</pre>
*/
public class ClickableTextBubble extends TextBubble {
public class ShadowedClickableTextBubble extends TextBubble {
private final Context mContext;
private final LoadingView mLoadingView;
private final boolean mInverseColor;

/**
* Constructs a {@link ClickableTextBubble} instance.
* Constructs a {@link ShadowedClickableTextBubble} instance.
*
* @param context Context to draw resources from.
* @param rootView The {@link View} to use for size calculations and for display.
Expand All @@ -65,17 +76,27 @@ public class ClickableTextBubble extends TextBubble {
* text and dismiss UX.
* @param onTouchListener The callback for all touch events being dispatched to the bubble.
*/
public ClickableTextBubble(Context context, View rootView, @StringRes int stringId,
public ShadowedClickableTextBubble(Context context, View rootView, @StringRes int stringId,
@StringRes int accessibilityStringId, RectProvider anchorRectProvider,
@DrawableRes int imageDrawableId, boolean isAccessibilityEnabled,
View.OnTouchListener onTouchListener) {
View.OnTouchListener onTouchListener, boolean inverseColor) {
super(context, rootView, stringId, accessibilityStringId, /*showArrow=*/false,
anchorRectProvider, imageDrawableId, /*isRoundBubble=*/true, /*inverseColor=*/false,
isAccessibilityEnabled);
anchorRectProvider, imageDrawableId, /*isRoundBubble=*/true,
/*inverseColor=*/inverseColor, isAccessibilityEnabled);
mContext = context;
mInverseColor = inverseColor;
setTouchInterceptor(onTouchListener);
mLoadingView = mContentView.findViewById(R.id.loading_view);
}

/** Get the backgound to use. We use a color button with a dark shadow. */
@Override
public Drawable getBackground(Context context, boolean showArrow, boolean isRoundBubble) {
Drawable background = ResourcesCompat.getDrawable(
context.getResources(), R.drawable.follow_accelerator_shadow, null);
return background;
}

/**
* Replaces image with loading spinner. Dismisses the entire button when loading spinner is
* hidden.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.chromium.chrome.browser.user_education.UserEducationHelper;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.components.browser_ui.widget.highlight.ViewHighlighter;
import org.chromium.components.browser_ui.widget.textbubble.ClickableTextBubble;
import org.chromium.components.browser_ui.widget.textbubble.TextBubble;
import org.chromium.components.feature_engagement.FeatureConstants;
import org.chromium.components.feature_engagement.Tracker;
Expand Down Expand Up @@ -45,7 +44,7 @@ class WebFeedFollowIntroView {
private final Tracker mFeatureEngagementTracker;
private final Runnable mIntroDismissedCallback;

private ClickableTextBubble mFollowBubble;
private ShadowedClickableTextBubble mFollowBubble;
private final int mShowTimeoutMillis;

/**
Expand Down Expand Up @@ -76,9 +75,10 @@ void showAccelerator(View.OnTouchListener onTouchListener, Runnable introShownCa
return;
}

mFollowBubble = new ClickableTextBubble(mActivity, mMenuButtonAnchorView,
mFollowBubble = new ShadowedClickableTextBubble(mActivity, mMenuButtonAnchorView,
R.string.menu_follow, R.string.menu_follow, createRectProvider(), R.drawable.ic_add,
ChromeAccessibilityUtil.get().isAccessibilityEnabled(), onTouchListener);
ChromeAccessibilityUtil.get().isAccessibilityEnabled(), onTouchListener,
/*inverseColor*/ false);
mFollowBubble.addOnDismissListener(this::introDismissed);
// TODO(crbug/1152592): Figure out a way to dismiss on outside taps as well.
mFollowBubble.setAutoDismissTimeout(mShowTimeoutMillis);
Expand Down
1 change: 0 additions & 1 deletion components/browser_ui/widget/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ android_library("java") {
"java/src/org/chromium/components/browser_ui/widget/text/TextViewWithCompoundDrawables.java",
"java/src/org/chromium/components/browser_ui/widget/text/VerticallyFixedEditText.java",
"java/src/org/chromium/components/browser_ui/widget/textbubble/ArrowBubbleDrawable.java",
"java/src/org/chromium/components/browser_ui/widget/textbubble/ClickableTextBubble.java",
"java/src/org/chromium/components/browser_ui/widget/textbubble/TextBubble.java",
"java/src/org/chromium/components/browser_ui/widget/tile/TileView.java",
"java/src/org/chromium/components/browser_ui/widget/tile/TileViewBinder.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@
android:layout_height="wrap_content"
android:textAppearance="@style/TextAppearance.TextMediumThick.Primary.Inverse" />

</LinearLayout>
</LinearLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,11 @@ public class TextBubble implements AnchoredPopupWindow.LayoutObserver {
private final AnchoredPopupWindow mPopupWindow;

/** The {@link Drawable} that is responsible for drawing the bubble and the arrow. */
private final ArrowBubbleDrawable mBubbleDrawable;
@Nullable
private ArrowBubbleDrawable mBubbleDrawable;

/** The {@link Drawable} that precedes the text in the bubble. */
private final Drawable mImageDrawable;
protected final Drawable mImageDrawable;

private final Runnable mDismissRunnable = new Runnable() {
@Override
Expand Down Expand Up @@ -253,8 +254,9 @@ public TextBubble(Context context, View rootView, String contentString,
mInverseColor = inverseColor;
mIsAccessibilityEnabled = isAccessibilityEnabled;

mBubbleDrawable = new ArrowBubbleDrawable(context, isRoundBubble);
mBubbleDrawable.setShowArrow(showArrow);
// For round, arrowless bubbles, we use a specialized background instead of the
// ArrowBubbleDrawable.
Drawable backgroundDrawable = getBackground(mContext, showArrow, isRoundBubble);

mContentView = createContentView();
// On some versions of Android, the LayoutParams aren't set until after the popup window
Expand All @@ -264,7 +266,7 @@ public TextBubble(Context context, View rootView, String contentString,
new FrameLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT));

mPopupWindow = new AnchoredPopupWindow(
context, rootView, mBubbleDrawable, mContentView, anchorRectProvider);
context, rootView, backgroundDrawable, mContentView, anchorRectProvider);
mPopupWindow.setMargin(
context.getResources().getDimensionPixelSize(R.dimen.text_bubble_margin));
mPopupWindow.setPreferredHorizontalOrientation(
Expand All @@ -277,15 +279,21 @@ public TextBubble(Context context, View rootView, String contentString,

addOnDismissListener(mDismissListener);
if (mIsAccessibilityEnabled) setDismissOnTouchInteraction(true);
}

/** Get the background to use. May be overridden by subclasses. */
protected Drawable getBackground(Context context, boolean showArrow, boolean isRoundBubble) {
mBubbleDrawable = new ArrowBubbleDrawable(mContext, isRoundBubble);
mBubbleDrawable.setShowArrow(showArrow);
// Set predefined styles for the TextBubble.
if (inverseColor) {
if (mInverseColor) {
mBubbleDrawable.setBubbleColor(ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.default_bg_color));
} else {
mBubbleDrawable.setBubbleColor(ApiCompatibilityUtils.getColor(
mContext.getResources(), R.color.default_control_color_active));
}
return mBubbleDrawable;
}

/** Shows the bubble. Will have no effect if the bubble is already showing. */
Expand Down Expand Up @@ -399,6 +407,9 @@ public void setPreferredVerticalOrientation(
@Override
public void onPreLayoutChange(
boolean positionBelow, int x, int y, int width, int height, Rect anchorRect) {
// mBubbleDrawable might not be in use if a subclass replaces the drawable.
if (mBubbleDrawable == null) return;

int arrowXOffset = 0;
if (mBubbleDrawable.isShowingArrow()) {
arrowXOffset = anchorRect.centerX() - x;
Expand Down

0 comments on commit b1733c7

Please sign in to comment.