From bdf1528903c9d9c3fd02d7f44213fd292d8576c0 Mon Sep 17 00:00:00 2001
From: Kyle Doherty <kdoh@users.noreply.github.com>
Date: Thu, 13 Apr 2017 18:43:54 -0600
Subject: [PATCH] Fix various collection editor form bugs (#4764)
* fixes issue 4758
* refactor into own component
* fix #4693 & #4769 + tests
* add flow types
* don't actually need this
* better flow types
---
frontend/src/metabase/lib/colors.js | 12 +++++
frontend/src/metabase/lib/colors.spec.js | 8 +++
.../containers/CollectionEditorForm.jsx | 50 +++++++++++++------
.../containers/CollectionEditorForm.spec.js | 38 ++++++++++++++
4 files changed, 93 insertions(+), 15 deletions(-)
create mode 100644 frontend/src/metabase/lib/colors.spec.js
create mode 100644 frontend/src/metabase/questions/containers/CollectionEditorForm.spec.js
diff --git a/frontend/src/metabase/lib/colors.js b/frontend/src/metabase/lib/colors.js
index 339a2217527..dc43b44b322 100644
--- a/frontend/src/metabase/lib/colors.js
+++ b/frontend/src/metabase/lib/colors.js
@@ -1,3 +1,9 @@
+// @flow
+
+type ColorName = string;
+type Color = string
+type ColorFamily = { [name: ColorName]: Color };
+
export const normal = {
blue: '#509EE3',
green: '#9CC177',
@@ -57,3 +63,9 @@ export const harmony = [
'#c1a877',
'#f95c67',
]
+
+export const getRandomColor = (family: ColorFamily): Color => {
+ // $FlowFixMe: Object.values doesn't preserve the type :-/
+ const colors: Color[] = Object.values(family)
+ return colors[Math.floor(Math.random() * colors.length)]
+}
diff --git a/frontend/src/metabase/lib/colors.spec.js b/frontend/src/metabase/lib/colors.spec.js
new file mode 100644
index 00000000000..1d5d641f775
--- /dev/null
+++ b/frontend/src/metabase/lib/colors.spec.js
@@ -0,0 +1,8 @@
+import { getRandomColor, normal } from 'metabase/lib/colors'
+
+describe('getRandomColor', () => {
+ it('should return a color string from the proper family', () => {
+ const color = getRandomColor(normal)
+ expect(Object.values(normal)).toContain(color)
+ })
+})
diff --git a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
index 305923466c0..73b18e3d00b 100644
--- a/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
+++ b/frontend/src/metabase/questions/containers/CollectionEditorForm.jsx
@@ -8,9 +8,9 @@ import Modal from "metabase/components/Modal";
import { reduxForm } from "redux-form";
-import { normal } from "metabase/lib/colors";
+import { normal, getRandomColor } from "metabase/lib/colors";
-@reduxForm({
+const formConfig = {
form: 'collection',
fields: ['id', 'name', 'description', 'color'],
validate: (values) => {
@@ -29,25 +29,43 @@ import { normal } from "metabase/lib/colors";
name: "",
description: "",
// pick a random color to start so everything isn't blue all the time
- color: normal[Math.floor(Math.random() * normal.length)]
+ color: getRandomColor(normal)
}
-})
-export default class CollectionEditorForm extends Component {
+}
+
+export const getFormTitle = ({ id, name }) =>
+ id.value ? name.value : "New collection"
+
+export const getActionText = ({ id }) =>
+ id.value ? "Update": "Create"
+
+
+export const CollectionEditorFormActions = ({ handleSubmit, invalid, onClose, fields}) =>
+ <div>
+ <Button className="mr1" onClick={onClose}>
+ Cancel
+ </Button>
+ <Button primary disabled={invalid} onClick={handleSubmit}>
+ { getActionText(fields) }
+ </Button>
+ </div>
+
+export class CollectionEditorForm extends Component {
+ props: {
+ fields: Object,
+ onClose: Function,
+ invalid: Boolean,
+ handleSubmit: Function,
+ }
+
render() {
- const { fields, handleSubmit, invalid, onClose } = this.props;
+ const { fields, onClose } = this.props;
return (
<Modal
inline
form
- title={fields.id.value != null ? fields.name.value : "New collection"}
- footer={[
- <Button className="mr1" onClick={onClose}>
- Cancel
- </Button>,
- <Button primary disabled={invalid} onClick={handleSubmit}>
- { fields.id.value != null ? "Update" : "Create" }
- </Button>
- ]}
+ title={getFormTitle(fields)}
+ footer={<CollectionEditorFormActions {...this.props} />}
onClose={onClose}
>
<div className="NewForm ml-auto mr-auto mt4 pt2" style={{ width: 540 }}>
@@ -83,3 +101,5 @@ export default class CollectionEditorForm extends Component {
)
}
}
+
+export default reduxForm(formConfig)(CollectionEditorForm)
diff --git a/frontend/src/metabase/questions/containers/CollectionEditorForm.spec.js b/frontend/src/metabase/questions/containers/CollectionEditorForm.spec.js
new file mode 100644
index 00000000000..cc7b5f07b34
--- /dev/null
+++ b/frontend/src/metabase/questions/containers/CollectionEditorForm.spec.js
@@ -0,0 +1,38 @@
+import {
+ getFormTitle,
+ getActionText
+} from './CollectionEditorForm'
+
+const FORM_FIELDS = {
+ id: { value: 4 },
+ name: { value: 'Test collection' },
+ color: { value: '#409ee3' },
+ initialValues: {
+ color: '#409ee3'
+ }
+}
+const NEW_COLLECTION_FIELDS = { ...FORM_FIELDS, id: '', color: '' }
+
+describe('CollectionEditorForm', () => {
+
+ describe('Title', () => {
+ it('should have a default title if no collection exists', () =>
+ expect(getFormTitle(NEW_COLLECTION_FIELDS)).toEqual('New collection')
+ )
+
+ it('should have the title of the colleciton if one exists', () =>
+ expect(getFormTitle(FORM_FIELDS)).toEqual(FORM_FIELDS.name.value)
+ )
+ })
+
+ describe('Form actions', () => {
+ it('should have a "create" primary action if no collection exists', () =>
+ expect(getActionText(NEW_COLLECTION_FIELDS)).toEqual('Create')
+ )
+
+ it('should have an "update" primary action if no collection exists', () =>
+ expect(getActionText(FORM_FIELDS)).toEqual('Update')
+ )
+ })
+
+})
--
GitLab