From 65b52fae3f5fbb6d0a0976f690a4bc4a12caa052 Mon Sep 17 00:00:00 2001
From: Kyle Doherty <kdoh@users.noreply.github.com>
Date: Wed, 4 Jan 2017 10:55:50 -0800
Subject: [PATCH] more generic color picker and collection colors (#4051)

* more generic color picker and collection colors

* use color picker for chart settings colors
---
 .../src/metabase/components/ColorPicker.jsx   | 88 +++++++++++--------
 .../containers/CollectionEditorForm.jsx       | 18 +++-
 .../settings/ChartSettingColorPicker.jsx      | 41 +--------
 .../settings/ChartSettingColorsPicker.jsx     | 10 ++-
 4 files changed, 79 insertions(+), 78 deletions(-)

diff --git a/frontend/src/metabase/components/ColorPicker.jsx b/frontend/src/metabase/components/ColorPicker.jsx
index 18432574795..076c097e7c5 100644
--- a/frontend/src/metabase/components/ColorPicker.jsx
+++ b/frontend/src/metabase/components/ColorPicker.jsx
@@ -1,60 +1,72 @@
-import React, { Component } from "react";
+import React, { Component, PropTypes } from "react";
 
-import Icon from "metabase/components/Icon";
 import PopoverWithTrigger from "metabase/components/PopoverWithTrigger";
 
-import { normal, saturated, desaturated } from "metabase/lib/colors";
+import { normal } from "metabase/lib/colors";
 
-const COLORS = [
-    ...Object.values(normal),
-    ...Object.values(saturated),
-    ...Object.values(desaturated),
-];
+const DEFAULT_COLOR_SQUARE_SIZE = 32;
 
-const COLOR_SQUARE_SIZE = 32;
-const COLOR_SQUARE = {
-    width: COLOR_SQUARE_SIZE,
-    height: COLOR_SQUARE_SIZE
-};
-
-const ColorSquare = ({ color }) =>
+const ColorSquare = ({ color, size }) =>
     <div style={{
-        ...COLOR_SQUARE,
+        width: size,
+        height: size,
         backgroundColor: color,
-        borderRadius: 4
+        borderRadius: size / 8
     }}></div>
 
 class ColorPicker extends Component {
+    static defaultProps = {
+        colors: [...Object.values(normal)],
+        size: DEFAULT_COLOR_SQUARE_SIZE,
+        triggerSize: DEFAULT_COLOR_SQUARE_SIZE,
+        padding: 4
+    }
+
+    static propTypes = {
+        colors: PropTypes.array,
+        onChange: PropTypes.func.isRequired,
+        size: PropTypes.number,
+        triggerSize: PropTypes.number,
+        value: PropTypes.string
+    }
+
     render () {
-        const { value, onChange } = this.props;
+        const { colors, onChange, padding, size, triggerSize, value } = this.props;
         return (
             <div className="inline-block">
                 <PopoverWithTrigger
                     ref="colorPopover"
                     triggerElement={
-                        <div className="bordered p1 rounded flex align-center">
-                            <ColorSquare color={value} />
-                            <Icon
-                                className="ml1"
-                                name="chevrondown"
-                            />
+                        <div
+                            className="bordered rounded flex align-center"
+                            style={{ padding: triggerSize / 4 }}
+                        >
+                            <ColorSquare color={value} size={triggerSize} />
                         </div>
                     }
                 >
-                    <ol className="flex p1">
-                        { COLORS.map((color, index) =>
-                            <li
-                                className="cursor-pointer mr1 mb1"
-                                key={index}
-                                onClick={() => {
-                                    onChange(color);
-                                    this.refs.colorPopover.close();
-                                }}
-                            >
-                                <ColorSquare color={color} />
-                            </li>
-                        )}
-                    </ol>
+                    <div className="p1">
+                        <ol
+                            className="flex flex-wrap"
+                            style={{
+                                maxWidth: 120
+                            }}
+                        >
+                            { colors.map((color, index) =>
+                                <li
+                                    className="cursor-pointer"
+                                    style={{ padding }}
+                                    key={index}
+                                    onClick={() => {
+                                        onChange(color);
+                                        this.refs.colorPopover.close();
+                                    }}
+                                >
+                                    <ColorSquare color={color} size={size} />
+                                </li>
+                            )}
+                        </ol>
+                    </div>
                 </PopoverWithTrigger>
             </div>
         );
diff --git a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
index 4b530c66804..834a5d4a1a6 100644
--- a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
+++ b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
@@ -8,6 +8,16 @@ import Modal from "metabase/components/Modal";
 
 import { reduxForm } from "redux-form";
 
+import { normal } from "metabase/lib/colors";
+
+const COLLECTION_COLORS = [
+    ...Object.values(normal),
+    '#F1B556',
+    '#A6E7F3',
+    '#7172AD',
+    '#7B8797',
+];
+
 @reduxForm({
     form: 'collection',
     fields: ['id', 'name', 'description', 'color'],
@@ -23,7 +33,12 @@ import { reduxForm } from "redux-form";
         }
         return errors;
     },
-    initialValues: { name: "", description: "", color: "#509EE3" }
+    initialValues: {
+        name: "",
+        description: "",
+        // pick a random color to start so everything isn't blue all the time
+        color: COLLECTION_COLORS[Math.floor(Math.random() * COLLECTION_COLORS.length)]
+    }
 })
 export default class CollectionEditorForm extends Component {
     render() {
@@ -71,6 +86,7 @@ export default class CollectionEditorForm extends Component {
                     >
                         <ColorPicker
                             {...fields.color}
+                            colors={COLLECTION_COLORS}
                         />
                     </FormField>
                 </div>
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
index 631b8506451..2a2731d01a5 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
@@ -1,46 +1,13 @@
 import React, { Component, PropTypes } from "react";
 
-import { normal } from 'metabase/lib/colors'
-const DEFAULT_COLOR_HARMONY = Object.values(normal);
-
-import PopoverWithTrigger from "metabase/components/PopoverWithTrigger.jsx";
+import ColorPicker from "metabase/components/ColorPicker";
 
 export default class ChartSettingColorPicker extends Component {
     render() {
-        const { value, onChange, title } = this.props;
         return (
-            <div className="flex align-center">
-                <PopoverWithTrigger
-                    ref="colorPopover"
-                    hasArrow={false}
-                    tetherOptions={{
-                        attachment: 'middle left',
-                        targetAttachment: 'middle right',
-                        targetOffset: '0 0',
-                        constraints: [{ to: 'window', attachment: 'together', pin: ['left', 'right']}]
-                    }}
-                    triggerElement={
-                        <span className="ml1 mr2 bordered inline-block cursor-pointer" style={{ padding: 4, borderRadius: 3 }}>
-                            <div style={{ width: 15, height: 15, backgroundColor: value }} />
-                        </span>
-                    }
-                >
-                    <ol className="p1">
-                        {DEFAULT_COLOR_HARMONY.map((color, colorIndex) =>
-                            <li
-                                key={colorIndex}
-                                className="CardSettings-colorBlock"
-                                style={{ backgroundColor: color }}
-                                onClick={() => {
-                                    onChange(color);
-                                    this.refs.colorPopover.close();
-                                }}
-                            ></li>
-                        )}
-                    </ol>
-                </PopoverWithTrigger>
-
-                <span className="text-bold">{title}</span>
+            <div className="flex align-center mb1">
+                <ColorPicker {...this.props} triggerSize={12} />
+                {this.props.title && <h4 className="ml1">{this.props.title}</h4>}
             </div>
         );
     }
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
index 9ca6cae19b3..740e52a670f 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorsPicker.jsx
@@ -10,9 +10,15 @@ export default class ChartSettingColorsPicker extends Component {
                 { seriesTitles.map((title, index) =>
                     <ChartSettingColorPicker
                         key={index}
-                        value={value[index]}
-                        onChange={(color) => onChange([...value.slice(0, index), color, ...value.slice(index + 1)])}
+                        onChange={color =>
+                            onChange([
+                                ...value.slice(0, index),
+                                color,
+                                ...value.slice(index + 1)
+                            ])
+                        }
                         title={title}
+                        value={value[index]}
                     />
                 )}
             </div>
-- 
GitLab