📰 The Status card reports disabled sync states

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java
index 09737987930009275f81fbcefa2d48f7dac3e1bd..7e6cdbc93830d61ff3972a3cf4bf4568a95477e1 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusListItem.java
@@ -4,11 +4,18 @@
 
 package org.chromium.chrome.browser.ntp.cards;
 
+import android.content.Context;
 import android.view.View;
 import android.view.View.OnClickListener;
 import android.widget.Button;
+import android.widget.TextView;
 
+import org.chromium.base.Log;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge;
+import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig;
+import org.chromium.chrome.browser.preferences.PreferencesLauncher;
+import org.chromium.chrome.browser.sync.ui.SyncCustomizationFragment;
 
 /**
  * Card that is shown when the user needs to be made aware of some information about their
@@ -20,19 +27,75 @@
      * ViewHolder for an item of type {@link #VIEW_TYPE_STATUS}.
      */
     public static class ViewHolder extends CardViewHolder {
-        public ViewHolder(NewTabPageRecyclerView parent, final NewTabPageAdapter adapter) {
+        private StatusListItem mListItem;

[Bernhard Bauer, 2016/06/14 12:10:40]

Do you actually need this as a member, or could you make it a final local
variable so you can reference it in the closure?

[dgn, 2016/06/15 16:46:28]

Removed.

+
+        public ViewHolder(NewTabPageRecyclerView parent) {
             super(R.layout.new_tab_page_status_card, parent);
-            Button reloadButton = ((Button) itemView.findViewById(R.id.reload_button));
+        }
+
+        @Override
+        public void onBindViewHolder(NewTabPageListItem item) {
+            assert item instanceof StatusListItem;
+            super.onBindViewHolder(item);
+            mListItem = (StatusListItem) item;
+
+            ((TextView) itemView.findViewById(R.id.status_title))
+                    .setText(mListItem.mHeaderStringId);

[Bernhard Bauer, 2016/06/14 12:10:40]

Would it make sense to push the code that sets the text into CardViewHolder, so
it can be used for snippet cards as well?

Also, you could cache the views so you don't have to get them every time the
ViewHolder is rebound. (FWIU, this is the main rationale for using the
ViewHolder pattern; see e.g.
https://www.javacodegeeks.com/2013/09/android-viewholder-pattern-example.html).

[dgn, 2016/06/15 16:46:28]

Done.

+            ((TextView) itemView.findViewById(R.id.status_body))
+                    .setText(mListItem.mDescriptionStringId);
+
+            Button reloadButton = ((Button) itemView.findViewById(R.id.status_action_button));
+            reloadButton.setText(mListItem.mActionStringId);
             reloadButton.setOnClickListener(new OnClickListener() {
 
                 @Override
                 public void onClick(View v) {
-                    adapter.reloadSnippets();
+                    mListItem.performAction(v.getContext());
                 }
             });
         }
     }
 
+    private static final String TAG = "NtpCards";
+
+    private final SnippetsBridge mSnippetsBridge;
+    private final NewTabPageAdapter mNewTabPageAdapter;
+    private final boolean mIsFeatureEnabled;
+    private final int mHeaderStringId;
+    private final int mDescriptionStringId;
+    private final int mActionStringId;
+
+    public StatusListItem(SnippetsBridge snippetsBridge, NewTabPageAdapter adapter) {
+        mNewTabPageAdapter = adapter;
+        mSnippetsBridge = snippetsBridge;
+
+        /* TODO(dgn): When we can report more status, this probably should return multiple states,

[Bernhard Bauer, 2016/06/14 12:10:39]

Nit: We actually use // style even for multiline comments.

[dgn, 2016/06/15 16:46:29]

Removed.

+         * or have some factory return StatusListItem subclasses with the right handling built in.
+         */
+        mIsFeatureEnabled =
+                mSnippetsBridge.getDisabledReason() == SnippetsConfig.DISABLED_REASON_NONE;
+        if (mIsFeatureEnabled) {
+            Log.d(TAG, "Showing status: No Snippets");
+            mHeaderStringId = R.string.ntp_status_card_title_empty;
+            mDescriptionStringId = R.string.ntp_status_card_body_empty;
+            mActionStringId = R.string.reload;
+        } else {
+            Log.d(TAG, "Showing status: Sync Disabled");
+            mHeaderStringId = R.string.ntp_status_card_title_sync;
+            mDescriptionStringId = R.string.ntp_status_card_body_sync;
+            mActionStringId = R.string.ntp_status_card_action_sync;
+        }
+    }
+
+    private void performAction(Context context) {
+        if (mIsFeatureEnabled) {
+            mNewTabPageAdapter.reloadSnippets();
+        } else {
+            PreferencesLauncher.launchSettingsPage(
+                    context, SyncCustomizationFragment.class.getName());
+        }
+    }
+
     @Override
     public int getType() {
         return NewTabPageListItem.VIEW_TYPE_STATUS;