diff --git a/webapp/channels/src/components/__snapshots__/color_input.test.tsx.snap b/webapp/channels/src/components/__snapshots__/color_input.test.tsx.snap deleted file mode 100644 index 6f106e97536..00000000000 --- a/webapp/channels/src/components/__snapshots__/color_input.test.tsx.snap +++ /dev/null @@ -1,479 +0,0 @@ -// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing - -exports[`components/ColorInput should match snapshot, click on picker 1`] = ` -
-
- - - - -
-
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- - -
-
-
-
-
- - - -
-
-
-
-
-
-
-
-`; - -exports[`components/ColorInput should match snapshot, init 1`] = ` -
-
- - - - -
-
-`; - -exports[`components/ColorInput should match snapshot, opened 1`] = ` -
-
- - - - -
-
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
- - -
-
-
-
-
- - - -
-
-
-
-
-
-
-
-`; - -exports[`components/ColorInput should match snapshot, toggle picker 1`] = ` -
-
- - - - -
-
-`; diff --git a/webapp/channels/src/components/admin_console/__snapshots__/color_setting.test.tsx.snap b/webapp/channels/src/components/admin_console/__snapshots__/color_setting.test.tsx.snap index 835f626afa1..3e35be13cfb 100644 --- a/webapp/channels/src/components/admin_console/__snapshots__/color_setting.test.tsx.snap +++ b/webapp/channels/src/components/admin_console/__snapshots__/color_setting.test.tsx.snap @@ -28,10 +28,12 @@ exports[`components/ColorSetting should match snapshot, all 1`] = ` /> @@ -76,10 +78,12 @@ exports[`components/ColorSetting should match snapshot, clicked on color setting /> @@ -161,10 +165,12 @@ exports[`components/ColorSetting should match snapshot, no help text 1`] = ` /> diff --git a/webapp/channels/src/components/color_input.test.tsx b/webapp/channels/src/components/color_input.test.tsx index 85e16ca2dd7..223fd36ed4b 100644 --- a/webapp/channels/src/components/color_input.test.tsx +++ b/webapp/channels/src/components/color_input.test.tsx @@ -1,11 +1,28 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {useCallback, useState} from 'react'; -import {render, screen, fireEvent, userEvent} from 'tests/react_testing_utils'; +import {renderWithContext, userEvent, screen, act} from 'tests/react_testing_utils'; -import ColorInput from './color_input'; +import ColorInput, {type ColorInputProps} from './color_input'; + +function ColorInputWrapper({onChange, value: initialValue, ...otherProps}: ColorInputProps) { + const [value, setValue] = useState(initialValue); + + const handleChange = useCallback((value: string) => { + setValue(value); + onChange(value); + }, [onChange]); + + return ( + + ); +} describe('components/ColorInput', () => { const baseProps = { @@ -14,105 +31,68 @@ describe('components/ColorInput', () => { value: '#ffffff', }; - test('should match snapshot, init', () => { - const {container} = render( - , - ); + test('should hide color picker when first rendered', () => { + const {getByTestId} = renderWithContext(); - expect(container).toMatchSnapshot(); + const inputElement = getByTestId('color-inputColorValue'); + + expect(inputElement).toBeInTheDocument(); + expect(inputElement).toBeVisible(); + expect(screen.queryByTestId('color-popover')).not.toBeInTheDocument(); }); - test('should match snapshot, opened', async () => { - const {container} = render( - , - ); + test('should show color picker when color picker button is clicked', async () => { + const {getByTestId} = renderWithContext(); - await userEvent.click(container.querySelector('.input-group-addon')!); + const colorPickerToggleButton = getByTestId('color-togglerButton'); - expect(container).toMatchSnapshot(); + await userEvent.click(colorPickerToggleButton); + + const colorPopover = getByTestId('color-popover'); + + expect(colorPopover).toBeInTheDocument(); + expect(colorPopover).toBeVisible(); + expect(document.activeElement).toBe(getByTestId('color-inputColorValue')); + + await userEvent.click(document.body); + + expect(screen.queryByTestId('color-popover')).not.toBeInTheDocument(); }); - test('should match snapshot, toggle picker', async () => { - const {container} = render( - , - ); - await userEvent.click(container.querySelector('.input-group-addon')!); - await userEvent.click(container.querySelector('.input-group-addon')!); + test('should change color when the color picker is clicked', async () => { + const {getByTestId} = renderWithContext(); - expect(container).toMatchSnapshot(); - }); + await userEvent.click(getByTestId('color-togglerButton')); - test('should match snapshot, click on picker', async () => { - const {container} = render( - , - ); + const colorPopover = getByTestId('color-popover'); - await userEvent.click(container.querySelector('.input-group-addon')!); - await userEvent.click(container.querySelector('.color-popover')!); + await userEvent.click(colorPopover); - expect(container).toMatchSnapshot(); - }); - - test('should have match state on togglePicker', async () => { - const {container} = render( - , - ); - - // Initially picker should be closed (no color-popover) - expect(container.querySelector('.color-popover')).not.toBeInTheDocument(); - - // Click to open - await userEvent.click(container.querySelector('.input-group-addon')!); - expect(container.querySelector('.color-popover')).toBeInTheDocument(); - - // Click to close - await userEvent.click(container.querySelector('.input-group-addon')!); - expect(container.querySelector('.color-popover')).not.toBeInTheDocument(); - - // Click to open again - await userEvent.click(container.querySelector('.input-group-addon')!); - expect(container.querySelector('.color-popover')).toBeInTheDocument(); + expect(colorPopover).toBeInTheDocument(); + expect(baseProps.onChange).toHaveBeenCalledTimes(1); }); test('should keep what the user types in the textbox until blur', async () => { - let currentValue = '#ffffff'; - const onChange = jest.fn((value: string) => { - currentValue = value; + const {getByTestId} = renderWithContext(); + + const inputElement = getByTestId('color-inputColorValue'); + + await userEvent.clear(inputElement); + await userEvent.type(inputElement, '#abc'); + + expect(inputElement).toHaveValue('#abc'); + + // The RGB here is the equivalent of '#abc'. + expect(getByTestId('color-icon').style.backgroundColor).toBe('rgb(170, 187, 204)'); + + await act(() => { + inputElement.blur(); }); - const {container, rerender} = render( - , - ); + expect(document.activeElement).not.toBe(inputElement); + expect(inputElement).toHaveValue('#aabbcc'); - const input = screen.getByRole('textbox'); - const colorIcon = container.querySelector('.color-icon') as HTMLElement; - - // Simulate focus on input - fireEvent used because userEvent doesn't have direct focus/blur methods - fireEvent.focus(input); - - await userEvent.clear(input); - await userEvent.type(input, '#abc'); - expect(onChange).toHaveBeenLastCalledWith('#aabbcc'); - expect(input).toHaveValue('#abc'); - expect(colorIcon.style.backgroundColor).toBe('rgb(170, 187, 204)'); - - // Rerender with updated value prop (simulating parent component update) - rerender( - , - ); - - fireEvent.blur(input); - - // After blur, the input should show the normalized value - expect(input).toHaveValue('#aabbcc'); - expect(colorIcon.style.backgroundColor).toBe('rgb(170, 187, 204)'); + // The RGB value passed in the assertion is the equivalent of '#aabbcc'. + expect(getByTestId('color-icon').style.backgroundColor).toBe('rgb(170, 187, 204)'); }); }); diff --git a/webapp/channels/src/components/color_input.tsx b/webapp/channels/src/components/color_input.tsx index f305b02e1a5..c8764b955d8 100644 --- a/webapp/channels/src/components/color_input.tsx +++ b/webapp/channels/src/components/color_input.tsx @@ -1,184 +1,159 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -import React from 'react'; +import React, {useCallback, useEffect, useRef, useState} from 'react'; import {ChromePicker} from 'react-color'; import type {ColorResult} from 'react-color'; import tinycolor from 'tinycolor2'; -type Props = { +export interface ColorInputProps { id: string; onChange: (color: string) => void; value: string; isDisabled?: boolean; } -type State = { - focused: boolean; - isOpened: boolean; - value: string; -} +const ColorInput = ({ + id, + onChange: onChangeFromProps, + value: valueFromProps, + isDisabled, +}: ColorInputProps) => { + const container = useRef(null); + const colorInput = useRef(null); -export default class ColorInput extends React.PureComponent { - private colorPicker: React.RefObject; - private colorInput: React.RefObject; + const [isFocused, setIsFocused] = useState(false); + const [isOpened, setIsOpened] = useState(false); + const [valueFromState, setValueFromState] = useState(valueFromProps); - public constructor(props: Props) { - super(props); - this.colorPicker = React.createRef(); - this.colorInput = React.createRef(); - - this.state = { - focused: false, - isOpened: false, - value: props.value, - }; + if (!isFocused && valueFromProps !== valueFromState) { + setValueFromState(valueFromProps); } - static getDerivedStateFromProps(props: Props, state: State) { - if (!state.focused && props.value !== state.value) { - return { - value: props.value, - }; + useEffect(() => { + if (!isOpened) { + return () => {}; } - return null; - } - - public componentDidUpdate(prevProps: Props, prevState: State) { - const {isOpened: prevIsOpened} = prevState; - const {isOpened} = this.state; - - if (isOpened !== prevIsOpened) { - if (isOpened) { - document.addEventListener('click', this.checkClick, {capture: true}); - } else { - document.removeEventListener('click', this.checkClick); + const checkClick = (e: MouseEvent): void => { + if (!container.current || !container.current.contains(e.target as Element)) { + setIsOpened(false); } - } - } + }; - private checkClick = (e: MouseEvent): void => { - if (!this.colorPicker.current || !this.colorPicker.current.contains(e.target as Element)) { - this.setState({isOpened: false}); - } + document.addEventListener('mousedown', checkClick); + + return () => { + document.removeEventListener('mousedown', checkClick); + }; + }, [isOpened]); + + const togglePicker = () => { + colorInput.current?.focus(); + setIsOpened(true); }; - private togglePicker = () => { - if (!this.state.isOpened && this.colorInput.current) { - this.colorInput.current.focus(); - } - this.setState({isOpened: !this.state.isOpened}); - }; + const handleColorChange = useCallback((newColorData: ColorResult) => { + setIsFocused(false); + onChangeFromProps(newColorData.hex); + }, [onChangeFromProps]); - public handleColorChange = (newColorData: ColorResult) => { - this.setState({focused: false}); - this.props.onChange(newColorData.hex); - }; - - private onChange = (event: React.ChangeEvent) => { + const onChange = (event: React.ChangeEvent) => { const value = event.target.value; const color = tinycolor(value); const normalizedColor = '#' + color.toHex(); if (color.isValid()) { - this.props.onChange(normalizedColor); + onChangeFromProps(normalizedColor); } - this.setState({value}); + setValueFromState(value); }; - private onFocus = (event: React.FocusEvent): void => { - this.setState({ - focused: true, - }); + const onFocus = (event: React.FocusEvent): void => { + setIsFocused(true); if (event.target) { event.target.setSelectionRange(1, event.target.value.length); } }; - private onBlur = () => { - const value = this.state.value; + const onBlur = () => { + const value = valueFromState; const color = tinycolor(value); const normalizedColor = '#' + color.toHex(); if (color.isValid()) { - this.props.onChange(normalizedColor); + onChangeFromProps(normalizedColor); - this.setState({ - value: normalizedColor, - }); + setValueFromState(normalizedColor); } else { - this.setState({ - value: this.props.value, - }); + setValueFromState(valueFromProps); } - this.setState({ - focused: false, - }); + setIsFocused(false); }; - private onKeyDown = (event: React.KeyboardEvent) => { + const onKeyDown = (event: React.KeyboardEvent) => { // open picker on enter or space if (event.key === 'Enter' || event.key === ' ') { - this.togglePicker(); + togglePicker(); } }; - public render() { - const {id} = this.props; - const {isOpened, value} = this.state; + return ( +
+ + {!isDisabled && + + + + } + {isOpened && ( +
+ +
+ )} +
+ ); +}; - return ( -
- - {!this.props.isDisabled && - - - - } - {isOpened && ( -
- -
- )} -
- ); - } -} +export default React.memo(ColorInput); diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap index 6865c380420..1c887dd8b2e 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/color_chooser/__snapshots__/color_chooser.test.tsx.snap @@ -21,10 +21,12 @@ exports[`components/user_settings/display/ColorChooser should match, init 1`] = /> diff --git a/webapp/channels/src/components/user_settings/display/user_settings_theme/custom_theme_chooser/__snapshots__/custom_theme_chooser.test.tsx.snap b/webapp/channels/src/components/user_settings/display/user_settings_theme/custom_theme_chooser/__snapshots__/custom_theme_chooser.test.tsx.snap index 563929073b6..256e8805fc9 100644 --- a/webapp/channels/src/components/user_settings/display/user_settings_theme/custom_theme_chooser/__snapshots__/custom_theme_chooser.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/display/user_settings_theme/custom_theme_chooser/__snapshots__/custom_theme_chooser.test.tsx.snap @@ -63,10 +63,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -95,10 +97,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -127,10 +131,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -159,10 +165,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -191,10 +199,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -223,10 +233,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -255,10 +267,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -287,10 +301,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -319,10 +335,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -351,10 +369,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -383,10 +403,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -415,10 +437,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -447,10 +471,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -479,10 +505,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -547,10 +575,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -579,10 +609,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -611,10 +643,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -643,10 +677,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -675,10 +711,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -707,10 +745,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -824,10 +864,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -856,10 +898,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init /> @@ -888,10 +932,12 @@ exports[`components/user_settings/display/CustomThemeChooser should match, init />