Add support for color input datalist on Android

ui/android/java/src/org/chromium/ui/ColorPickerSimple.java

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 package org.chromium.ui;
 
 import android.content.Context;
 import android.graphics.Canvas;
 import android.graphics.Color;
 import android.graphics.Paint;
 import android.graphics.Rect;
 import android.util.AttributeSet;
 import android.view.MotionEvent;
 import android.view.View;
-
+import android.view.ViewGroup;
 
 /**
  * Draws a grid of (predefined) colors and allows the user to choose one of
  * those colors.
  */
 public class ColorPickerSimple extends View {
-    private static final int ROW_COUNT = 2;
-
     private static final int COLUMN_COUNT = 4;
 
-    private static final int GRID_CELL_COUNT = ROW_COUNT * COLUMN_COUNT;
+    private static final int MINIMUM_CELL_HEIGHT = 50;

[newt (away), 2013/08/19 23:18:19]

MINIMUM_CELL_HEIGHT should be measured in DP, and probably should be called
MINIMUM_CELL_HEIGHT_DP for clarity. You can convert DP to pixels like so:

    int minimumCellHeight = Math.round(MINIMUM_CELL_HEIGHT_DP *
getResources().getDisplayMetrics().density);

+    private static final int GRID_LINE_WIDTH = 1;
 
-    private static final int[] COLORS = { Color.RED,
-                                          Color.CYAN,
-                                          Color.BLUE,
-                                          Color.GREEN,
-                                          Color.MAGENTA,
-                                          Color.YELLOW,
-                                          Color.BLACK,
-                                          Color.WHITE
-                                        };
+    private static final int[] DEFAULT_COLORS = { Color.RED,
+                                                  Color.CYAN,
+                                                  Color.BLUE,
+                                                  Color.GREEN,
+                                                  Color.MAGENTA,
+                                                  Color.YELLOW,
+                                                  Color.BLACK,
+                                                  Color.WHITE
+                                                };
 
     private Paint mBorderPaint;
 
     private Rect[] mBounds;
 
+    private int[] mColors;
+
     private Paint[] mPaints;
 
     private OnColorChangedListener mOnColorTouchedListener;
 
     private int mLastTouchedXPosition;
 
     private int mLastTouchedYPosition;
 
     public ColorPickerSimple(Context context) {
         super(context);
     }
 
     public ColorPickerSimple(Context context, AttributeSet attrs) {
         super(context, attrs);
     }
 
     public ColorPickerSimple(Context context, AttributeSet attrs, int defStyle) {
         super(context, attrs, defStyle);
     }
 
     /**
      * Initializes the listener and precalculates the grid and color positions.

[newt (away), 2013/08/19 23:18:19]

explain what happens if colors is empty. Also, I'd prefer passing null (instead
of an empty array) to signify "use default colors".

[keishi, 2013/08/26 05:28:54]

Changed to use default colors when passed null.

+     * TODO(keishi): Use colorLabels to improve accessibility.
      *
+     * @param colors The list of colors that should be displayed.
+     * @param colorLabels The label text that describe the colors.
      * @param onColorChangedListener The listener that gets notified when the user touches
      *                               a color.
      */
-    public void init(OnColorChangedListener onColorChangedListener) {
+    public void init(int[] colors,

[Miguel Garcia, 2013/08/19 14:43:27]

I think, somewhere in the whole stack, preferably before we convert it to a bare
array that needs to be mapped into memory. We should implement a limit on the
number of suggestions. Is there precedent for this on desktop?

[keishi, 2013/08/26 05:28:54]

No. I checked the limits for <input type=text> suggestions on other browsers. IE
only shows 16 suggestions. And Firefox has no limit.

I added a 10000 limit in the Blink patch.

[Miguel Garcia, 2013/08/27 14:32:00]

Do we really need 10K? Can we settle for 1K colors? I can't conveive a scenario
where more than 1K datalist element would be required and mapping 10K structs
would be expensive.

On 2013/08/26 05:28:54, keishi1 wrote:

[newt (away), 2013/08/27 16:05:29]

Should we limit the total amount of data sent over IPC (e.g. 1024 strings, max
length 1024 chars each) so we don't cause an out of memory error in the browser
process?

[keishi, 2013/08/29 05:04:11]

Added the 1024 char limit in Blink. I wonder if its the best way to handle the
IPC size limit.

+                     String[] colorLabels,
+                     OnColorChangedListener onColorChangedListener) {
         mOnColorTouchedListener = onColorChangedListener;
 
         // This will get calculated when the layout size is updated.
         mBounds = null;
 
-        mPaints = new Paint[GRID_CELL_COUNT];
-        for (int i = 0; i < GRID_CELL_COUNT; ++i) {
+        if (colors.length == 0)

[newt (away), 2013/08/19 23:18:19]

need curly braces

+            mColors = DEFAULT_COLORS;
+        else
+            mColors = colors;
+
+        mPaints = new Paint[mColors.length];
+        for (int i = 0; i < mColors.length; ++i) {
             Paint newPaint = new Paint();
-            newPaint.setColor(COLORS[i]);
+            newPaint.setColor(mColors[i]);
             mPaints[i] = newPaint;
         }
 
         mBorderPaint = new Paint();
         int borderColor = getContext().getResources().getColor(R.color.color_picker_border_color);
         mBorderPaint.setColor(borderColor);
 
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

Calculate mRowCount only once, and store it as a member variable. Also, this can
be done with integer arithmetic:

    mRowCount = (mColors.length + COLUMN_COUNT - 1) / COLUMN_COUNT;

+        int minimumHeight = rowCount * (MINIMUM_CELL_HEIGHT + GRID_LINE_WIDTH) + GRID_LINE_WIDTH;
+        setMinimumHeight(Math.max(minimumHeight, getMinimumHeight()));
+
         // Responds to the user touching the grid and works out which color has been chosen as
         // a result, depending on the X,Y coordinate. Note that we respond to the click event
         // here, but the onClick() method doesn't provide us with the X,Y coordinates, so we
         // track them in onTouchEvent() below. This way the grid reacts properly to touch events
         // whereas if we put this onClick() code in onTouchEvent below then we get some strange
         // interactions with the ScrollView in the parent ColorPickerDialog.
         setOnClickListener(new OnClickListener() {
             @Override
             public void onClick(View v) {
                 if (mOnColorTouchedListener != null && getWidth() > 0 && getHeight() > 0) {
+                    int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

use mRowCount here and below.

                     int column = mLastTouchedXPosition * COLUMN_COUNT / getWidth();
-                    int row = mLastTouchedYPosition * ROW_COUNT / getHeight();
+                    int row = mLastTouchedYPosition * rowCount / getHeight();
 
                     int colorIndex = (row * COLUMN_COUNT) + column;
-                    if (colorIndex >= 0 && colorIndex < COLORS.length) {
-                        mOnColorTouchedListener.onColorChanged(COLORS[colorIndex]);
+                    if (colorIndex >= 0 && colorIndex < mColors.length) {
+                        mOnColorTouchedListener.onColorChanged(mColors[colorIndex]);
                     }
                 }
             }
         });
     }
 
     /**
      * Draws the grid of colors, based on the rectangles calculated in onSizeChanged().
      * Also draws borders in between the colored rectangles.
      *
      * @param canvas The canvas the colors are drawn onto.
      */
     @Override
     public void onDraw(Canvas canvas) {
         if (mBounds == null || mPaints == null) {
             return;
         }
 
         canvas.drawColor(Color.WHITE);
 
         // Draw the actual colored rectangles.
-        for (int i = 0; i < GRID_CELL_COUNT; ++i) {
+        for (int i = 0; i < mColors.length; ++i) {

[Miguel Garcia, 2013/08/19 14:43:27]

can you calculate the number of cells and rows at the beginning and store it in
a local variable for readbility?

             canvas.drawRect(mBounds[i], mPaints[i]);
         }
 
         // Draw 1px borders between the rows.
-        for (int i = 0; i < ROW_COUNT - 1; ++i) {
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);
+        for (int i = 0; i < rowCount - 1; ++i) {
+          if (i * COLUMN_COUNT >= mBounds.length)

[newt (away), 2013/08/19 23:18:19]

this condition can never be true.

+            break;
           canvas.drawLine(0,
                   mBounds[i * COLUMN_COUNT].bottom + 1,
                   getWidth(),
                   mBounds[i * COLUMN_COUNT].bottom + 1,
                   mBorderPaint);
         }
 
         // Draw 1px borders between the columns.
         for (int j = 0; j < COLUMN_COUNT - 1; ++j) {
+          if (j >= mBounds.length)
+            break;
           canvas.drawLine(mBounds[j].right + 1,
                   0,
                   mBounds[j].right + 1,
                   getHeight(),
                   mBorderPaint);
         }
     }
 
     /**
      * Stores the X,Y coordinates of the touch so that we can use them in the onClick() listener
@@ skipping 18 matching lines @@
         calculateGrid(width, height);
     }
 
     /**
      * Calculates the sizes and positions of the cells in the grid, splitting
      * them up as evenly as possible. Leaves 3 pixels between each cell so that
      * we can draw a border between them as well, and leaves a pixel around the
      * edge.
      */
     private void calculateGrid(final int width, final int height) {
-        mBounds = new Rect[GRID_CELL_COUNT];
+        mBounds = new Rect[mColors.length];
 
-        for (int i = 0; i < ROW_COUNT; ++i) {
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

I don't see why you need to add/change lines 197-199 and 205-209. It should work
fine without those changes right?

If your goal is to use the value of GRID_LINE_WIDTH, you could change lines
178-182 to be:

    int left = j * (width + GRID_LINE_WIDTH) / COLUMN_COUNT + GRID_LINE_WIDTH;
    int right = (j + 1) * (width + GRID_LINE_WIDTH) / COLUMN_COUNT - 2 *
GRID_LINE_WIDTH;
    ...

But again, I'd prefer integer arithmetic over floating point division and
casting.

Also, could you move the calculations that use "i" but not "j" into the outer
loop?

+        double cellWidth = (width + GRID_LINE_WIDTH) / (double) COLUMN_COUNT;
+        double cellHeight = (height + GRID_LINE_WIDTH) / (double) rowCount;
+        for (int i = 0; i < rowCount; ++i) {
             for (int j = 0; j < COLUMN_COUNT; ++j) {
-                int left = j * (width + 1) / COLUMN_COUNT + 1;
-                int right = (j + 1) * (width + 1) / COLUMN_COUNT - 2;
+                int index = (i * COLUMN_COUNT) + j;
+                if (index >= mBounds.length)
+                    break;
+                int left = (int) (j * cellWidth + GRID_LINE_WIDTH);
+                int right = (int) ((j + 1) * cellWidth - GRID_LINE_WIDTH * 2);
 
-                int top = i * (height + 1) / ROW_COUNT + 1;
-                int bottom = (i + 1) * (height + 1) / ROW_COUNT - 2;
+                int top = (int) (i * cellHeight + GRID_LINE_WIDTH);
+                int bottom = (int) ((i + 1) * cellHeight - GRID_LINE_WIDTH * 2);
 
                 Rect rect = new Rect(left, top, right, bottom);
-                mBounds[(i * COLUMN_COUNT) + j] = rect;
+                mBounds[index] = rect;
             }
         }
     }
 }