Redesign GUI of Website Setting Popup Dialog

chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java

Index: chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java b/chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java
index 887ea4a3047adc5958d18360f800856810c73741..3d325db572eb4c21ff556e330471b5cf000747fd 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/WebsiteSettingsPopup.java
@@ -10,12 +10,11 @@ import android.content.DialogInterface;
 import android.content.Intent;
 import android.graphics.Color;
 import android.provider.Browser;
-import android.text.Html;
 import android.text.TextUtils;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.View.OnClickListener;
-import android.view.ViewGroup.LayoutParams;
+import android.view.ViewGroup;
 import android.view.Window;
 import android.widget.ImageView;
 import android.widget.LinearLayout;
@@ -33,14 +32,16 @@ import java.net.URISyntaxException;
  * Java side of Android implementation of the website settings UI.
  */
 public class WebsiteSettingsPopup implements OnClickListener {
+    private static final String LINK_COLOR_BLUE = "#2AAFE0";

[aurimas (slooooooooow), 2014/07/22 22:05:36]

Please move this to colors.xml

[Ian Wen, 2014/07/23 00:05:41]

Done.

     private static final String HELP_URL =
             "http://www.google.com/support/chrome/bin/answer.py?answer=95617";
     private final Context mContext;
     private final Dialog mDialog;
     private final LinearLayout mContainer;
     private final WebContents mWebContents;
-    private final int mPadding;
+    private final int mPaddingWide, mPaddingThin;
     private TextView mCertificateViewer, mMoreInfoLink;
+    private ViewGroup mCertificateLayout, mDescriptionLayout;
     private String mLinkUrl;
 
     private WebsiteSettingsPopup(Context context, WebContents webContents) {
@@ -49,8 +50,12 @@ public class WebsiteSettingsPopup implements OnClickListener {
 
         mContainer = new LinearLayout(mContext);
         mContainer.setOrientation(LinearLayout.VERTICAL);
-        mPadding = (int) context.getResources().getDimension(R.dimen.certificate_viewer_padding);
-        mContainer.setPadding(mPadding, 0, mPadding, 0);
+        mPaddingWide = (int) context.getResources().getDimension(
+                R.dimen.certificate_viewer_padding);
+        mPaddingThin = (int) context.getResources().getDimension(
+                R.dimen.certificate_viewer_padding_thin);
+        mContainer.setPadding(mPaddingWide, mPaddingWide + mPaddingThin, mPaddingWide,
+                mPaddingWide);
 
         mDialog = new Dialog(mContext);
         mDialog.requestWindowFeature(Window.FEATURE_NO_TITLE);
@@ -76,9 +81,24 @@ public class WebsiteSettingsPopup implements OnClickListener {
         });
     }
 
-    /** Adds a section, which contains an icon, a headline, and a description. */
+    /** Adds certificate section, which contains an icon, a headline, and a description. */

[aurimas (slooooooooow), 2014/07/22 22:05:36]

Change this to be a proper javadoc to make it look like:
/**
 * Adds certificate section, which contains an icon, a headline, and a
description.
 */

(please also fix other comments in this file)

[Ian Wen, 2014/07/23 00:05:41]

Done.

     @CalledByNative
-    private void addSection(int enumeratedIconId, String headline, String description) {
+    private void addCertificateSection(int enumeratedIconId, String headline, String description) {
+        View section = addSection(enumeratedIconId, headline, description);
+        mCertificateLayout = (ViewGroup) section.findViewById(R.id.website_settings_headline)
+                .getParent();
+    }
+
+    /** Adds Description section, which contains an icon, a headline, and a description.
+     *  Most likely headline for description is empty */
+    @CalledByNative
+    private void addDescriptionSection(int enumeratedIconId, String headline, String description) {
+        View section = addSection(enumeratedIconId, headline, description);
+        mDescriptionLayout = (ViewGroup) section.findViewById(R.id.website_settings_headline)
+                .getParent();
+    }
+
+    private View addSection(int enumeratedIconId, String headline, String description) {
         View section = LayoutInflater.from(mContext).inflate(R.layout.website_settings, null);
         ImageView i = (ImageView) section.findViewById(R.id.website_settings_icon);
         int drawableId = ResourceId.mapToDrawableId(enumeratedIconId);
@@ -93,26 +113,20 @@ public class WebsiteSettingsPopup implements OnClickListener {
         if (TextUtils.isEmpty(description)) d.setVisibility(View.GONE);
 
         mContainer.addView(section);
-    }
-
-    /** Adds a horizontal dividing line to separate sections. */
-    @CalledByNative
-    private void addDivider() {
-        View divider = new View(mContext);
-        final int dividerHeight = (int) (2 * mContext.getResources().getDisplayMetrics().density);
-        divider.setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, dividerHeight));
-        divider.setBackgroundColor(Color.GRAY);
-        mContainer.addView(divider);
+        return section;
     }
 
     @CalledByNative
     private void setCertificateViewer(String label) {
         assert mCertificateViewer == null;
         mCertificateViewer =  new TextView(mContext);
-        mCertificateViewer.setText(Html.fromHtml("<a href='#'>" + label + "</a>"));
+        mCertificateViewer.setText(label);
+        mCertificateViewer.setTextColor(Color.parseColor(LINK_COLOR_BLUE));
         mCertificateViewer.setOnClickListener(this);
-        mCertificateViewer.setPadding(0, 0, 0, mPadding);
-        mContainer.addView(mCertificateViewer);
+        mCertificateViewer.setPadding(0, mPaddingWide, 0, mPaddingWide);
+        if (mCertificateLayout != null) {

[aurimas (slooooooooow), 2014/07/22 22:05:36]

In what cases will mCertificateLayout will be null? Should we make it into an
assertion instead of an if check?

[Ian Wen, 2014/07/23 00:05:41]

Done.

+            mCertificateLayout.addView(mCertificateViewer);
+        }
     }
 
     @CalledByNative
@@ -124,10 +138,13 @@ public class WebsiteSettingsPopup implements OnClickListener {
     private void addUrl(String label, String url) {
         mMoreInfoLink = new TextView(mContext);
         mLinkUrl = url;
-        mMoreInfoLink.setText(Html.fromHtml("<a href='#'>" + label + "</a>"));
-        mMoreInfoLink.setPadding(0, mPadding, 0, mPadding);
+        mMoreInfoLink.setText(label);
+        mMoreInfoLink.setTextColor(Color.parseColor(LINK_COLOR_BLUE));
+        mMoreInfoLink.setPadding(0, mPaddingWide + mPaddingThin, 0, mPaddingWide);
         mMoreInfoLink.setOnClickListener(this);
-        mContainer.addView(mMoreInfoLink);
+        if (mDescriptionLayout != null) {
+            mDescriptionLayout.addView(mMoreInfoLink);
+        }
     }
 
     /** Displays the WebsiteSettingsPopup. */