From 2b073b9337bbab5cbda2f66ac0b7f045d1ca52e8 Mon Sep 17 00:00:00 2001 From: Viktor Tsvetkov <142901247+vtsvetkov-splunk@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:40:53 +0100 Subject: [PATCH 01/12] fix(a11y): connect labels to inputs Signed-off-by: Viktor Tsvetkov <142901247+vtsvetkov-splunk@users.noreply.github.com> --- ui/.storybook/withControlGroup.tsx | 11 +++++ .../components/BaseFormView/BaseFormView.tsx | 2 + .../BaseFormView/BaseFormViewGrups.test.tsx | 15 +++--- .../BaseFormView/tests/BaseFormView.test.tsx | 3 +- .../CheckBoxComponent/CheckBoxComponent.tsx | 8 +-- .../stories/CheckBoxComponent.stories.tsx | 2 + .../CheckboxGroup/CheckboxGroup.tsx | 9 ++-- .../components/CheckboxGroup/CheckboxRow.tsx | 3 +- .../CheckboxGroup/StyledComponent.tsx | 9 ---- .../stories/CheckboxGroup.stories.tsx | 2 + .../components/CheckboxTree/CheckboxTree.tsx | 9 ++-- .../CheckboxTree/StyledComponent.tsx | 9 ---- .../stories/CheckboxTree.stories.tsx | 2 + .../ControlWrapper/ControlWrapper.tsx | 14 ++---- .../FileInputComponent/FileInputComponent.tsx | 4 +- .../stories/FileInputComponent.stories.tsx | 2 + .../FormModifications.test.tsx | 28 +++++------ .../MultiInputComponent.test.tsx | 49 +++++++++++-------- .../MultiInputComponent.tsx | 26 +++++----- .../stories/MultiInputComponent.stories.tsx | 6 +-- .../RadioComponent/RadioComponent.tsx | 25 +++------- .../stories/RadioComponent.stories.ts | 2 + .../SingleInputComponent.test.tsx | 18 +++---- .../SingleInputComponent.tsx | 44 ++++++----------- .../stories/SingleInputComponent.stories.tsx | 2 + .../TextAreaComponent/TextAreaComponent.tsx | 44 ++++++++--------- .../stories/TextAreaComponent.stories.tsx | 2 + .../TextComponent/TextComponent.tsx | 25 +++------- .../stories/TextComponent.stories.tsx | 2 + 29 files changed, 172 insertions(+), 205 deletions(-) create mode 100644 ui/.storybook/withControlGroup.tsx delete mode 100644 ui/src/components/CheckboxGroup/StyledComponent.tsx diff --git a/ui/.storybook/withControlGroup.tsx b/ui/.storybook/withControlGroup.tsx new file mode 100644 index 000000000..bf6d0f08e --- /dev/null +++ b/ui/.storybook/withControlGroup.tsx @@ -0,0 +1,11 @@ +import React from 'react'; +import ControlGroup from '@splunk/react-ui/ControlGroup'; +import { Decorator } from '@storybook/react'; + +export const withControlGroup: Decorator = (StoryFn, { name }) => { + return ( + + {StoryFn()} + + ); +}; diff --git a/ui/src/components/BaseFormView/BaseFormView.tsx b/ui/src/components/BaseFormView/BaseFormView.tsx index 9adc26495..83cb4ea3e 100644 --- a/ui/src/components/BaseFormView/BaseFormView.tsx +++ b/ui/src/components/BaseFormView/BaseFormView.tsx @@ -1333,6 +1333,8 @@ class BaseFormView extends PureComponent { return (
diff --git a/ui/src/components/BaseFormView/BaseFormViewGrups.test.tsx b/ui/src/components/BaseFormView/BaseFormViewGrups.test.tsx index bd7d0bea6..c1b7ba569 100644 --- a/ui/src/components/BaseFormView/BaseFormViewGrups.test.tsx +++ b/ui/src/components/BaseFormView/BaseFormViewGrups.test.tsx @@ -24,7 +24,7 @@ it('should modify correctly all properties of field in groups', async () => { handleFormSubmit={handleFormSubmit} /> ); - await screen.findByText('Text 1 Group 2'); + await screen.findByRole('textbox', { name: 'Text 1 Group 2' }); const getAndValidateGroupFieldLabels = ( fieldId: string, @@ -46,14 +46,13 @@ it('should modify correctly all properties of field in groups', async () => { return modifiedFieldSameGroup; }; - const modifiedFieldSameGroup = getAndValidateGroupFieldLabels( - 'text_field_2_group_1', - 'help after mods 2-1', - 'label after mods 2-1', - 'markdown message after mods 2-1' - ); + const modifiedFieldSameGroup = screen.getByRole('textbox', { + name: 'label after mods 2-1', + description: 'markdown message after mods 2-1 help after mods 2-1', + }); - expect(within(modifiedFieldSameGroup).queryByText('*')).toBeInTheDocument(); + expect(modifiedFieldSameGroup).toBeInTheDocument(); + expect(modifiedFieldSameGroup).toHaveAttribute('required'); const modifiedFieldDiffGroup = getAndValidateGroupFieldLabels( 'text_field_2_group_2', diff --git a/ui/src/components/BaseFormView/tests/BaseFormView.test.tsx b/ui/src/components/BaseFormView/tests/BaseFormView.test.tsx index 63b31af09..d5170e89b 100644 --- a/ui/src/components/BaseFormView/tests/BaseFormView.test.tsx +++ b/ui/src/components/BaseFormView/tests/BaseFormView.test.tsx @@ -51,8 +51,7 @@ it('should render base form correctly with name and File fields', async () => { /> ); - const nameField = document.querySelector('[data-name="name"]'); - expect(nameField).toBeInTheDocument(); + screen.getByRole('textbox', { name: 'Name' }); const fileField = document.querySelector('[data-name="name"]'); expect(fileField).toBeInTheDocument(); diff --git a/ui/src/components/CheckBoxComponent/CheckBoxComponent.tsx b/ui/src/components/CheckBoxComponent/CheckBoxComponent.tsx index ca9a05732..71dea1310 100644 --- a/ui/src/components/CheckBoxComponent/CheckBoxComponent.tsx +++ b/ui/src/components/CheckBoxComponent/CheckBoxComponent.tsx @@ -19,13 +19,13 @@ class CheckBoxComponent extends React.Component { }; render() { + const { field, value, ...restSuiProps } = this.props; return ( ); diff --git a/ui/src/components/CheckBoxComponent/stories/CheckBoxComponent.stories.tsx b/ui/src/components/CheckBoxComponent/stories/CheckBoxComponent.stories.tsx index c804eb453..3b979631b 100644 --- a/ui/src/components/CheckBoxComponent/stories/CheckBoxComponent.stories.tsx +++ b/ui/src/components/CheckBoxComponent/stories/CheckBoxComponent.stories.tsx @@ -2,10 +2,12 @@ import type { Meta, StoryObj } from '@storybook/react'; import React, { useState } from 'react'; import { fn } from '@storybook/test'; import CheckBoxComponent from '../CheckBoxComponent'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: CheckBoxComponent, title: 'CheckBoxComponent', + decorators: [withControlGroup], render: (props) => { // due to stories incompatibility, eslint rule is off // React Hook "useState" is called in function "render" that is neither a React function component diff --git a/ui/src/components/CheckboxGroup/CheckboxGroup.tsx b/ui/src/components/CheckboxGroup/CheckboxGroup.tsx index 66907ee44..0d1968499 100644 --- a/ui/src/components/CheckboxGroup/CheckboxGroup.tsx +++ b/ui/src/components/CheckboxGroup/CheckboxGroup.tsx @@ -1,7 +1,6 @@ import React, { useEffect, useState } from 'react'; import ColumnLayout from '@splunk/react-ui/ColumnLayout'; import Button from '@splunk/react-ui/Button'; -import { StyledColumnLayout } from './StyledComponent'; import { CheckboxGroupProps, getDefaultValues, @@ -60,8 +59,8 @@ function CheckboxGroup(props: CheckboxGroupProps) { }; return ( - <> - +
+ {flattenedRowsWithGroups.map((row) => { if (isGroupWithRows(row)) { // labels are unique across groups @@ -88,7 +87,7 @@ function CheckboxGroup(props: CheckboxGroupProps) { ); })} - +
- +
); } diff --git a/ui/src/components/CheckboxGroup/CheckboxRow.tsx b/ui/src/components/CheckboxGroup/CheckboxRow.tsx index db5e41826..e59158b88 100644 --- a/ui/src/components/CheckboxGroup/CheckboxRow.tsx +++ b/ui/src/components/CheckboxGroup/CheckboxRow.tsx @@ -2,7 +2,6 @@ import React, { useEffect, useState } from 'react'; import NumberComponent, { NumberChangeHandler } from '@splunk/react-ui/Number'; import styled from 'styled-components'; import Switch from '@splunk/react-ui/Switch'; -import { FixedCheckboxRowWidth } from './StyledComponent'; const StyledSwitch = styled(Switch)` padding: 0 3px; @@ -16,7 +15,7 @@ const StyledRow = styled.div` display: flex; align-items: baseline; justify-content: space-between; - ${FixedCheckboxRowWidth} + width: 100%; `; interface CheckboxRowProps { diff --git a/ui/src/components/CheckboxGroup/StyledComponent.tsx b/ui/src/components/CheckboxGroup/StyledComponent.tsx deleted file mode 100644 index 7fac4233b..000000000 --- a/ui/src/components/CheckboxGroup/StyledComponent.tsx +++ /dev/null @@ -1,9 +0,0 @@ -import styled, { css } from 'styled-components'; -import ColumnLayout from '@splunk/react-ui/ColumnLayout'; - -export const FixedCheckboxRowWidth = css` - width: 320px; -`; -export const StyledColumnLayout = styled(ColumnLayout)` - ${FixedCheckboxRowWidth} -`; diff --git a/ui/src/components/CheckboxGroup/stories/CheckboxGroup.stories.tsx b/ui/src/components/CheckboxGroup/stories/CheckboxGroup.stories.tsx index d8a4a680c..958f89a98 100644 --- a/ui/src/components/CheckboxGroup/stories/CheckboxGroup.stories.tsx +++ b/ui/src/components/CheckboxGroup/stories/CheckboxGroup.stories.tsx @@ -2,10 +2,12 @@ import type { Meta, StoryObj } from '@storybook/react'; import { fn } from '@storybook/test'; import CheckboxGroup from '../CheckboxGroup'; import { MODE_CREATE, MODE_EDIT } from '../../../constants/modes'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: CheckboxGroup, title: 'CheckboxGroup/Component', + decorators: [withControlGroup], } satisfies Meta; export default meta; diff --git a/ui/src/components/CheckboxTree/CheckboxTree.tsx b/ui/src/components/CheckboxTree/CheckboxTree.tsx index 76df9ae1e..a36a69a9a 100644 --- a/ui/src/components/CheckboxTree/CheckboxTree.tsx +++ b/ui/src/components/CheckboxTree/CheckboxTree.tsx @@ -1,7 +1,6 @@ import React, { useEffect, useState, useCallback, useMemo } from 'react'; import ColumnLayout from '@splunk/react-ui/ColumnLayout'; import Button from '@splunk/react-ui/Button'; -import { StyledColumnLayout } from './StyledComponent'; import { getDefaultValues, getFlattenRowsWithGroups, @@ -94,8 +93,8 @@ function CheckboxTree(props: CheckboxTreeProps) { ); return ( - <> - +
+ {flattenedRowsWithGroups.map((row) => row && isGroupWithRows(row) ? ( @@ -121,7 +120,7 @@ function CheckboxTree(props: CheckboxTreeProps) { ) )} - +
- +
); } diff --git a/ui/src/components/CheckboxTree/StyledComponent.tsx b/ui/src/components/CheckboxTree/StyledComponent.tsx index 6b80e603f..a9c86adc5 100644 --- a/ui/src/components/CheckboxTree/StyledComponent.tsx +++ b/ui/src/components/CheckboxTree/StyledComponent.tsx @@ -1,13 +1,8 @@ import styled, { css } from 'styled-components'; -import ColumnLayout from '@splunk/react-ui/ColumnLayout'; import CollapsiblePanel from '@splunk/react-ui/CollapsiblePanel'; import { pick, variables } from '@splunk/themes'; import Switch from '@splunk/react-ui/Switch'; -const FixedCheckboxRowWidth = css` - width: 320px; -`; - const CheckboxInHeader = css` align-self: center; background-color: ${pick({ @@ -15,10 +10,6 @@ const CheckboxInHeader = css` })}; `; -export const StyledColumnLayout = styled(ColumnLayout)` - ${FixedCheckboxRowWidth} -`; - export const CheckboxContainer = styled.div` display: flex; flex-direction: column; diff --git a/ui/src/components/CheckboxTree/stories/CheckboxTree.stories.tsx b/ui/src/components/CheckboxTree/stories/CheckboxTree.stories.tsx index 2dc56a807..6c703321a 100644 --- a/ui/src/components/CheckboxTree/stories/CheckboxTree.stories.tsx +++ b/ui/src/components/CheckboxTree/stories/CheckboxTree.stories.tsx @@ -2,10 +2,12 @@ import type { Meta, StoryObj } from '@storybook/react'; import { fn } from '@storybook/test'; import CheckboxTree from '../CheckboxTree'; import { MODE_CREATE, MODE_EDIT } from '../../../constants/modes'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: CheckboxTree, title: 'CheckboxTree/Component', + decorators: [withControlGroup], } satisfies Meta; export default meta; diff --git a/ui/src/components/ControlWrapper/ControlWrapper.tsx b/ui/src/components/ControlWrapper/ControlWrapper.tsx index 6014b3b8f..e4571daa1 100644 --- a/ui/src/components/ControlWrapper/ControlWrapper.tsx +++ b/ui/src/components/ControlWrapper/ControlWrapper.tsx @@ -8,22 +8,14 @@ import { AcceptableFormValueOrNullish } from '../../types/components/shareableTy import CustomControl from '../CustomControl/CustomControl'; import { Mode } from '../../constants/modes'; -const CustomElement = styled.div``; - const ControlGroupWrapper = styled(ControlGroup).attrs((props: { dataName: string }) => ({ 'data-name': props.dataName, }))` - max-width: 100%; - + // label width + control width + width: calc(260px + 320px); span[class*='ControlGroupStyles__StyledAsterisk-'] { color: red; } - - > * { - &:nth-child(3) { - width: 320px; - } - } `; export interface ControlWrapperProps { @@ -152,7 +144,7 @@ class ControlWrapper extends React.PureComponent { required={isFieldRequired} label={label} > - {rowView} + {rowView} ) ); diff --git a/ui/src/components/FileInputComponent/FileInputComponent.tsx b/ui/src/components/FileInputComponent/FileInputComponent.tsx index 115b0075c..30199f006 100644 --- a/ui/src/components/FileInputComponent/FileInputComponent.tsx +++ b/ui/src/components/FileInputComponent/FileInputComponent.tsx @@ -6,7 +6,7 @@ import FileConstants from '../../constants/fileInputConstant'; import { getFormattedMessage } from '../../util/messageUtil'; const FileWrapper = styled(File)` - width: 320px !important; + width: 100%; > div[class*='FileStyles__StyledHelp-'] { margin-bottom: 0px; } @@ -65,7 +65,7 @@ function FileInputComponent(props: FileInputComponentProps) { */ const [fileName, setFileName] = useState(fileNameToDisplay || ''); - /* + /* if the file data is encrypted and we display its name then we display error message "file needs to be reuploaded" as there is no access to data inside due to encription diff --git a/ui/src/components/FileInputComponent/stories/FileInputComponent.stories.tsx b/ui/src/components/FileInputComponent/stories/FileInputComponent.stories.tsx index 2433fa5c5..83de1608c 100644 --- a/ui/src/components/FileInputComponent/stories/FileInputComponent.stories.tsx +++ b/ui/src/components/FileInputComponent/stories/FileInputComponent.stories.tsx @@ -1,6 +1,7 @@ import type { Meta, StoryObj } from '@storybook/react'; import { fn } from '@storybook/test'; import FileInputComponent from '../FileInputComponent'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: FileInputComponent, @@ -10,6 +11,7 @@ const meta = { height: 300, }, }, + decorators: [withControlGroup], } satisfies Meta; export default meta; diff --git a/ui/src/components/FormModifications/FormModifications.test.tsx b/ui/src/components/FormModifications/FormModifications.test.tsx index 5e139b23e..12c21f043 100644 --- a/ui/src/components/FormModifications/FormModifications.test.tsx +++ b/ui/src/components/FormModifications/FormModifications.test.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render, within } from '@testing-library/react'; +import { screen, render, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { setUnifiedConfig } from '../../util/util'; import { @@ -55,27 +55,23 @@ const getTextElementForField = (field: string) => { return [componentParentElement, componentInput]; }; -const getCheckBoxElementForField = (field: string) => { - const componentParentElement = document.querySelector(`[data-name="${field}"]`)!; - expect(componentParentElement).toBeInTheDocument(); - const componentInput = within(componentParentElement).getByRole('checkbox'); - - return [componentParentElement, componentInput]; -}; - beforeEach(() => { setUpConfigWithDefaultValue(); renderModalWithProps(props); }); it('render fields with modifications correctly', async () => { - expect(() => { - getTextElementForField(firstStandardTextField.field); - getTextElementForField(secondStandardTextField.field); - getTextElementForField(firstModificationField.field); - getTextElementForField(secondModificationField.field); - getCheckBoxElementForField(thirdModificationField.field); - }).not.toThrow(); + expect(screen.getByRole('textbox', { name: firstStandardTextField.label })).toBeInTheDocument(); + expect( + screen.getByRole('textbox', { name: secondStandardTextField.label }) + ).toBeInTheDocument(); + expect(screen.getByRole('textbox', { name: firstModificationField.label })).toBeInTheDocument(); + expect( + screen.getByRole('textbox', { name: secondModificationField.label }) + ).toBeInTheDocument(); + expect( + screen.getByRole('checkbox', { name: thirdModificationField.label }) + ).toBeInTheDocument(); }); it('verify modification after text components change', async () => { diff --git a/ui/src/components/MultiInputComponent/MultiInputComponent.test.tsx b/ui/src/components/MultiInputComponent/MultiInputComponent.test.tsx index fed1af8a3..78ae8ec11 100644 --- a/ui/src/components/MultiInputComponent/MultiInputComponent.test.tsx +++ b/ui/src/components/MultiInputComponent/MultiInputComponent.test.tsx @@ -28,7 +28,6 @@ const defaultInputProps = { }, disabled: false, value: 'defaultValue', - error: false, dependencyValues: {}, handleChange, } satisfies MultiInputComponentProps; @@ -81,11 +80,10 @@ it('renders as disabled correctly', () => { it.each(defaultInputProps.controlOptions.items)('handler called correctly', async (item) => { renderFeature(); - const inputComponent = screen.getByTestId('multiselect'); - + const inputComponent = screen.getByRole('combobox'); await userEvent.click(inputComponent); - const option = document.querySelector(`[data-test-value="${item.value}"]`); + const option = screen.getByRole('option', { name: item.label }); expect(option).toBeInTheDocument(); if (option) { @@ -111,14 +109,15 @@ it.each(mockedEntries)('handler endpoint data loading', async (mockedEntry) => { value: undefined, }); - const inputComponent = screen.getByTestId('multiselect'); + const inputComponent = screen.getByRole('listbox'); + expect(inputComponent).toBeInTheDocument(); await userEvent.click(inputComponent); const apiEntry = mockedEntry; - const option = document.querySelector(`[data-test-value="${apiEntry.name}"]`); + const option = screen.getByRole('option', { name: apiEntry.name }); expect(option).toBeInTheDocument(); if (option) { await userEvent.click(option); @@ -143,13 +142,14 @@ describe.each(mockedEntries)('handler endpoint data loading', (mockedEntry) => { value: undefined, }); - const inputComponent = screen.getByTestId('multiselect'); + const inputComponent = screen.getByRole('listbox'); + expect(inputComponent).toBeInTheDocument(); await userEvent.click(inputComponent); const apiEntry = mockedEntry; - const option = document.querySelector(`[data-test-value="${apiEntry.content.testValue}"]`); + const option = screen.getByRole('option', { name: apiEntry.content.testLabel }); expect(option).toBeInTheDocument(); if (option) { await userEvent.click(option); @@ -181,11 +181,15 @@ it('should render label (boolean-like)', () => { ], }, }); - const inputComponent = screen.getByTestId('multiselect'); - - expect(within(inputComponent).getByText('truevalue')).toBeInTheDocument(); - expect(within(inputComponent).queryByText('falsevalue')).not.toBeInTheDocument(); - expect(within(inputComponent).queryByText('optionone')).not.toBeInTheDocument(); + const inputComponent = screen.getByRole('listbox'); + + expect(within(inputComponent).getByRole('option', { name: /truevalue/ })).toBeInTheDocument(); + expect( + within(inputComponent).queryByRole('option', { name: /falsevalue/ }) + ).not.toBeInTheDocument(); + expect( + within(inputComponent).queryByRole('option', { name: /optionone/ }) + ).not.toBeInTheDocument(); }); it('should render singe value (numeric)', () => { @@ -204,10 +208,12 @@ it('should render singe value (numeric)', () => { ], }, }); - const inputComponent = screen.getByTestId('multiselect'); + const inputComponent = screen.getByRole('listbox'); - expect(within(inputComponent).getByText('label1')).toBeInTheDocument(); - expect(within(inputComponent).queryByText('label2')).not.toBeInTheDocument(); + expect(within(inputComponent).getByRole('option', { name: /label1/ })).toBeInTheDocument(); + expect( + within(inputComponent).queryByRole('option', { name: /label2/ }) + ).not.toBeInTheDocument(); }); it('should render two values (number + boolean)', () => { @@ -231,9 +237,12 @@ it('should render two values (number + boolean)', () => { ], }, }); - const inputComponent = screen.getByTestId('multiselect'); - expect(within(inputComponent).getByText('label1')).toBeInTheDocument(); - expect(within(inputComponent).getByText('label2')).toBeInTheDocument(); - expect(within(inputComponent).queryByText('label3')).not.toBeInTheDocument(); + const inputComponent = screen.getByRole('listbox'); + + expect(within(inputComponent).getByRole('option', { name: 'label1' })).toBeInTheDocument(); + expect(within(inputComponent).getByRole('option', { name: 'label2' })).toBeInTheDocument(); + expect( + within(inputComponent).queryByRole('option', { name: 'label3' }) + ).not.toBeInTheDocument(); }); diff --git a/ui/src/components/MultiInputComponent/MultiInputComponent.tsx b/ui/src/components/MultiInputComponent/MultiInputComponent.tsx index 58e944abb..e003a22fa 100644 --- a/ui/src/components/MultiInputComponent/MultiInputComponent.tsx +++ b/ui/src/components/MultiInputComponent/MultiInputComponent.tsx @@ -4,16 +4,13 @@ import styled from 'styled-components'; import WaitSpinner from '@splunk/react-ui/WaitSpinner'; import { z } from 'zod'; +import { MultiselectChangeHandler } from '@splunk/react-ui/types/src/Multiselect/Multiselect'; import { RequestParams, generateEndPointUrl, getRequest } from '../../util/api'; import { filterResponse, FilterResponseParams } from '../../util/util'; import { MultipleSelectCommonOptions } from '../../types/globalConfig/entities'; import { invariant } from '../../util/invariant'; import { AcceptableFormValue } from '../../types/components/shareableTypes'; -const MultiSelectWrapper = styled(Multiselect)` - width: 320px !important; -`; - const WaitSpinnerWrapper = styled(WaitSpinner)` margin-left: 5px; `; @@ -25,7 +22,6 @@ export interface MultiInputComponentProps { controlOptions: z.TypeOf; disabled?: boolean; value?: AcceptableFormValue; - error?: boolean; dependencyValues?: Record; } @@ -34,11 +30,11 @@ function MultiInputComponent(props: MultiInputComponentProps) { id, field, disabled = false, - error = false, value, controlOptions, dependencyValues, - ...restProps + handleChange, + ...restSuiProps } = props; const { endpointUrl, @@ -53,11 +49,11 @@ function MultiInputComponent(props: MultiInputComponentProps) { delimiter = ',', } = controlOptions; - function handleChange(e: unknown, { values }: { values: (string | number | boolean)[] }) { + const onChange: MultiselectChangeHandler = (_e, { values }) => { if (typeof values[0] === 'string' || values.length === 0) { - restProps.handleChange(field, values.join(delimiter)); + handleChange(field, values.join(delimiter)); } - } + }; function generateOptions(itemList: { label: string; value: string | number | boolean }[]) { return itemList.map((item) => ( @@ -137,18 +133,18 @@ function MultiInputComponent(props: MultiInputComponentProps) { return ( <> - {options && options.length > 0 && options} - + {loadingIndicator} ); diff --git a/ui/src/components/MultiInputComponent/stories/MultiInputComponent.stories.tsx b/ui/src/components/MultiInputComponent/stories/MultiInputComponent.stories.tsx index 9c84f05ad..bffcb02a3 100644 --- a/ui/src/components/MultiInputComponent/stories/MultiInputComponent.stories.tsx +++ b/ui/src/components/MultiInputComponent/stories/MultiInputComponent.stories.tsx @@ -7,6 +7,7 @@ import MultiInputComponent from '../MultiInputComponent'; import { getGlobalConfigMock } from '../../../mocks/globalConfigMock'; import { setUnifiedConfig } from '../../../util/util'; import { getMockServerResponseForInput } from '../../../mocks/server-response'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: MultiInputComponent, @@ -23,13 +24,14 @@ const meta = { { + handleChange={(field, data) => { setState(data); props.handleChange(field, data); }} /> ); }, + decorators: [withControlGroup], } satisfies Meta; export default meta; @@ -70,7 +72,6 @@ export const AllProps: Story = { }, disabled: false, value: undefined, - error: false, dependencyValues: {}, }, }; @@ -91,7 +92,6 @@ export const EndpointApi: Story = { }, disabled: false, value: undefined, - error: false, dependencyValues: {}, }, parameters: { diff --git a/ui/src/components/RadioComponent/RadioComponent.tsx b/ui/src/components/RadioComponent/RadioComponent.tsx index f6cbd2ebe..7eaceecd4 100644 --- a/ui/src/components/RadioComponent/RadioComponent.tsx +++ b/ui/src/components/RadioComponent/RadioComponent.tsx @@ -4,10 +4,6 @@ import styled from 'styled-components'; import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy'; import { StandardPages } from '../../types/components/shareableTypes'; -const RadioBarWrapper = styled(RadioBar)` - width: 320px; -`; - const RadioBarOption = styled(RadioBar.Option)` margin-left: 0px !important; `; @@ -33,27 +29,22 @@ class RadioComponent extends Component { }; render() { + const { value, controlOptions, disabled, page, ...restSuiProps } = this.props; return ( - - {this.props.controlOptions.items.map((item) => ( + {controlOptions.items.map((item) => ( ))} - + ); } } diff --git a/ui/src/components/RadioComponent/stories/RadioComponent.stories.ts b/ui/src/components/RadioComponent/stories/RadioComponent.stories.ts index 59d8f8e35..b1ffb7a80 100644 --- a/ui/src/components/RadioComponent/stories/RadioComponent.stories.ts +++ b/ui/src/components/RadioComponent/stories/RadioComponent.stories.ts @@ -1,10 +1,12 @@ import type { Meta, StoryObj } from '@storybook/react'; import { fn } from '@storybook/test'; import RadioComponent from '../RadioComponent'; +import { withControlGroup } from '../../../../.storybook/withControlGroup'; const meta = { component: RadioComponent, title: 'RadioComponent', + decorators: [withControlGroup], } satisfies Meta; export default meta; diff --git a/ui/src/components/SingleInputComponent/SingleInputComponent.test.tsx b/ui/src/components/SingleInputComponent/SingleInputComponent.test.tsx index e8563e1e1..d889a7a45 100644 --- a/ui/src/components/SingleInputComponent/SingleInputComponent.test.tsx +++ b/ui/src/components/SingleInputComponent/SingleInputComponent.test.tsx @@ -27,7 +27,6 @@ const defaultInputProps = { }, disabled: false, value: 'defaultValue', - error: false, dependencyValues: {}, required: false, handleChange, @@ -65,9 +64,9 @@ const mockAPI = () => { it('renders correctly', () => { renderFeature(); - const inputComponent = screen.getByTestId('combo-box'); + const inputComponent = screen.getByRole('combobox'); expect(inputComponent).toBeInTheDocument(); - expect(inputComponent.getAttribute('data-test-value')).toEqual(defaultInputProps.value); + expect(inputComponent).toHaveValue(defaultInputProps.value); }); it('renders as disabled correctly', () => { @@ -81,11 +80,11 @@ it.each(defaultInputProps.controlOptions.autoCompleteFields)( 'handler called correctly', async (item) => { renderFeature({ value: undefined }); - const inputComponent = screen.getByTestId('combo-box'); + const inputComponent = screen.getByRole('combobox'); await userEvent.click(inputComponent); - const option = document.querySelector(`[data-test-value="${item.value}"]`); + const option = screen.getByRole('option', { name: item.label }); expect(option).toBeInTheDocument(); if (option) { await userEvent.click(option); @@ -96,9 +95,9 @@ it.each(defaultInputProps.controlOptions.autoCompleteFields)( it('clear calls handler with empty data', async () => { renderFeature(); - const inputComponent = screen.getByTestId('combo-box'); + const inputComponent = screen.getByRole('combobox'); await userEvent.click(inputComponent); - const clearBtn = screen.getByTestId('clear'); + const clearBtn = screen.getByLabelText(/clear/i); await userEvent.click(clearBtn); expect(handleChange).toHaveBeenCalledWith(defaultInputProps.field, ``); }); @@ -116,10 +115,11 @@ describe.each(mockedEntries)('handler endpoint data loading', (entry) => { valueField: 'testValue', }, }); - const inputComponent = screen.getByTestId('combo-box'); + const inputComponent = screen.getByRole('combobox'); + await userEvent.click(inputComponent); - const option = document.querySelector(`[data-test-value="${entry.content.testValue}"]`); + const option = screen.getByRole('option', { name: entry.content.testLabel }); expect(option).toBeInTheDocument(); if (option) { diff --git a/ui/src/components/SingleInputComponent/SingleInputComponent.tsx b/ui/src/components/SingleInputComponent/SingleInputComponent.tsx index 7b32f749c..60535cd4f 100755 --- a/ui/src/components/SingleInputComponent/SingleInputComponent.tsx +++ b/ui/src/components/SingleInputComponent/SingleInputComponent.tsx @@ -7,24 +7,19 @@ import styled from 'styled-components'; import WaitSpinner from '@splunk/react-ui/WaitSpinner'; import { z } from 'zod'; +import { variables } from '@splunk/themes'; import { RequestParams, generateEndPointUrl, getRequest } from '../../util/api'; import { SelectCommonOptions } from '../../types/globalConfig/entities'; import { filterResponse, FilterResponseParams } from '../../util/util'; import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy'; import { AcceptableFormValue, StandardPages } from '../../types/components/shareableTypes'; -const SelectWrapper = styled(Select)` - width: 320px !important; -`; - const WaitSpinnerWrapper = styled(WaitSpinner)` - margin-left: 5px; + margin-left: ${variables.spacingSmall}; `; -const StyledDiv = styled.div` - div:first-child { - width: 320px !important; - } +const StyledClearButton = styled(Button)` + margin-left: ${variables.spacingSmall}; `; type BasicFormItem = { value: AcceptableFormValue; label: string }; @@ -40,7 +35,6 @@ export interface SingleInputComponentProps { id?: string; disabled?: boolean; value: AcceptableFormValue; - error?: boolean; handleChange: (field: string, value: string) => void; field: string; dependencyValues?: Record; @@ -52,14 +46,7 @@ export interface SingleInputComponentProps { } function SingleInputComponent(props: SingleInputComponentProps) { - const { - field, - disabled = false, - error = false, - controlOptions, - dependencyValues, - ...restProps - } = props; + const { id, field, disabled = false, controlOptions, dependencyValues, ...restProps } = props; const { endpointUrl, denyList, @@ -175,8 +162,11 @@ function SingleInputComponent(props: SingleInputComponentProps) { // effectiveIsClearable button will be visible only for the required=false and createSearchChoice=false single-select fields. const effectiveIsClearable = !(effectiveDisabled || restProps.required || hideClearBtn); return createSearchChoice ? ( - + <> {options && options.length > 0 && options} {loadingIndicator} - + ) : ( <> - {options && options.length > 0 && options} - {' '} + {' '} {loadingIndicator} {effectiveIsClearable ? ( -
+ ); } diff --git a/ui/src/components/CheckboxTree/CheckboxTree.tsx b/ui/src/components/CheckboxTree/CheckboxTree.tsx index a36a69a9a..1ee506901 100644 --- a/ui/src/components/CheckboxTree/CheckboxTree.tsx +++ b/ui/src/components/CheckboxTree/CheckboxTree.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useState, useCallback, useMemo } from 'react'; import ColumnLayout from '@splunk/react-ui/ColumnLayout'; import Button from '@splunk/react-ui/Button'; +import styled from 'styled-components'; import { getDefaultValues, getFlattenRowsWithGroups, @@ -13,6 +14,10 @@ import { MODE_CREATE } from '../../constants/modes'; import { CheckboxTreeProps, ValueByField } from './types'; import { packValue, parseValue } from './utils'; +const FullWidth = styled.div` + width: 100%; +`; + function CheckboxTree(props: CheckboxTreeProps) { const { field, handleChange, controlOptions, disabled } = props; @@ -93,7 +98,7 @@ function CheckboxTree(props: CheckboxTreeProps) { ); return ( -
+ {flattenedRowsWithGroups.map((row) => row && isGroupWithRows(row) ? ( @@ -133,7 +138,7 @@ function CheckboxTree(props: CheckboxTreeProps) { onClick={() => handleCheckboxToggleAll(false)} />
- + ); } diff --git a/ui/src/components/ControlWrapper/utils.ts b/ui/src/components/ControlWrapper/utils.ts new file mode 100644 index 000000000..3d80b5556 --- /dev/null +++ b/ui/src/components/ControlWrapper/utils.ts @@ -0,0 +1,27 @@ +const excludeProps = ( + obj: T, + keys: K[] +): Omit> => { + const result = { ...obj }; + keys.forEach((key) => { + if (key in result) { + // @ts-expect-error allow to pass keys that does not exist + delete result[key]; + } + }); + return result; +}; +export const excludeControlWrapperProps = (obj: T) => + excludeProps(obj, [ + 'addCustomValidator', + 'defaultValue', // value is provided anyway, so no need to pass defaultValue + 'dependencyValues', + 'encrypted', + 'fileNameToDisplay', + 'handleChange', + 'mode', + 'options', + 'page', + 'style', + 'type', + ]); diff --git a/ui/src/components/MultiInputComponent/MultiInputComponent.tsx b/ui/src/components/MultiInputComponent/MultiInputComponent.tsx index e003a22fa..90dd95483 100644 --- a/ui/src/components/MultiInputComponent/MultiInputComponent.tsx +++ b/ui/src/components/MultiInputComponent/MultiInputComponent.tsx @@ -10,6 +10,7 @@ import { filterResponse, FilterResponseParams } from '../../util/util'; import { MultipleSelectCommonOptions } from '../../types/globalConfig/entities'; import { invariant } from '../../util/invariant'; import { AcceptableFormValue } from '../../types/components/shareableTypes'; +import { excludeControlWrapperProps } from '../ControlWrapper/utils'; const WaitSpinnerWrapper = styled(WaitSpinner)` margin-left: 5px; @@ -34,7 +35,7 @@ function MultiInputComponent(props: MultiInputComponentProps) { controlOptions, dependencyValues, handleChange, - ...restSuiProps + ...restProps } = props; const { endpointUrl, @@ -130,6 +131,7 @@ function MultiInputComponent(props: MultiInputComponentProps) { const loadingIndicator = loading ? : null; const valueList = value ? String(value).split(delimiter) : []; + const restSuiProps = excludeControlWrapperProps(restProps); return ( <> diff --git a/ui/src/components/RadioComponent/RadioComponent.tsx b/ui/src/components/RadioComponent/RadioComponent.tsx index 7eaceecd4..b291b6656 100644 --- a/ui/src/components/RadioComponent/RadioComponent.tsx +++ b/ui/src/components/RadioComponent/RadioComponent.tsx @@ -4,6 +4,8 @@ import styled from 'styled-components'; import { getValueMapTruthyFalse } from '../../util/considerFalseAndTruthy'; import { StandardPages } from '../../types/components/shareableTypes'; +import { excludeControlWrapperProps } from '../ControlWrapper/utils'; + const RadioBarOption = styled(RadioBar.Option)` margin-left: 0px !important; `; @@ -29,7 +31,9 @@ class RadioComponent extends Component { }; render() { - const { value, controlOptions, disabled, page, ...restSuiProps } = this.props; + const { value, controlOptions, disabled, page, ...restProps } = this.props; + + const restSuiProps = excludeControlWrapperProps(restProps); return ( { - restProps.handleChange(field, String(obj.value)); + props.handleChange(field, String(obj.value)); }; const Option = createSearchChoice ? ComboBox.Option : Select.Option; const Heading = createSearchChoice ? ComboBox.Heading : Select.Heading; @@ -160,11 +161,14 @@ function SingleInputComponent(props: SingleInputComponentProps) { const loadingIndicator = loading ? : null; // hideClearBtn=true only passed for OAuth else its undefined // effectiveIsClearable button will be visible only for the required=false and createSearchChoice=false single-select fields. - const effectiveIsClearable = !(effectiveDisabled || restProps.required || hideClearBtn); + const effectiveIsClearable = !(effectiveDisabled || props.required || hideClearBtn); + + // ControlWrapper passes a lot of extra props that conflict with SUI components + const restSuiProps = excludeControlWrapperProps(restProps); return createSearchChoice ? ( <>