Commit 0c0a2f90 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Remove LocationBar/UrlBar references from SuggestionView.

This converts the custom view tracking logic with just static/
explicit offsets.  This view no longer is under the crazy development
that made the custom tracking somewhat reasonable.  It is now just
overly complicated and more difficult to reason about.  Phone converted
to static offsets a while ago, so this unifies it with tablets.

BUG=883143

Change-Id: I6076501bee54f052a372be97aaab97686f44497b
Reviewed-on: https://chromium-review.googlesource.com/1228985
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Reviewed-by: default avatarPedro Amaral <amaralp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592993}
parent 5fe10abf
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
<dimen name="tab_strip_and_toolbar_height">96dp</dimen> <dimen name="tab_strip_and_toolbar_height">96dp</dimen>
<dimen name="location_bar_icon_width">40dp</dimen> <dimen name="location_bar_icon_width">40dp</dimen>
<dimen name="omnibox_suggestion_start_offset">@dimen/location_bar_icon_width</dimen>
<!-- NTP dimensions --> <!-- NTP dimensions -->
<dimen name="ntp_search_box_transition_length">60dp</dimen> <dimen name="ntp_search_box_transition_length">60dp</dimen>
......
...@@ -287,13 +287,11 @@ ...@@ -287,13 +287,11 @@
<!-- omnibox_suggestion_answer_line2_vertical_spacing + 2dp --> <!-- omnibox_suggestion_answer_line2_vertical_spacing + 2dp -->
<dimen name="omnibox_suggestion_answer_image_vertical_spacing">5dp</dimen> <dimen name="omnibox_suggestion_answer_image_vertical_spacing">5dp</dimen>
<dimen name="omnibox_suggestion_answer_image_horizontal_spacing">4dp</dimen> <dimen name="omnibox_suggestion_answer_image_horizontal_spacing">4dp</dimen>
<dimen name="omnibox_suggestion_phone_url_bar_left_offset">10dp</dimen> <dimen name="omnibox_suggestion_start_offset">18dp</dimen>
<dimen name="omnibox_suggestion_phone_url_bar_left_offset_rtl">10dp</dimen>
<dimen name="omnibox_suggestion_refine_width">48dp</dimen> <dimen name="omnibox_suggestion_refine_width">48dp</dimen>
<dimen name="omnibox_suggestion_text_vertical_padding">5dp</dimen> <dimen name="omnibox_suggestion_text_vertical_padding">5dp</dimen>
<dimen name="omnibox_suggestion_multiline_text_vertical_padding">10dp</dimen> <dimen name="omnibox_suggestion_multiline_text_vertical_padding">10dp</dimen>
<dimen name="omnibox_suggestion_refine_view_modern_end_padding">4dp</dimen> <dimen name="omnibox_suggestion_refine_view_modern_end_padding">4dp</dimen>
<dimen name="omnibox_suggestion_list_modern_offset">8dp</dimen>
<!-- NTP dimensions --> <!-- NTP dimensions -->
<dimen name="tile_grid_layout_max_width">504dp</dimen> <dimen name="tile_grid_layout_max_width">504dp</dimen>
......
...@@ -165,15 +165,6 @@ public interface LocationBar extends UrlBarDelegate { ...@@ -165,15 +165,6 @@ public interface LocationBar extends UrlBarDelegate {
*/ */
void setDefaultTextEditActionModeCallback(ToolbarActionModeCallback callback); void setDefaultTextEditActionModeCallback(ToolbarActionModeCallback callback);
/**
* Returns whether the {@link UrlBar} must be queried for its location on screen when
* suggestions are being laid out by {@link SuggestionView}.
* TODO(dfalcantara): Revisit this after M58.
*
* @return Whether or not the {@link UrlBar} has to be explicitly checked for its location.
*/
boolean mustQueryUrlBarLocationForSuggestions();
/** /**
* @return Whether suggestions are being shown for the location bar. * @return Whether suggestions are being shown for the location bar.
*/ */
......
...@@ -21,6 +21,7 @@ import android.support.annotation.DrawableRes; ...@@ -21,6 +21,7 @@ import android.support.annotation.DrawableRes;
import android.support.annotation.IntDef; import android.support.annotation.IntDef;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.support.v4.view.MarginLayoutParamsCompat; import android.support.v4.view.MarginLayoutParamsCompat;
import android.support.v4.view.ViewCompat;
import android.support.v7.content.res.AppCompatResources; import android.support.v7.content.res.AppCompatResources;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.AttributeSet; import android.util.AttributeSet;
...@@ -406,7 +407,7 @@ public class LocationBarLayout extends FrameLayout ...@@ -406,7 +407,7 @@ public class LocationBarLayout extends FrameLayout
mUrlCoordinator.setDelegate(this); mUrlCoordinator.setDelegate(this);
mSuggestionItems = new ArrayList<OmniboxResultItem>(); mSuggestionItems = new ArrayList<OmniboxResultItem>();
mSuggestionListAdapter = new OmniboxResultsAdapter(getContext(), this, mSuggestionItems); mSuggestionListAdapter = new OmniboxResultsAdapter(getContext(), mSuggestionItems);
mMicButton = (TintedImageButton) findViewById(R.id.mic_button); mMicButton = (TintedImageButton) findViewById(R.id.mic_button);
...@@ -471,11 +472,8 @@ public class LocationBarLayout extends FrameLayout ...@@ -471,11 +472,8 @@ public class LocationBarLayout extends FrameLayout
mUrlCoordinator.setUrlDirectionListener(new UrlBar.UrlDirectionListener() { mUrlCoordinator.setUrlDirectionListener(new UrlBar.UrlDirectionListener() {
@Override @Override
public void onUrlDirectionChanged(int layoutDirection) { public void onUrlDirectionChanged(int layoutDirection) {
ApiCompatibilityUtils.setLayoutDirection(LocationBarLayout.this, layoutDirection); ViewCompat.setLayoutDirection(LocationBarLayout.this, layoutDirection);
updateSuggestionListLayoutDirection();
if (mSuggestionList != null) {
mSuggestionList.updateSuggestionsLayoutDirection(layoutDirection);
}
} }
}); });
} }
...@@ -958,6 +956,7 @@ public class LocationBarLayout extends FrameLayout ...@@ -958,6 +956,7 @@ public class LocationBarLayout extends FrameLayout
updateButtonVisibility(); updateButtonVisibility();
mSuggestionListAdapter.setToolbarDataProvider(toolbarDataProvider);
mUrlCoordinator.setOnFocusChangedCallback(this::onUrlFocusChange); mUrlCoordinator.setOnFocusChangedCallback(this::onUrlFocusChange);
} }
...@@ -1303,6 +1302,11 @@ public class LocationBarLayout extends FrameLayout ...@@ -1303,6 +1302,11 @@ public class LocationBarLayout extends FrameLayout
public View getAnchorView() { public View getAnchorView() {
return getRootView().findViewById(R.id.toolbar); return getRootView().findViewById(R.id.toolbar);
} }
@Override
public View getAlignmentView() {
return mIsTablet ? LocationBarLayout.this : null;
}
}; };
// TODO(tedchoc): Investigate lazily building the suggestion list off of the UI thread. // TODO(tedchoc): Investigate lazily building the suggestion list off of the UI thread.
try (StrictModeContext unused = StrictModeContext.allowDiskReads()) { try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
...@@ -1424,6 +1428,7 @@ public class LocationBarLayout extends FrameLayout ...@@ -1424,6 +1428,7 @@ public class LocationBarLayout extends FrameLayout
} }
mSuggestionList.show(); mSuggestionList.show();
updateSuggestionListLayoutDirection();
} else if (!visible && isShowing) { } else if (!visible && isShowing) {
mSuggestionList.setVisibility(GONE); mSuggestionList.setVisibility(GONE);
...@@ -1433,6 +1438,13 @@ public class LocationBarLayout extends FrameLayout ...@@ -1433,6 +1438,13 @@ public class LocationBarLayout extends FrameLayout
maybeShowOmniboxResultsContainer(); maybeShowOmniboxResultsContainer();
} }
private void updateSuggestionListLayoutDirection() {
if (mSuggestionList == null) return;
int layoutDirection = ViewCompat.getLayoutDirection(this);
mSuggestionList.updateSuggestionsLayoutDirection(layoutDirection);
mSuggestionListAdapter.setLayoutDirection(layoutDirection);
}
/** /**
* Updates the URL we will navigate to from suggestion, if needed. This will update the search * Updates the URL we will navigate to from suggestion, if needed. This will update the search
* URL to be of the corpus type if query in the omnibox is displayed and update aqs= parameter * URL to be of the corpus type if query in the omnibox is displayed and update aqs= parameter
...@@ -2165,11 +2177,6 @@ public class LocationBarLayout extends FrameLayout ...@@ -2165,11 +2177,6 @@ public class LocationBarLayout extends FrameLayout
@Override @Override
public void setShowTitle(boolean showTitle) { } public void setShowTitle(boolean showTitle) { }
@Override
public boolean mustQueryUrlBarLocationForSuggestions() {
return mIsTablet;
}
@Override @Override
public AutocompleteController getAutocompleteController() { public AutocompleteController getAutocompleteController() {
return mAutocomplete; return mAutocomplete;
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.omnibox; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.omnibox;
import android.content.Context; import android.content.Context;
import android.graphics.Bitmap; import android.graphics.Bitmap;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import android.support.v4.view.ViewCompat;
import android.text.TextUtils; import android.text.TextUtils;
import android.view.View; import android.view.View;
import android.view.ViewGroup; import android.view.ViewGroup;
...@@ -14,6 +15,7 @@ import android.widget.BaseAdapter; ...@@ -14,6 +15,7 @@ import android.widget.BaseAdapter;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
...@@ -27,17 +29,16 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -27,17 +29,16 @@ public class OmniboxResultsAdapter extends BaseAdapter {
private final List<OmniboxResultItem> mSuggestionItems; private final List<OmniboxResultItem> mSuggestionItems;
private final Context mContext; private final Context mContext;
private final LocationBar mLocationBar; private ToolbarDataProvider mDataProvider;
private OmniboxSuggestionDelegate mSuggestionDelegate; private OmniboxSuggestionDelegate mSuggestionDelegate;
private boolean mUseDarkColors = true; private boolean mUseDarkColors = true;
private Set<String> mPendingAnswerRequestUrls = new HashSet<>(); private Set<String> mPendingAnswerRequestUrls = new HashSet<>();
private int mLayoutDirection;
public OmniboxResultsAdapter( public OmniboxResultsAdapter(
Context context, Context context,
LocationBar locationBar,
List<OmniboxResultItem> suggestionItems) { List<OmniboxResultItem> suggestionItems) {
mContext = context; mContext = context;
mLocationBar = locationBar;
mSuggestionItems = suggestionItems; mSuggestionItems = suggestionItems;
} }
...@@ -66,17 +67,23 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -66,17 +67,23 @@ public class OmniboxResultsAdapter extends BaseAdapter {
if (convertView instanceof SuggestionView) { if (convertView instanceof SuggestionView) {
suggestionView = (SuggestionView) convertView; suggestionView = (SuggestionView) convertView;
} else { } else {
suggestionView = new SuggestionView(mContext, mLocationBar); suggestionView = new SuggestionView(mContext);
} }
OmniboxResultItem item = mSuggestionItems.get(position); OmniboxResultItem item = mSuggestionItems.get(position);
maybeFetchAnswerIcon(item); maybeFetchAnswerIcon(item);
suggestionView.init(item, mSuggestionDelegate, position, mUseDarkColors); suggestionView.init(item, mSuggestionDelegate, position, mUseDarkColors);
ViewCompat.setLayoutDirection(suggestionView, mLayoutDirection);
return suggestionView; return suggestionView;
} }
private void maybeFetchAnswerIcon(OmniboxResultItem item) { private void maybeFetchAnswerIcon(OmniboxResultItem item) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
// Attempting to fetch answer data before we have a profile to request it for.
if (mDataProvider == null) return;
// Do not refetch an answer image if it already exists. // Do not refetch an answer image if it already exists.
if (item.getAnswerImage() != null) return; if (item.getAnswerImage() != null) return;
OmniboxSuggestion suggestion = item.getSuggestion(); OmniboxSuggestion suggestion = item.getSuggestion();
...@@ -88,8 +95,8 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -88,8 +95,8 @@ public class OmniboxResultsAdapter extends BaseAdapter {
if (mPendingAnswerRequestUrls.contains(url)) return; if (mPendingAnswerRequestUrls.contains(url)) return;
mPendingAnswerRequestUrls.add(url); mPendingAnswerRequestUrls.add(url);
AnswersImage.requestAnswersImage(mLocationBar.getToolbarDataProvider().getProfile(), url, AnswersImage.requestAnswersImage(
new AnswersImage.AnswersImageObserver() { mDataProvider.getProfile(), url, new AnswersImage.AnswersImageObserver() {
@Override @Override
public void onAnswersImageChanged(Bitmap bitmap) { public void onAnswersImageChanged(Bitmap bitmap) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
...@@ -120,6 +127,13 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -120,6 +127,13 @@ public class OmniboxResultsAdapter extends BaseAdapter {
} }
} }
/**
* Sets the data provider for the toolbar.
*/
public void setToolbarDataProvider(ToolbarDataProvider provider) {
mDataProvider = provider;
}
/** /**
* Set the selection delegate for suggestion entries in the adapter. * Set the selection delegate for suggestion entries in the adapter.
* *
...@@ -129,6 +143,14 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -129,6 +143,14 @@ public class OmniboxResultsAdapter extends BaseAdapter {
mSuggestionDelegate = delegate; mSuggestionDelegate = delegate;
} }
/**
* Sets the layout direction to be used for any new suggestion views.
* @see View#setLayoutDirection(int)
*/
public void setLayoutDirection(int layoutDirection) {
mLayoutDirection = layoutDirection;
}
/** /**
* @return The selection delegate for suggestion entries in the adapter. * @return The selection delegate for suggestion entries in the adapter.
*/ */
......
...@@ -16,7 +16,6 @@ import android.view.View; ...@@ -16,7 +16,6 @@ import android.view.View;
import android.widget.FrameLayout; import android.widget.FrameLayout;
import android.widget.ListView; import android.widget.ListView;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.WindowDelegate; import org.chromium.chrome.browser.WindowDelegate;
...@@ -39,6 +38,7 @@ public class OmniboxSuggestionsList extends ListView { ...@@ -39,6 +38,7 @@ public class OmniboxSuggestionsList extends ListView {
private final int mSuggestionAnswerHeight; private final int mSuggestionAnswerHeight;
private final int mSuggestionDefinitionHeight; private final int mSuggestionDefinitionHeight;
private final View mAnchorView; private final View mAnchorView;
private final View mAlignmentView;
private final int[] mTempPosition = new int[2]; private final int[] mTempPosition = new int[2];
private final Rect mTempRect = new Rect(); private final Rect mTempRect = new Rect();
...@@ -55,6 +55,14 @@ public class OmniboxSuggestionsList extends ListView { ...@@ -55,6 +55,14 @@ public class OmniboxSuggestionsList extends ListView {
/** Return the anchor view the suggestion list should be drawn below. */ /** Return the anchor view the suggestion list should be drawn below. */
View getAnchorView(); View getAnchorView();
/**
* Return the view that the omnibox suggestions should be aligned horizontally to. The
* view must be a descendant of {@link #getAnchorView()}. If null, the suggestions will
* be aligned to the start of {@link #getAnchorView()}.
*/
@Nullable
View getAlignmentView();
/** Return the bottom sheet for the containing UI to be used in sizing. */ /** Return the bottom sheet for the containing UI to be used in sizing. */
@Nullable @Nullable
BottomSheet getBottomSheet(); BottomSheet getBottomSheet();
...@@ -100,6 +108,26 @@ public class OmniboxSuggestionsList extends ListView { ...@@ -100,6 +108,26 @@ public class OmniboxSuggestionsList extends ListView {
mTempRect.top + mTempRect.bottom + getPaddingTop() + getPaddingBottom(); mTempRect.top + mTempRect.bottom + getPaddingTop() + getPaddingBottom();
mAnchorView = mEmbedder.getAnchorView(); mAnchorView = mEmbedder.getAnchorView();
mAlignmentView = mEmbedder.getAlignmentView();
if (mAlignmentView != null) {
adjustSidePadding();
mAlignmentView.addOnLayoutChangeListener(new OnLayoutChangeListener() {
@Override
public void onLayoutChange(View v, int left, int top, int right, int bottom,
int oldLeft, int oldTop, int oldRight, int oldBottom) {
adjustSidePadding();
}
});
}
}
private void adjustSidePadding() {
if (mAlignmentView == null) return;
ViewUtils.getRelativeLayoutPosition(mAnchorView, mAlignmentView, mTempPosition);
setPadding(mTempPosition[0], getPaddingTop(),
mAnchorView.getWidth() - mAlignmentView.getWidth() - mTempPosition[0],
getPaddingBottom());
} }
/** /**
...@@ -297,7 +325,7 @@ public class OmniboxSuggestionsList extends ListView { ...@@ -297,7 +325,7 @@ public class OmniboxSuggestionsList extends ListView {
for (int i = 0; i < getChildCount(); i++) { for (int i = 0; i < getChildCount(); i++) {
View childView = getChildAt(i); View childView = getChildAt(i);
if (!(childView instanceof SuggestionView)) continue; if (!(childView instanceof SuggestionView)) continue;
ApiCompatibilityUtils.setLayoutDirection(childView, layoutDirection); ViewCompat.setLayoutDirection(childView, layoutDirection);
} }
} }
......
...@@ -63,11 +63,6 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout { ...@@ -63,11 +63,6 @@ public class SearchActivityLocationBarLayout extends LocationBarLayout {
mDelegate.backKeyPressed(); mDelegate.backKeyPressed();
} }
@Override
public boolean mustQueryUrlBarLocationForSuggestions() {
return true;
}
@Override @Override
public void setUrlToPageUrl() { public void setUrlToPageUrl() {
// Explicitly do nothing. The tab is invisible, so showing its URL would be confusing. // Explicitly do nothing. The tab is invisible, so showing its URL would be confusing.
......
...@@ -816,11 +816,6 @@ public class CustomTabToolbar extends ToolbarLayout implements LocationBar, ...@@ -816,11 +816,6 @@ public class CustomTabToolbar extends ToolbarLayout implements LocationBar,
} }
} }
@Override
public boolean mustQueryUrlBarLocationForSuggestions() {
return false;
}
// Temporary fix to override ToolbarLayout's highlight-related methods // Temporary fix to override ToolbarLayout's highlight-related methods
@Override @Override
public void setMenuButtonHighlight(boolean highlight) {} public void setMenuButtonHighlight(boolean highlight) {}
......
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