From f8e873f87d15014a6f5364482e72ef3ff67e46ad Mon Sep 17 00:00:00 2001 From: Marta Bondyra <4283304+mbondyra@users.noreply.github.com> Date: Mon, 19 Aug 2024 14:27:06 +0200 Subject: [PATCH] [TSVB] Visualization blows up when invalid color is passed (#190658) https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6784 ## Summary Fixes https://github.com/elastic/kibana/issues/190657 Fixes https://github.com/elastic/kibana/issues/182136 Co-authored-by: Elastic Machine --- .../components/color_picker.test.tsx | 80 +++++++++++-------- .../application/components/color_picker.tsx | 8 +- .../visualize/group5/_tsvb_time_series.ts | 4 +- 3 files changed, 56 insertions(+), 36 deletions(-) diff --git a/src/plugins/vis_types/timeseries/public/application/components/color_picker.test.tsx b/src/plugins/vis_types/timeseries/public/application/components/color_picker.test.tsx index 3c32275ea5ee5..0619228404191 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/color_picker.test.tsx +++ b/src/plugins/vis_types/timeseries/public/application/components/color_picker.test.tsx @@ -8,60 +8,76 @@ import React from 'react'; import { ColorPicker, ColorPickerProps } from './color_picker'; -import { mount } from 'enzyme'; -import { ReactWrapper } from 'enzyme'; -import { EuiColorPicker, EuiIconTip } from '@elastic/eui'; -import { findTestSubject } from '@elastic/eui/lib/test'; +import { fireEvent, render, screen } from '@testing-library/react'; describe('ColorPicker', () => { + const onChange = jest.fn(); const defaultProps: ColorPickerProps = { name: 'color', value: null, - onChange: jest.fn(), + onChange, disableTrash: true, }; - let component: ReactWrapper; + + const renderColorPicker = (props?: Partial) => + render(); + + afterEach(() => { + jest.clearAllMocks(); + }); it('should render the EuiColorPicker', () => { - component = mount(); - expect(component.find(EuiColorPicker).length).toBe(1); + renderColorPicker(); + expect(screen.getByTestId('tvbColorPicker')).toBeInTheDocument(); }); it('should not render the clear button', () => { - component = mount(); - expect(findTestSubject(component, 'tvbColorPickerClear').length).toBe(0); + renderColorPicker(); + expect(screen.queryByTestId('tvbColorPickerClear')).toBeNull(); }); - it('should render the correct value to the input text if the prop value is hex', () => { - const props = { ...defaultProps, value: '#68BC00' }; - component = mount(); - findTestSubject(component, 'tvbColorPicker').find('button').simulate('click'); - const input = findTestSubject(component, 'euiColorPickerInput_top'); - expect(input.props().value).toBe('#68BC00'); + it('should render incorrect value to the input text but not call onChange prop', () => { + renderColorPicker({ value: '#68BC00' }); + fireEvent.click(screen.getByRole('button')); + fireEvent.change(screen.getAllByTestId('euiColorPickerInput_top')[0], { + target: { value: 'INVALID' }, + }); + expect(onChange).not.toHaveBeenCalled(); + expect(screen.getAllByTestId('euiColorPickerInput_top')[0]).toHaveValue('INVALID'); }); - - it('should render the correct value to the input text if the prop value is rgba', () => { - const props = { ...defaultProps, value: 'rgba(85,66,177,1)' }; - component = mount(); - findTestSubject(component, 'tvbColorPicker').find('button').simulate('click'); - const input = findTestSubject(component, 'euiColorPickerInput_top'); - expect(input.props().value).toBe('85,66,177,1'); + it('should render correct value to the input text and call onChange prop', () => { + renderColorPicker({ value: '#68BC00' }); + fireEvent.click(screen.getByRole('button')); + fireEvent.change(screen.getAllByTestId('euiColorPickerInput_top')[0], { + target: { value: '#FFF' }, + }); + expect(onChange).toHaveBeenCalled(); + expect(screen.getAllByTestId('euiColorPickerInput_top')[0]).toHaveValue('#FFF'); }); it('should render the correct aria label to the color swatch button', () => { - const props = { ...defaultProps, value: 'rgba(85,66,177,0.59)' }; - component = mount(); - const button = findTestSubject(component, 'tvbColorPicker').find('button'); - expect(button.prop('aria-label')).toBe('Color picker (rgba(85,66,177,0.59)), not accessible'); + renderColorPicker({ value: 'rgba(85,66,177,0.59)' }); + expect( + screen.getByLabelText('Color picker (rgba(85,66,177,0.59)), not accessible') + ).toBeInTheDocument(); }); it('should call clear function if the disableTrash prop is false', () => { - const props = { ...defaultProps, disableTrash: false, value: 'rgba(85,66,177,1)' }; - component = mount(); + const { container } = renderColorPicker({ disableTrash: false, value: 'rgba(85,66,177,1)' }); + fireEvent.click(screen.getByTestId('tvbColorPickerClear')); + expect(onChange).toHaveBeenCalled(); + expect(container.querySelector('[data-euiicon-type="cross"]')).toBeInTheDocument(); + }); - findTestSubject(component, 'tvbColorPickerClear').simulate('click'); + it('should render the correct value to the input text if the prop value is hex', () => { + renderColorPicker({ value: '#68BC00' }); + fireEvent.click(screen.getByRole('button')); + expect(screen.getAllByTestId('euiColorPickerInput_top')[0]).toHaveValue('#68BC00'); + }); - expect(component.find(EuiIconTip).length).toBe(1); - expect(defaultProps.onChange).toHaveBeenCalled(); + it('should render the correct value to the input text if the prop value is rgba', () => { + renderColorPicker({ value: 'rgba(85,66,177,1)' }); + fireEvent.click(screen.getByRole('button')); + expect(screen.getAllByTestId('euiColorPickerInput_top')[0]).toHaveValue('85,66,177,1'); }); }); diff --git a/src/plugins/vis_types/timeseries/public/application/components/color_picker.tsx b/src/plugins/vis_types/timeseries/public/application/components/color_picker.tsx index a44134dfa919c..474ce95412797 100644 --- a/src/plugins/vis_types/timeseries/public/application/components/color_picker.tsx +++ b/src/plugins/vis_types/timeseries/public/application/components/color_picker.tsx @@ -41,8 +41,14 @@ export function ColorPicker({ name, value, disableTrash = false, onChange }: Col const { euiTheme } = useEuiTheme(); - const handleColorChange: EuiColorPickerProps['onChange'] = (text: string, { rgba, hex }) => { + const handleColorChange: EuiColorPickerProps['onChange'] = ( + text: string, + { rgba, hex, isValid } + ) => { setColor(text); + if (!isValid) { + return; + } onChange({ [name]: hex ? `rgba(${rgba.join(',')})` : '' }); }; diff --git a/test/functional/apps/visualize/group5/_tsvb_time_series.ts b/test/functional/apps/visualize/group5/_tsvb_time_series.ts index 55c59e38f359a..5e471b3ebb825 100644 --- a/test/functional/apps/visualize/group5/_tsvb_time_series.ts +++ b/test/functional/apps/visualize/group5/_tsvb_time_series.ts @@ -145,8 +145,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { expect(actualCountMin).to.be('3 hours'); }); - // FLAKY: https://github.com/elastic/kibana/issues/182136 - describe.skip('Dark mode', () => { + describe('Dark mode', () => { before(async () => { await kibanaServer.uiSettings.update({ 'theme:darkMode': true, @@ -156,7 +155,6 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) { it(`viz should have light class when background color is white`, async () => { await visualBuilder.clickPanelOptions('timeSeries'); await visualBuilder.setBackgroundColor('#FFFFFF'); - await retry.try(async () => { expect(await visualBuilder.checkTimeSeriesIsLight()).to.be(true); });