From 28fd9caf4c9effce8d1d8eb2c3ec610500c1f199 Mon Sep 17 00:00:00 2001 From: Daniel Lautzenheiser Date: Wed, 19 Apr 2023 10:57:34 -0400 Subject: [PATCH] fix: improve color widget, add tests (#712) --- packages/core/dev-test/config.yml | 6 +- .../components/common/field/ErrorMessage.tsx | 2 +- packages/core/src/locales/en/index.ts | 6 +- .../src/widgets/colorstring/ColorControl.tsx | 14 +- .../__tests__/ColorControl.spec.ts | 236 ++++++++++++++ .../colorstring/__tests__/validator.spec.ts | 294 ++++++++++++++++++ .../core/src/widgets/colorstring/index.ts | 5 +- .../core/src/widgets/colorstring/validator.ts | 31 ++ .../select/__tests__/SelectControl.spec.ts | 32 ++ packages/core/test/data/fields.mock.ts | 7 + packages/demo/public/config.yml | 6 +- 11 files changed, 628 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/widgets/colorstring/__tests__/ColorControl.spec.ts create mode 100644 packages/core/src/widgets/colorstring/__tests__/validator.spec.ts create mode 100644 packages/core/src/widgets/colorstring/validator.ts diff --git a/packages/core/dev-test/config.yml b/packages/core/dev-test/config.yml index e67eac11..d0acc1b9 100644 --- a/packages/core/dev-test/config.yml +++ b/packages/core/dev-test/config.yml @@ -179,6 +179,10 @@ collections: - name: required label: Required Validation widget: color + - name: allow_input + label: Allow Input + widget: color + allow_input: true - name: with_default label: Required With Default widget: color @@ -186,7 +190,7 @@ collections: - name: pattern label: Pattern Validation widget: color - pattern: ['^#([0-9a-fA-F]{3})(?:[0-9a-fA-F]{3})?$', 'Must be a valid hex code'] + pattern: ['^#[a-fA-F0-9]{3}$|^[a-fA-F0-9]{4}$|^[a-fA-F0-9]{6}$', 'Must be a valid hex code'] allow_input: true required: false - name: alpha diff --git a/packages/core/src/components/common/field/ErrorMessage.tsx b/packages/core/src/components/common/field/ErrorMessage.tsx index b0de317a..80165d89 100644 --- a/packages/core/src/components/common/field/ErrorMessage.tsx +++ b/packages/core/src/components/common/field/ErrorMessage.tsx @@ -22,7 +22,7 @@ const ErrorMessage: FC = ({ errors, className }) => { text-xs text-red-500 px-3 - pt-1 + pt-2 `, className, )} diff --git a/packages/core/src/locales/en/index.ts b/packages/core/src/locales/en/index.ts index f706e230..51b15db4 100644 --- a/packages/core/src/locales/en/index.ts +++ b/packages/core/src/locales/en/index.ts @@ -93,8 +93,10 @@ const en: LocalePhrasesRoot = { rangeCountExact: '%{fieldLabel} must have exactly %{count} item(s).', rangeMin: '%{fieldLabel} must be at least %{minCount} item(s).', rangeMax: '%{fieldLabel} must be %{maxCount} or less item(s).', - invalidPath: `'%{path}' is not a valid path`, - pathExists: `Path '%{path}' already exists`, + invalidPath: `'%{path}' is not a valid path.`, + pathExists: `Path '%{path}' already exists.`, + invalidColor: `Color '%{color}' is invalid.`, + invalidHexCode: `Hex codes must start with a # sign.`, }, i18n: { writingInLocale: 'Writing in %{locale}', diff --git a/packages/core/src/widgets/colorstring/ColorControl.tsx b/packages/core/src/widgets/colorstring/ColorControl.tsx index 00e14d45..cdf82617 100644 --- a/packages/core/src/widgets/colorstring/ColorControl.tsx +++ b/packages/core/src/widgets/colorstring/ColorControl.tsx @@ -9,7 +9,7 @@ import TextField from '@staticcms/core/components/common/text-field/TextField'; import classNames from '@staticcms/core/lib/util/classNames.util'; import type { ColorField, WidgetControlProps } from '@staticcms/core/interface'; -import type { ChangeEvent, FC, MouseEvent } from 'react'; +import type { ChangeEvent, FC, MouseEvent, MouseEventHandler } from 'react'; import type { ColorResult } from 'react-color'; const ColorControl: FC> = ({ @@ -33,9 +33,10 @@ const ColorControl: FC> = ({ ); // show/hide color picker - const handleClick = useCallback(() => { - setShowColorPicker(!showColorPicker); - }, [showColorPicker]); + const handleClick: MouseEventHandler = useCallback(event => { + event.stopPropagation(); + setShowColorPicker(oldShowColorPicker => !oldShowColorPicker); + }, []); const handleClear = useCallback( (event: MouseEvent) => { @@ -84,6 +85,7 @@ const ColorControl: FC> = ({ forSingleList={forSingleList} cursor={allowInput ? 'text' : 'pointer'} disabled={disabled} + disableClick={showColorPicker} >
> = ({
> = ({ onChange={handleInputChange} // make readonly and open color picker on click if set to allow_input: false onClick={!allowInput && !disabled ? handleClick : undefined} - disabled={!allowInput || disabled} + disabled={disabled} + readonly={!allowInput} cursor={allowInput ? 'text' : 'pointer'} /> {showClearButton ? ( diff --git a/packages/core/src/widgets/colorstring/__tests__/ColorControl.spec.ts b/packages/core/src/widgets/colorstring/__tests__/ColorControl.spec.ts new file mode 100644 index 00000000..5dcac721 --- /dev/null +++ b/packages/core/src/widgets/colorstring/__tests__/ColorControl.spec.ts @@ -0,0 +1,236 @@ +/** + * @jest-environment jsdom + */ +import '@testing-library/jest-dom'; +import userEvent from '@testing-library/user-event'; +import { act } from '@testing-library/react'; + +import { mockColorField } from '@staticcms/test/data/fields.mock'; +import { createWidgetControlHarness } from '@staticcms/test/harnesses/widget.harness'; +import ColorControl from '../ColorControl'; + +describe(ColorControl.name, () => { + const renderControl = createWidgetControlHarness(ColorControl, { field: mockColorField }); + + it('should render', () => { + const { getByTestId } = renderControl({ label: 'I am a label' }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + expect(input).toBeInTheDocument(); + + // Color Widget input should be read only by default + expect(input).toHaveAttribute('readonly'); + + const label = getByTestId('label'); + expect(label.textContent).toBe('I am a label'); + expect(label).toHaveClass('text-slate-500'); + + const field = getByTestId('field'); + expect(field).toHaveClass('group/active'); + + const fieldWrapper = getByTestId('field-wrapper'); + expect(fieldWrapper).not.toHaveClass('mr-14'); + + // Color Widget uses pointer cursor + expect(label).toHaveClass('cursor-pointer'); + expect(field).toHaveClass('cursor-pointer'); + + // Color Widget uses default label layout, with bottom padding on field + expect(label).toHaveClass('px-3', 'pt-3'); + expect(field).toHaveClass('pb-3'); + }); + + it('should render as single list item', () => { + const { getByTestId } = renderControl({ label: 'I am a label', forSingleList: true }); + + expect(getByTestId('text-input')).toBeInTheDocument(); + + const fieldWrapper = getByTestId('field-wrapper'); + expect(fieldWrapper).toHaveClass('mr-14'); + }); + + it('should only use prop value as initial value', async () => { + const { rerender, getByTestId } = renderControl({ value: '#ffffff' }); + + const swatch = getByTestId('color-swatch'); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + expect(input).toHaveValue('#ffffff'); + expect(swatch).toHaveStyle({ + background: '#ffffff', + color: 'rgba(255, 255, 255, 0)', + }); + + rerender({ value: '#000000' }); + expect(input).toHaveValue('#ffffff'); + expect(swatch).toHaveStyle({ + background: '#ffffff', + color: 'rgba(255, 255, 255, 0)', + }); + }); + + it('should use prop value exclusively if field is i18n duplicate', async () => { + const { rerender, getByTestId } = renderControl({ + field: { ...mockColorField, i18n: 'duplicate' }, + duplicate: true, + value: '#ffffff', + }); + + const swatch = getByTestId('color-swatch'); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + expect(input).toHaveValue('#ffffff'); + expect(swatch).toHaveStyle({ + background: '#ffffff', + color: 'rgba(255, 255, 255, 0)', + }); + + rerender({ value: '#000000' }); + expect(input).toHaveValue('#000000'); + expect(swatch).toHaveStyle({ + background: '#000000', + color: 'rgba(255, 255, 255, 0)', + }); + }); + + it('should show error', async () => { + const { getByTestId } = renderControl({ + errors: [{ type: 'error-type', message: 'i am an error' }], + }); + + const error = getByTestId('error'); + expect(error.textContent).toBe('i am an error'); + + const field = getByTestId('field'); + expect(field).not.toHaveClass('group/active'); + + const label = getByTestId('label'); + expect(label).toHaveClass('text-red-500'); + }); + + it('should disable input if disabled', async () => { + const { getByTestId } = renderControl({ disabled: true }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + expect(input).toBeDisabled(); + }); + + describe('allow input', () => { + it('should call onChange when text input changes (hex code)', async () => { + const { + getByTestId, + props: { onChange }, + } = renderControl({ + field: { ...mockColorField, allow_input: true }, + }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + + await act(async () => { + await userEvent.type(input, '#000000'); + }); + + expect(onChange).toHaveBeenLastCalledWith('#000000'); + + const swatch = getByTestId('color-swatch'); + expect(swatch).toHaveStyle({ + background: '#000000', + color: 'rgba(255, 255, 255, 0)', + }); + }); + + it('should call onChange when text input changes (rgb code)', async () => { + const { + getByTestId, + props: { onChange }, + } = renderControl({ + field: { ...mockColorField, allow_input: true }, + }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + + await act(async () => { + await userEvent.type(input, 'rgb(25, 50, 5)'); + }); + + expect(onChange).toHaveBeenLastCalledWith('rgb(25, 50, 5)'); + + const swatch = getByTestId('color-swatch'); + expect(swatch).toHaveStyle({ + background: 'rgb(25, 50, 5)', + color: 'rgba(255, 255, 255, 0)', + }); + }); + + it('should call onChange when text input changes (rgba code)', async () => { + const { + getByTestId, + props: { onChange }, + } = renderControl({ + field: { ...mockColorField, allow_input: true }, + }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + + await act(async () => { + await userEvent.type(input, 'rgba(25, 50, 5, 0.5)'); + }); + + expect(onChange).toHaveBeenLastCalledWith('rgba(25, 50, 5, 0.5)'); + + const swatch = getByTestId('color-swatch'); + expect(swatch).toHaveStyle({ + background: 'rgba(25, 50, 5, 0.5)', + color: 'rgba(255, 255, 255, 0)', + }); + }); + + it('should make question mark visible if bad color is put in input', async () => { + const { + getByTestId, + props: { onChange }, + } = renderControl({ + field: { ...mockColorField, allow_input: true }, + }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + + await act(async () => { + await userEvent.type(input, '#0muz14'); + }); + + expect(onChange).toHaveBeenLastCalledWith('#0muz14'); + + const swatch = getByTestId('color-swatch'); + expect(swatch).toHaveStyle({ + background: '#fff', + color: 'rgb(150, 150, 150)', + }); + }); + + it('should focus input on field click', async () => { + const { getByTestId } = renderControl({ + field: { ...mockColorField, allow_input: true }, + }); + + const inputWrapper = getByTestId('text-input'); + const input = inputWrapper.getElementsByTagName('input')[0]; + expect(input).not.toHaveFocus(); + + await act(async () => { + const field = getByTestId('field'); + await userEvent.click(field); + }); + + expect(input).toHaveFocus(); + }); + }); +}); diff --git a/packages/core/src/widgets/colorstring/__tests__/validator.spec.ts b/packages/core/src/widgets/colorstring/__tests__/validator.spec.ts new file mode 100644 index 00000000..7fc908ad --- /dev/null +++ b/packages/core/src/widgets/colorstring/__tests__/validator.spec.ts @@ -0,0 +1,294 @@ +/** + * @jest-environment jsdom + */ +import '@testing-library/jest-dom'; + +import ValidationErrorTypes from '@staticcms/core/constants/validationErrorTypes'; +import { mockColorField } from '@staticcms/test/data/fields.mock'; +import validator from '../validator'; + +describe('validator relation', () => { + const t = jest.fn(); + + beforeEach(() => { + t.mockReset(); + t.mockImplementation((key: string) => key); + }); + + describe('hex code', () => { + it('should accept 6 digit hex code', () => { + expect( + validator({ + field: mockColorField, + value: '#ffffff', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept 4 digit hex code', () => { + expect( + validator({ + field: mockColorField, + value: '#1b30', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept 3 digit hex code', () => { + expect( + validator({ + field: mockColorField, + value: '#1b3', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should return error on hex code with invalid characters', () => { + expect( + validator({ + field: mockColorField, + value: '#fzffOm', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: '#fzffOm', + }); + }); + + it('should return error on hex code that is not 3, 4 or 6 characters', () => { + expect( + validator({ + field: mockColorField, + value: '#ff', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: '#ff', + }); + }); + + it('should return error on hex code without pound sign', () => { + expect( + validator({ + field: mockColorField, + value: 'ffffff', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidHexCode', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidHexCode'); + }); + }); + + describe('rgb', () => { + it('should accept rgb string with commas', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb(25, 50, 5)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept rgb string with just red and green', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb(25, 50)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept rgb string without commas', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb(25 50 5)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should return error with no parentheses', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb 25 50 5', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgb 25 50 5', + }); + }); + + it('should return error with invalid characters', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb(25f, 50, B)', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgb(25f, 50, B)', + }); + }); + + it('should return error with just red', () => { + expect( + validator({ + field: mockColorField, + value: 'rgb(25)', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgb(25)', + }); + }); + }); + + describe('rgba', () => { + it('should accept rgba string with commas', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25, 50, 5, 0.5)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept rgba string without alpha', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25, 50, 5)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept rgba string with just red and green', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25, 50)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should accept rgba string without commas', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25 50 5 0.5)', + t, + }), + ).toBeFalsy(); + + expect(t).not.toHaveBeenCalled(); + }); + + it('should return error with no parentheses', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba 25 50 5 0.5', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgba 25 50 5 0.5', + }); + }); + + it('should return error with invalid characters', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25f, 50, B, 0.z)', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgba(25f, 50, B, 0.z)', + }); + }); + + it('should return error with just red', () => { + expect( + validator({ + field: mockColorField, + value: 'rgba(25)', + t, + }), + ).toEqual({ + type: ValidationErrorTypes.CUSTOM, + message: 'editor.editorControlPane.widget.invalidColor', + }); + + expect(t).toHaveBeenCalledWith('editor.editorControlPane.widget.invalidColor', { + color: 'rgba(25)', + }); + }); + }); +}); diff --git a/packages/core/src/widgets/colorstring/index.ts b/packages/core/src/widgets/colorstring/index.ts index 2608dd24..abce5147 100644 --- a/packages/core/src/widgets/colorstring/index.ts +++ b/packages/core/src/widgets/colorstring/index.ts @@ -1,6 +1,7 @@ import controlComponent from './ColorControl'; import previewComponent from './ColorPreview'; import schema from './schema'; +import validator from './validator'; import type { ColorField, WidgetParam } from '@staticcms/core/interface'; @@ -11,6 +12,7 @@ const ColorWidget = (): WidgetParam => { previewComponent, options: { schema, + validator, }, }; }; @@ -18,7 +20,8 @@ const ColorWidget = (): WidgetParam => { export { controlComponent as ColorControl, previewComponent as ColorPreview, - schema as ColorSchema, + schema as colorSchema, + validator as colorValidator, }; export default ColorWidget; diff --git a/packages/core/src/widgets/colorstring/validator.ts b/packages/core/src/widgets/colorstring/validator.ts new file mode 100644 index 00000000..ab81806b --- /dev/null +++ b/packages/core/src/widgets/colorstring/validator.ts @@ -0,0 +1,31 @@ +import validateColor from 'validate-color'; + +import ValidationErrorTypes from '@staticcms/core/constants/validationErrorTypes'; + +import type { ColorField, FieldValidationMethod } from '@staticcms/core/interface'; + +const validator: FieldValidationMethod = ({ value, t }) => { + if (typeof value !== 'string') { + return false; + } + + if (validateColor(value)) { + return false; + } + + if (/^[a-fA-F0-9]{3}$|^[a-fA-F0-9]{4}$|^[a-fA-F0-9]{6}$/g.test(value)) { + return { + type: ValidationErrorTypes.CUSTOM, + message: t(`editor.editorControlPane.widget.invalidHexCode`), + }; + } + + return { + type: ValidationErrorTypes.CUSTOM, + message: t(`editor.editorControlPane.widget.invalidColor`, { + color: value, + }), + }; +}; + +export default validator; diff --git a/packages/core/src/widgets/select/__tests__/SelectControl.spec.ts b/packages/core/src/widgets/select/__tests__/SelectControl.spec.ts index 14af3e4e..5fc0d90a 100644 --- a/packages/core/src/widgets/select/__tests__/SelectControl.spec.ts +++ b/packages/core/src/widgets/select/__tests__/SelectControl.spec.ts @@ -21,6 +21,38 @@ describe(SelectControl.name, () => { expect(input).toBeDisabled(); }); + it('should open and close options on field click', async () => { + const { getByTestId, queryByTestId } = renderControl(); + + const option1 = 'select-option-Option 1'; + const option2 = 'select-option-Option 2'; + + await waitFor(() => { + expect(queryByTestId(option1)).not.toBeInTheDocument(); + expect(queryByTestId(option2)).not.toBeInTheDocument(); + }); + + await act(async () => { + const field = getByTestId('field'); + await userEvent.click(field); + }); + + await waitFor(() => { + expect(queryByTestId(option1)).toBeInTheDocument(); + expect(queryByTestId(option2)).toBeInTheDocument(); + }); + + await act(async () => { + const field = getByTestId('field'); + await userEvent.click(field); + }); + + await waitFor(() => { + expect(queryByTestId(option1)).not.toBeInTheDocument(); + expect(queryByTestId(option2)).not.toBeInTheDocument(); + }); + }); + describe('simple string select', () => { it('should render', () => { const { getByTestId } = renderControl({ label: 'I am a label' }); diff --git a/packages/core/test/data/fields.mock.ts b/packages/core/test/data/fields.mock.ts index 21b6fef9..e5d7666a 100644 --- a/packages/core/test/data/fields.mock.ts +++ b/packages/core/test/data/fields.mock.ts @@ -1,5 +1,6 @@ import type { BooleanField, + ColorField, DateTimeField, FileOrImageField, MarkdownField, @@ -15,6 +16,12 @@ export const mockBooleanField: BooleanField = { widget: 'boolean', }; +export const mockColorField: ColorField = { + label: 'Color', + name: 'mock_color', + widget: 'color', +}; + export const mockDateTimeField: DateTimeField = { label: 'DateTime', name: 'mock_datetime', diff --git a/packages/demo/public/config.yml b/packages/demo/public/config.yml index c761930c..080d3e1b 100644 --- a/packages/demo/public/config.yml +++ b/packages/demo/public/config.yml @@ -179,6 +179,10 @@ collections: - name: required label: Required Validation widget: color + - name: allow_input + label: Allow Input + widget: color + allow_input: true - name: with_default label: Required With Default widget: color @@ -186,7 +190,7 @@ collections: - name: pattern label: Pattern Validation widget: color - pattern: ['^#([0-9a-fA-F]{3})(?:[0-9a-fA-F]{3})?$', 'Must be a valid hex code'] + pattern: ['^#[a-fA-F0-9]{3}$|^[a-fA-F0-9]{4}$|^[a-fA-F0-9]{6}$', 'Must be a valid hex code'] allow_input: true required: false - name: alpha