Bug 1260149 - Only show single status icon in TwoLinePageRow. r=liuche, a=sylvestre
authorAndrzej Hunt <ahunt@mozilla.com>
Wed, 27 Apr 2016 21:25:10 +0200
changeset 332993 b6dd4aaea6414628c91bab8df3f0d2a48f01b767
parent 332992 8cbf44e60074128b0dd870005455670a0d3acd1f
child 332994 33e0573ce75fc29edf451b0c128db22ef5843ce6
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersliuche, sylvestre
bugs1260149
milestone48.0a2
Bug 1260149 - Only show single status icon in TwoLinePageRow. r=liuche, a=sylvestre MozReview-Commit-ID: 3wjBRHGioH4
mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
mobile/android/base/resources/layout/two_line_page_row.xml
--- a/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/TwoLinePageRow.java
@@ -38,17 +38,16 @@ public class TwoLinePageRow extends Line
 
     private final TextView mTitle;
     private final TextView mUrl;
     private final ImageView mStatusIcon;
 
     private int mSwitchToTabIconId;
 
     private final FaviconView mFavicon;
-    private final View mReaderCached;
 
     private boolean mShowIcons;
     private int mLoadFaviconJobId = Favicons.NOT_LOADING;
 
     // Only holds a reference to the FaviconView itself, so if the row gets
     // discarded while a task is outstanding, we'll leak less memory.
     private static class UpdateViewFaviconLoadedListener implements OnFaviconLoadedListener {
         private final WeakReference<FaviconView> view;
@@ -103,18 +102,16 @@ public class TwoLinePageRow extends Line
         mUrl = (TextView) findViewById(R.id.url);
         mStatusIcon = (ImageView) findViewById(R.id.status_icon_bookmark);
 
         mSwitchToTabIconId = NO_ICON;
         mShowIcons = true;
 
         mFavicon = (FaviconView) findViewById(R.id.icon);
         mFaviconListener = new UpdateViewFaviconLoadedListener(mFavicon);
-
-        mReaderCached = findViewById(R.id.is_reader_cached);
     }
 
     @Override
     protected void onAttachedToWindow() {
         super.onAttachedToWindow();
 
         Tabs.registerOnTabsChangedListener(this);
     }
@@ -195,19 +192,33 @@ public class TwoLinePageRow extends Line
         if (mSwitchToTabIconId == iconId) {
             return;
         }
 
         mSwitchToTabIconId = iconId;
         mUrl.setCompoundDrawablesWithIntrinsicBounds(mSwitchToTabIconId, 0, 0, 0);
     }
 
-    private void showBookmarkIcon(boolean toShow) {
-        final int visibility = toShow ? VISIBLE : GONE;
-        mStatusIcon.setVisibility(visibility);
+    private void updateStatusIcon(boolean isBookmark, boolean isReaderItem) {
+        if (isReaderItem) {
+            mStatusIcon.setImageResource(R.drawable.status_icon_readercache);
+        } else if (isBookmark) {
+            mStatusIcon.setImageResource(R.drawable.star_blue);
+        }
+
+        if (mShowIcons && (isBookmark || isReaderItem)) {
+            mStatusIcon.setVisibility(View.VISIBLE);
+        } else if (mShowIcons) {
+            // We use INVISIBLE to have consistent padding for our items. This means text/URLs
+            // fade consistently in the same location, regardless of them being bookmarked.
+            mStatusIcon.setVisibility(View.INVISIBLE);
+        } else {
+            mStatusIcon.setVisibility(View.GONE);
+        }
+
     }
 
     /**
      * Stores the page URL, so that we can use it to replace "Switch to tab" if the open
      * tab changes or is closed.
      */
     private void updateDisplayedUrl(String url, boolean hasReaderCacheItem) {
         mPageUrl = url;
@@ -255,19 +266,20 @@ public class TwoLinePageRow extends Line
         update(title, url, 0, false);
     }
 
     protected void update(String title, String url, long bookmarkId, boolean hasReaderCacheItem) {
         if (mShowIcons) {
             // The bookmark id will be 0 (null in database) when the url
             // is not a bookmark.
             final boolean isBookmark = bookmarkId != 0;
-            showBookmarkIcon(isBookmark);
+
+            updateStatusIcon(isBookmark, hasReaderCacheItem);
         } else {
-            showBookmarkIcon(false);
+            updateStatusIcon(false, false);
         }
 
         // Use the URL instead of an empty title for consistency with the normal URL
         // bar view - this is the equivalent of getDisplayTitle() in Tab.java
         setTitle(TextUtils.isEmpty(title) ? url : title);
 
         // No point updating the below things if URL has not changed. Prevents evil Favicon flicker.
         if (url.equals(mPageUrl)) {
@@ -280,18 +292,16 @@ public class TwoLinePageRow extends Line
 
         // Displayed RecentTabsPanel URLs may refer to pages opened in reader mode, so we
         // remove the about:reader prefix to ensure the Favicon loads properly.
         final String pageURL = AboutPages.isAboutReader(url) ?
             ReaderModeUtils.getUrlFromAboutReader(url) : url;
         mLoadFaviconJobId = Favicons.getSizedFaviconForPageFromLocal(getContext(), pageURL, mFaviconListener);
 
         updateDisplayedUrl(url, hasReaderCacheItem);
-
-        mReaderCached.setVisibility(hasReaderCacheItem ? View.VISIBLE : View.INVISIBLE);
     }
 
     /**
      * Update the data displayed by this row.
      * <p>
      * This method must be invoked on the UI thread.
      *
      * @param cursor to extract data from.
--- a/mobile/android/base/resources/layout/two_line_page_row.xml
+++ b/mobile/android/base/resources/layout/two_line_page_row.xml
@@ -43,16 +43,9 @@
 
     <ImageView android:id="@+id/status_icon_bookmark"
                android:layout_width="20dp"
                android:layout_height="20dp"
                android:layout_gravity="center"
                android:visibility="gone"
                android:src="@drawable/star_blue"/>
 
-    <ImageView android:id="@+id/is_reader_cached"
-               android:layout_width="wrap_content"
-               android:layout_height="wrap_content"
-               android:layout_gravity="center_vertical"
-               android:visibility="invisible"
-               android:src="@drawable/status_icon_readercache"/>
-
 </merge>