[TSVB] Visualization blows up when invalid color is passed (elastic#1…

…90658) https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6784 ## Summary Fixes elastic#190657 Fixes elastic#182136 Co-authored-by: Elastic Machine <[email protected]>
xcrzx · Aug 19, 2024 · f8e873f · f8e873f
1 parent d1c4bb3
commit f8e873f
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 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
@@ -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<ColorPickerProps>;
+
+  const renderColorPicker = (props?: Partial<ColorPickerProps>) =>
+    render(<ColorPicker {...defaultProps} {...props} />);
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
 
   it('should render the EuiColorPicker', () => {
-    component = mount(<ColorPicker {...defaultProps} />);
-    expect(component.find(EuiColorPicker).length).toBe(1);
+    renderColorPicker();
+    expect(screen.getByTestId('tvbColorPicker')).toBeInTheDocument();
   });
 
   it('should not render the clear button', () => {
-    component = mount(<ColorPicker {...defaultProps} />);
-    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(<ColorPicker {...props} />);
-    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(<ColorPicker {...props} />);
-    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(<ColorPicker {...props} />);
-    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(<ColorPicker {...props} />);
+    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
@@ -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
@@ -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);
             });