Skip to content

Commit

Permalink
[Start] Update triggerUrlFocusAnimation logic.
Browse files Browse the repository at this point in the history 
Before this CL, when calling triggerUrlFocusAnimation(), the value of
|hasFocus| is set as |!urlHasFocus()| sometimes, which is not easy to
understand. This CL makes the logic more understandable and keeps toolbar
expanded on start surface homepage.

Bug: 1184865
Change-Id: I5ebde44bc10fdec8a9674bdd4ae7f9c2d01b96d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2739915
Commit-Queue: Hao Dong <spdonghao@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Patrick Noland <pnoland@chromium.org>
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Reviewed-by: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#862218}
  • Loading branch information
spdonghao authored and Chromium LUCI CQ committed Mar 12, 2021
1 parent ce258c4 commit 2effd41
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 56 deletions.
14 changes: 8 additions & 6 deletions chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,15 +473,17 @@ public void setUrlFocusChangeInProgress(boolean inProgress) {
* change. This will be called after any animations are performed to transition from one
* focus state to the other.
*
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
* @param showExpandedState Whether the url bar is expanded.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value is determined by
* whether url bar has got focus. Most of the time this is the same as
* showExpandedState, but in some cases, e.g. url bar is scrolled to the top of the
* screen on homepage but not focused, we set it differently.
* @param shouldShowInOverviewMode Whether the location bar should be shown when in overview
* mode.
*/
public void finishUrlFocusChange(
boolean hasFocus, boolean shouldShowKeyboard, boolean shouldShowInOverviewMode) {
mLocationBarMediator.finishUrlFocusChange(hasFocus, shouldShowKeyboard);
public void finishUrlFocusChange(boolean showExpandedState, boolean shouldShowKeyboard,
boolean shouldShowInOverviewMode) {
mLocationBarMediator.finishUrlFocusChange(showExpandedState, shouldShowKeyboard);
if (shouldShowInOverviewMode) {
mStatusCoordinator.onSecurityStateChanged();
}
Expand Down
16 changes: 9 additions & 7 deletions chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarMediator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,17 +585,19 @@ public void set(LocationBarMediator object, Float value) {
* Handles any actions to be performed after all other actions triggered by the URL focus
* change. This will be called after any animations are performed to transition from one
* focus state to the other.
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
* @param showExpandedState Whether the url bar is expanded.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value is determined by
* whether url bar has got focus. Most of the time this is the same as
* showExpandedState, but in some cases, e.g. url bar is scrolled to the top of the
* screen on homepage but not focused, we set it differently.
*/
/* package */ void finishUrlFocusChange(boolean hasFocus, boolean shouldShowKeyboard) {
/* package */ void finishUrlFocusChange(boolean showExpandedState, boolean shouldShowKeyboard) {
if (mUrlCoordinator == null) return;
mUrlCoordinator.setKeyboardVisibility(hasFocus && shouldShowKeyboard, true);
mUrlCoordinator.setKeyboardVisibility(shouldShowKeyboard, true);
setUrlFocusChangeInProgress(false);
updateShouldAnimateIconChanges();
mStatusCoordinator.onUrlAnimationFinished(hasFocus);
if (!mIsTablet && !hasFocus) {
mStatusCoordinator.onUrlAnimationFinished(showExpandedState);
if (!mIsTablet && !showExpandedState) {
mLocationBarLayout.setUrlActionContainerVisibility(View.GONE);
}
}
Expand Down
6 changes: 3 additions & 3 deletions chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusCoordinator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ public void onUrlFocusChange(boolean urlHasFocus) {
updateVerboseStatusVisibility();
}

/** @param urlHasFocus Whether the url currently has focus. */
public void onUrlAnimationFinished(boolean urlHasFocus) {
mMediator.setUrlAnimationFinished(urlHasFocus);
/** @param showExpandedState Whether the url bar is expanded currently. */
public void onUrlAnimationFinished(boolean showExpandedState) {
mMediator.setUrlAnimationFinished(showExpandedState);
}

/** @param show Whether the status icon should be VISIBLE, otherwise GONE. */
Expand Down
11 changes: 7 additions & 4 deletions chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,12 @@ void setUrlHasFocus(boolean urlHasFocus) {
if (!mUrlHasFocus) updateLocationBarIconForDefaultMatchCategory(true);
}

// Extra logic to support extra NTP use cases which show the status icon when animating and when
// focused, but hide it when unfocused.
void setUrlAnimationFinished(boolean urlHasFocus) {
/**
* Extra logic to support extra NTP use cases which show the status icon when animating and when
* focused, but hide it when unfocused.
* @param showExpandedState Whether the url bar is expanded currently.
*/
void setUrlAnimationFinished(boolean showExpandedState) {
if (mIsTablet
|| !mSearchEngineLogoUtils.shouldShowSearchEngineLogo(
mLocationBarDataProvider.isIncognito())) {
Expand All @@ -291,7 +294,7 @@ void setUrlAnimationFinished(boolean urlHasFocus) {
// Hide the icon when the url unfocus animation finishes.
// Note: When mUrlFocusPercent is non-zero, that means we're still in the focused state from
// scrolling on the NTP.
if (!urlHasFocus && MathUtils.areFloatsEqual(mUrlFocusPercent, 0f)
if (!showExpandedState && MathUtils.areFloatsEqual(mUrlFocusPercent, 0f)
&& mSearchEngineLogoUtils.currentlyOnNTP(mLocationBarDataProvider)) {
setStatusIconShown(false);
}
Expand Down
72 changes: 36 additions & 36 deletions chrome/android/java/src/org/chromium/chrome/browser/toolbar/top/ToolbarPhone.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public class ToolbarPhone extends ToolbarLayout implements OnClickListener, TabC

@ViewDebug.ExportedProperty(category = "chrome")
protected int mTabSwitcherState;
private boolean mIsShowingStartSurface;

// This determines whether or not the toolbar draws as expected (false) or whether it always
// draws as if it's showing the non-tabswitcher, non-animating toolbar. This is used in grabbing
Expand Down Expand Up @@ -472,14 +473,14 @@ protected boolean handleEnterKeyPress() {
}
});

// Calls the {@link triggerUrlFocusAnimation()} here if it is skipped in {@link
// handleOmniboxInOverviewMode()}. In the overview mode focusing omnibox should also show
// keyboard.
// Calls the {@link triggerUrlFocusAnimation()} here to finish the pending focus request if
// it is skipped in {@link handleOmniboxInOverviewMode()}.
if (mPendingTriggerUrlFocusRequest) {
// This pending focus request must be from user's click on the fake omnibox on start
// surface before native library is ready.
assert getToolbarDataProvider().isInOverviewAndShowingOmnibox();
mPendingTriggerUrlFocusRequest = false;
boolean hasFocus =
getToolbarDataProvider().isInOverviewAndShowingOmnibox() && !urlHasFocus();
triggerUrlFocusAnimation(hasFocus, /* shouldShowKeyboard= */ hasFocus);
triggerUrlFocusAnimation(true);
}

updateVisualsForLocationBarState();
Expand Down Expand Up @@ -1859,6 +1860,7 @@ private void updateTabSwitcherButtonRipple() {
*/
private boolean handleOmniboxInOverviewMode(boolean inTabSwitcherMode) {
if (!getToolbarDataProvider().shouldShowLocationBarInOverviewMode()) return false;
mIsShowingStartSurface = inTabSwitcherMode;

if (mToggleTabStackButton != null) {
boolean isGone = inTabSwitcherMode;
Expand All @@ -1871,15 +1873,8 @@ private boolean handleOmniboxInOverviewMode(boolean inTabSwitcherMode) {
// possible that this function is called before native initialization when Instant Start
// is enabled. Keyboard shouldn't be shown here.
if (isNativeLibraryReady()) {
boolean hasFocus = inTabSwitcherMode && !urlHasFocus();
boolean shouldShowKeyboard = false;
// When switching out of the tab switcher and the url has got focused, we don't clear
// the focus.
if (!inTabSwitcherMode && urlHasFocus()) {
hasFocus = true;
shouldShowKeyboard = true;
}
triggerUrlFocusAnimation(hasFocus, shouldShowKeyboard);
// When the url has got focused, we don't clear the focus.
triggerUrlFocusAnimation(urlHasFocus());
} else {
mPendingTriggerUrlFocusRequest = true;
}
Expand Down Expand Up @@ -1981,7 +1976,7 @@ public void set(TextView view, Integer scrollX) {
};
}

private void populateUrlFocusingAnimatorSet(List<Animator> animators) {
private void populateUrlExpansionAnimatorSet(List<Animator> animators) {
TraceEvent.begin("ToolbarPhone.populateUrlFocusingAnimatorSet");
Animator animator = ObjectAnimator.ofFloat(this, mUrlFocusChangeFractionProperty, 1f);
animator.setDuration(URL_FOCUS_CHANGE_ANIMATION_DURATION_MS);
Expand Down Expand Up @@ -2036,7 +2031,7 @@ private void populateUrlFocusingAnimatorSet(List<Animator> animators) {
TraceEvent.end("ToolbarPhone.populateUrlFocusingAnimatorSet");
}

private void populateUrlClearFocusingAnimatorSet(List<Animator> animators) {
private void populateUrlClearExpansionAnimatorSet(List<Animator> animators) {
Animator animator = ObjectAnimator.ofFloat(this, mUrlFocusChangeFractionProperty, 0f);
animator.setDuration(URL_FOCUS_CHANGE_ANIMATION_DURATION_MS);
animator.setInterpolator(BakedBezierInterpolator.TRANSFORM_CURVE);
Expand Down Expand Up @@ -2096,21 +2091,24 @@ public void onUrlFocusChange(final boolean hasFocus) {
super.onUrlFocusChange(hasFocus);

if (mToggleTabStackButton != null) mToggleTabStackButton.setClickable(!hasFocus);

if (getToolbarDataProvider().isInOverviewAndShowingOmnibox()) {
mUrlBar.setText("");
if (!hasFocus) return;
}

triggerUrlFocusAnimation(hasFocus, /* shouldShowKeyboard= */ hasFocus);
triggerUrlFocusAnimation(hasFocus);
}

/**
* @param hasFocus Whether the URL field has gained focus.
* @param shouldShowKeyboard Whether the keyboard should be shown. This value should be the same
* as hasFocus by default.
* @param showExpandedState Whether the url bar should be expanded.
*/
private void triggerUrlFocusAnimation(final boolean hasFocus, boolean shouldShowKeyboard) {
private void triggerUrlFocusAnimation(boolean showExpandedState) {
boolean shouldShowKeyboard = urlHasFocus();

// Sometimes when it's exiting start surface and showing a normal tab,
// |getToolbarDataProvider().isInOverviewAndShowingOmnibox()| is not updated yet and still
// true. So we need to check |mIsEnteringStartSurface| here.
if (mIsShowingStartSurface) {
// If now it's on start surface, omnibox should be expanded without showing the keyboard
// even though it's not focus.
showExpandedState = true;
}

TraceEvent.begin("ToolbarPhone.triggerUrlFocusAnimation");
if (mUrlFocusLayoutAnimator != null && mUrlFocusLayoutAnimator.isRunning()) {
mUrlFocusLayoutAnimator.cancel();
Expand All @@ -2119,19 +2117,21 @@ private void triggerUrlFocusAnimation(final boolean hasFocus, boolean shouldShow
if (mOptionalButtonAnimationRunning) mOptionalButtonAnimator.end();

List<Animator> animators = new ArrayList<>();
if (hasFocus) {
populateUrlFocusingAnimatorSet(animators);
if (showExpandedState) {
populateUrlExpansionAnimatorSet(animators);
} else {
populateUrlClearFocusingAnimatorSet(animators);
populateUrlClearExpansionAnimatorSet(animators);
}
mUrlFocusLayoutAnimator = new AnimatorSet();
mUrlFocusLayoutAnimator.playTogether(animators);

mUrlFocusChangeInProgress = true;
// |showExpandedState| needs to be final when accessed within inner class.
final boolean showExpandedStateFinal = showExpandedState;
mUrlFocusLayoutAnimator.addListener(new CancelAwareAnimatorListener() {
@Override
public void onStart(Animator animation) {
if (!hasFocus) {
if (!showExpandedStateFinal) {
mDisableLocationBarRelayout = true;
} else {
mLayoutLocationBarInFocusedMode = true;
Expand All @@ -2141,19 +2141,19 @@ public void onStart(Animator animation) {

@Override
public void onCancel(Animator animation) {
if (!hasFocus) mDisableLocationBarRelayout = false;
if (!showExpandedStateFinal) mDisableLocationBarRelayout = false;

mUrlFocusChangeInProgress = false;
}

@Override
public void onEnd(Animator animation) {
if (!hasFocus) {
if (!showExpandedStateFinal) {
mDisableLocationBarRelayout = false;
mLayoutLocationBarInFocusedMode = false;
requestLayout();
}
mLocationBar.finishUrlFocusChange(hasFocus, shouldShowKeyboard,
mLocationBar.finishUrlFocusChange(showExpandedStateFinal, shouldShowKeyboard,
getToolbarDataProvider().shouldShowLocationBarInOverviewMode());
mUrlFocusChangeInProgress = false;
}
Expand Down Expand Up @@ -2303,7 +2303,7 @@ private void updateNtpAnimationState() {
if (mTabSwitcherState == STATIC_TAB && previousNtpScrollFraction > 0f) {
mUrlFocusChangeFraction =
Math.max(previousNtpScrollFraction, mUrlFocusChangeFraction);
triggerUrlFocusAnimation(/* hasFocus= */ false, /* shouldShowKeyboard= */ false);
triggerUrlFocusAnimation(false);
}
requestLayout();
}
Expand Down

0 comments on commit 2effd41

Please sign in to comment.