Commit 8037a7b1 authored by Brandon Wylie's avatar Brandon Wylie Committed by Commit Bot

Fix (i) icon or gap appearing when scrolling the NTP

The Google G (or other dse icon) should appear when scrolling the NTP.
This icon wasn't displaying because the hasFocus boolean isn't set
when scrolling. Adding plumbing to store the current focus percent in
StatusMediator and factoring that into the dse icon showing logic.

Bug: 1012508
Change-Id: Ia1f5a935ba5ceb8814000a520f482c661da716f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1854505
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Reviewed-by: default avatarEnder <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#706755}
parent 2b7edbc2
......@@ -208,6 +208,7 @@ public class LocationBarPhone extends LocationBarLayout {
}
updateButtonVisibility();
mStatusViewCoordinator.setUrlFocusChangePercent(percent);
}
@Override
......@@ -279,7 +280,6 @@ public class LocationBarPhone extends LocationBarLayout {
protected void updateButtonVisibility() {
super.updateButtonVisibility();
updateMicButtonVisibility(mUrlFocusChangePercent);
updateStatusButtonVisibility(mUrlFocusChangePercent);
}
@Override
......@@ -293,24 +293,6 @@ public class LocationBarPhone extends LocationBarLayout {
mStatusViewCoordinator.setShowIconsWhenUrlFocused(showIcon);
}
/**
* Updates the display of the status button.
*
* @param urlFocusChangePercent The completion percentage of the URL focus change animation.
*/
private void updateStatusButtonVisibility(float urlFocusChangePercent) {
if (mIconView == null
|| !SearchEngineLogoUtils.shouldShowSearchEngineLogo(
getToolbarDataProvider().isIncognito())) {
return;
}
if (mToolbarDataProvider.getNewTabPageForCurrentTab() != null
&& mToolbarDataProvider.getNewTabPageForCurrentTab().isLocationBarShownInNTP()) {
mIconView.setAlpha(urlFocusChangePercent);
}
}
/**
* @param softInputMode The software input resize mode.
* @param delay Delay the change in input mode.
......
......@@ -92,6 +92,7 @@ class StatusMediator {
private UrlBarEditingTextStateProvider mUrlBarEditingTextStateProvider;
private String mUrlBarTextWithAutocomplete = "";
private boolean mUrlBarTextIsValidUrl;
private float mUrlFocusPercent;
StatusMediator(PropertyModel model, Resources resources,
UrlBarEditingTextStateProvider urlBarEditingTextStateProvider) {
......@@ -231,6 +232,26 @@ class StatusMediator {
updateLocationBarIcon();
}
/**
* Set the url focus change percent.
* @param percent The current focus percent.
*/
void setUrlFocusChangePercent(float percent) {
mUrlFocusPercent = percent;
if (!SearchEngineLogoUtils.shouldShowSearchEngineLogo(
mToolbarCommonPropertiesModel.isIncognito())) {
return;
}
if (mToolbarCommonPropertiesModel.getNewTabPageForCurrentTab() != null
&& mToolbarCommonPropertiesModel.getNewTabPageForCurrentTab()
.isLocationBarShownInNTP()) {
mModel.set(StatusProperties.STATUS_ALPHA, percent);
}
updateLocationBarIcon();
}
/**
* Reports whether the first omnibox suggestion is a search query.
*/
......@@ -405,7 +426,8 @@ class StatusMediator {
boolean maybeUpdateStatusIconForSearchEngineIcon() {
// When the search engine logo should be shown, but the engine isn't Google. In this case,
// we download the icon on the fly.
boolean showFocused = mUrlHasFocus && mShowStatusIconWhenUrlFocused;
boolean showFocused =
(mUrlHasFocus || mUrlFocusPercent > 0) && mShowStatusIconWhenUrlFocused;
// Show the logo unfocused if "Query in the omnibox" is active or we're on the NTP. Current
// "Query in the omnibox" behavior makes it active for non-dse searches if you've just
// changed your default search engine.The included workaround below
......
......@@ -9,6 +9,7 @@ import android.view.View;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableFloatPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableIntPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;
......@@ -29,6 +30,9 @@ class StatusProperties {
static final WritableObjectPropertyKey<Bitmap> STATUS_ICON =
new WritableObjectPropertyKey<>(true);
/** Specifies the icon alpha. */
static final WritableFloatPropertyKey STATUS_ALPHA = new WritableFloatPropertyKey();
/** Specifies accessibility string presented to user upon long click on security icon. */
public static final WritableIntPropertyKey STATUS_ICON_ACCESSIBILITY_TOAST_RES =
new WritableIntPropertyKey();
......@@ -63,7 +67,7 @@ class StatusProperties {
public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {ANIMATIONS_ENABLED,
STATUS_ICON_ACCESSIBILITY_TOAST_RES, STATUS_ICON_RES, STATUS_ICON_TINT_RES, STATUS_ICON,
STATUS_ICON_DESCRIPTION_RES, SEPARATOR_COLOR_RES, STATUS_CLICK_LISTENER,
STATUS_ALPHA, STATUS_ICON_DESCRIPTION_RES, SEPARATOR_COLOR_RES, STATUS_CLICK_LISTENER,
VERBOSE_STATUS_TEXT_COLOR_RES, VERBOSE_STATUS_TEXT_STRING_RES,
VERBOSE_STATUS_TEXT_VISIBLE, VERBOSE_STATUS_TEXT_WIDTH, INCOGNITO_BADGE_VISIBLE};
......
......@@ -317,6 +317,13 @@ public class StatusView extends LinearLayout {
animateStatusIcon();
}
/** Specify the status icon alpha. */
void setStatusIconAlpha(float alpha) {
if (mIconView == null) return;
mIconView.setAlpha(alpha);
mIconView.setVisibility(alpha > 0f ? VISIBLE : GONE);
}
/**
* Specify accessibility string presented to user upon long click.
*/
......
......@@ -22,6 +22,8 @@ class StatusViewBinder implements ViewBinder<PropertyModel, StatusView, Property
view.setStatusIcon(model.get(StatusProperties.STATUS_ICON_RES));
} else if (StatusProperties.STATUS_ICON.equals(propertyKey)) {
view.setStatusIcon(model.get(StatusProperties.STATUS_ICON));
} else if (StatusProperties.STATUS_ALPHA.equals(propertyKey)) {
view.setStatusIconAlpha(model.get(StatusProperties.STATUS_ALPHA));
} else if (StatusProperties.STATUS_ICON_ACCESSIBILITY_TOAST_RES.equals(propertyKey)) {
view.setStatusIconAccessibilityToast(
model.get(StatusProperties.STATUS_ICON_ACCESSIBILITY_TOAST_RES));
......
......@@ -94,6 +94,14 @@ public class StatusViewCoordinator implements View.OnClickListener, TextWatcher
updateVerboseStatusVisibility();
}
/**
* Set the url focus change percent.
* @param percent The current focus percent.
*/
public void setUrlFocusChangePercent(float percent) {
mMediator.setUrlFocusChangePercent(percent);
}
/**
* @param useDarkColors Whether dark colors should be for the status icon and text.
*/
......
......@@ -76,6 +76,19 @@ public final class StatusMediatorUnitTest {
R.drawable.ic_logo_googleg_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
}
@Test
@Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
public void searchEngineLogo_showGoogleLogo_whenScrolled() {
setupSearchEngineLogoForTesting(true, false, false, false);
mMediator.setUrlHasFocus(false);
mMediator.setShowIconsWhenUrlFocused(true);
mMediator.setUrlFocusChangePercent(1f);
mMediator.updateSearchEngineStatusIcon(true, true, TEST_SEARCH_URL);
Assert.assertEquals(
R.drawable.ic_logo_googleg_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
}
@Test
@Features.EnableFeatures(ChromeFeatureList.OMNIBOX_SEARCH_ENGINE_LOGO)
public void searchEngineLogo_showGoogleLogo_searchLoupeEverywhere() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment