From 702b4b106150f13f513838e26db4e17a403dbfd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 6 Jul 2021 19:53:54 +0200 Subject: [PATCH] Only allowing valid color classes (#665) * Only allowing valid color classes * Simplifying the menuColors map and addressing some PR review comments * Fixing type problems * Fixing color * Fixing snapshots --- .../components/kanban/kanbanColumnHeader.tsx | 10 ++++---- .../table/__snapshots__/table.test.tsx.snap | 4 ++-- .../tableGroupHeaderRow.test.tsx.snap | 10 ++++---- .../__snapshots__/tableRow.test.tsx.snap | 4 ++-- .../table/tableGroupHeaderRow.test.tsx | 4 ++-- .../components/table/tableGroupHeaderRow.tsx | 10 ++++---- webapp/src/constants.ts | 24 +++++++++---------- webapp/src/test/testBlockFactory.ts | 2 +- webapp/src/widgets/label.tsx | 8 ++++++- webapp/src/widgets/valueSelector.tsx | 10 ++++---- 10 files changed, 46 insertions(+), 40 deletions(-) diff --git a/webapp/src/components/kanban/kanbanColumnHeader.tsx b/webapp/src/components/kanban/kanbanColumnHeader.tsx index f6e1e7c64..81520052a 100644 --- a/webapp/src/components/kanban/kanbanColumnHeader.tsx +++ b/webapp/src/components/kanban/kanbanColumnHeader.tsx @@ -129,12 +129,12 @@ export default function KanbanColumnHeader(props: Props): JSX.Element { onClick={() => mutator.deletePropertyOption(boardTree, boardTree.groupByProperty!, group.option)} /> - {Constants.menuColors.map((color) => ( + {Object.entries(Constants.menuColors).map(([key, color]) => ( mutator.changePropertyOptionColor(boardTree.board, boardTree.groupByProperty!, group.option, color.id)} + key={key} + id={key} + name={color} + onClick={() => mutator.changePropertyOptionColor(boardTree.board, boardTree.groupByProperty!, group.option, key)} /> ))} } diff --git a/webapp/src/components/table/__snapshots__/table.test.tsx.snap b/webapp/src/components/table/__snapshots__/table.test.tsx.snap index 06ad21686..8f23bd453 100644 --- a/webapp/src/components/table/__snapshots__/table.test.tsx.snap +++ b/webapp/src/components/table/__snapshots__/table.test.tsx.snap @@ -148,7 +148,7 @@ exports[`components/table/Table should match snapshot 1`] = ` tabindex="0" > value 1 @@ -488,7 +488,7 @@ exports[`components/table/Table should match snapshot, read-only 1`] = ` tabindex="0" > value 1 diff --git a/webapp/src/components/table/__snapshots__/tableGroupHeaderRow.test.tsx.snap b/webapp/src/components/table/__snapshots__/tableGroupHeaderRow.test.tsx.snap index 50015d9d2..16add02c5 100644 --- a/webapp/src/components/table/__snapshots__/tableGroupHeaderRow.test.tsx.snap +++ b/webapp/src/components/table/__snapshots__/tableGroupHeaderRow.test.tsx.snap @@ -25,7 +25,7 @@ exports[`should match snapshot on read only 1`] = ` value 1 @@ -292,7 +292,7 @@ exports[`components/table/TableRow should match snapshot, resizing column 1`] = tabindex="0" > value 1 diff --git a/webapp/src/components/table/tableGroupHeaderRow.test.tsx b/webapp/src/components/table/tableGroupHeaderRow.test.tsx index 4319512a7..6313a314b 100644 --- a/webapp/src/components/table/tableGroupHeaderRow.test.tsx +++ b/webapp/src/components/table/tableGroupHeaderRow.test.tsx @@ -49,7 +49,7 @@ const boardTreeNoGroup = { option: { id: '', value: '', - color: 'color1', + color: 'propColorBrown', }, cards: [], } @@ -58,7 +58,7 @@ const boardTreeGroup = { option: { id: 'value1', value: 'value 1', - color: 'color1', + color: 'propColorBrown', }, cards: [], } diff --git a/webapp/src/components/table/tableGroupHeaderRow.tsx b/webapp/src/components/table/tableGroupHeaderRow.tsx index d4b64b149..8e7ad1fc6 100644 --- a/webapp/src/components/table/tableGroupHeaderRow.tsx +++ b/webapp/src/components/table/tableGroupHeaderRow.tsx @@ -127,12 +127,12 @@ const TableGroupHeaderRow = React.memo((props: Props): JSX.Element => { onClick={() => mutator.deletePropertyOption(boardTree, boardTree.groupByProperty!, group.option)} /> - {Constants.menuColors.map((color) => ( + {Object.entries(Constants.menuColors).map(([key, color]) => ( mutator.changePropertyOptionColor(boardTree.board, boardTree.groupByProperty!, group.option, color.id)} + key={key} + id={key} + name={color} + onClick={() => mutator.changePropertyOptionColor(boardTree.board, boardTree.groupByProperty!, group.option, key)} /> ))} } diff --git a/webapp/src/constants.ts b/webapp/src/constants.ts index a0dcffcbc..f8a6af90b 100644 --- a/webapp/src/constants.ts +++ b/webapp/src/constants.ts @@ -2,18 +2,18 @@ // See LICENSE.txt for license information. class Constants { - static readonly menuColors = [ - {id: 'propColorDefault', name: 'Default', type: 'color'}, - {id: 'propColorGray', name: 'Gray', type: 'color'}, - {id: 'propColorBrown', name: 'Brown', type: 'color'}, - {id: 'propColorOrange', name: 'Orange', type: 'color'}, - {id: 'propColorYellow', name: 'Yellow', type: 'color'}, - {id: 'propColorGreen', name: 'Green', type: 'color'}, - {id: 'propColorBlue', name: 'Blue', type: 'color'}, - {id: 'propColorPurple', name: 'Purple', type: 'color'}, - {id: 'propColorPink', name: 'Pink', type: 'color'}, - {id: 'propColorRed', name: 'Red', type: 'color'}, - ] + static readonly menuColors: {[key: string]: string} = { + propColorDefault: 'Default', + propColorGray: 'Gray', + propColorBrown: 'Brown', + propColorOrange: 'Orange', + propColorYellow: 'Yellow', + propColorGreen: 'Green', + propColorBlue: 'Blue', + propColorPurple: 'Purple', + propColorPink: 'Pink', + propColorRed: 'Red', + } static readonly minColumnWidth = 100 static readonly defaultTitleColumnWidth = 280 diff --git a/webapp/src/test/testBlockFactory.ts b/webapp/src/test/testBlockFactory.ts index 9a65d82a6..7496fc1a5 100644 --- a/webapp/src/test/testBlockFactory.ts +++ b/webapp/src/test/testBlockFactory.ts @@ -24,7 +24,7 @@ class TestBlockFactory { const propertyOption: IPropertyOption = { id: 'value1', value: 'value 1', - color: 'color1', + color: 'propColorBrown', } const propertyTemplate: IPropertyTemplate = { id: `property${i + 1}`, diff --git a/webapp/src/widgets/label.tsx b/webapp/src/widgets/label.tsx index 287c66d36..5926bf25f 100644 --- a/webapp/src/widgets/label.tsx +++ b/webapp/src/widgets/label.tsx @@ -2,6 +2,8 @@ // See LICENSE.txt for license information. import React from 'react' +import {Constants} from '../constants' + import './label.scss' type Props = { @@ -13,9 +15,13 @@ type Props = { // Switch is an on-off style switch / checkbox function Label(props: Props): JSX.Element { + let color = 'empty' + if (props.color && props.color in Constants.menuColors) { + color = props.color + } return ( {props.children} diff --git a/webapp/src/widgets/valueSelector.tsx b/webapp/src/widgets/valueSelector.tsx index c26b0bd17..dfa86a5ee 100644 --- a/webapp/src/widgets/valueSelector.tsx +++ b/webapp/src/widgets/valueSelector.tsx @@ -79,12 +79,12 @@ const ValueSelectorLabel = React.memo((props: LabelProps): JSX.Element => { onClick={() => props.onDeleteOption(option)} /> - {Constants.menuColors.map((color) => ( + {Object.entries(Constants.menuColors).map(([key, color]: any) => ( props.onChangeColor(option, color.id)} + key={key} + id={key} + name={color} + onClick={() => props.onChangeColor(option, key)} /> ))}