From 16a213a616c4e5d328e344797323abdf910e7a53 Mon Sep 17 00:00:00 2001 From: Andrey Yamanov Date: Thu, 18 Aug 2022 20:02:10 +0200 Subject: [PATCH] fix(Field): respect onSelectionChange event of ComboBox (#177) --- .changeset/orange-ties-itch.md | 5 +++ .size-limit.js | 2 +- src/components/forms/Form/Field.tsx | 29 ++++++++++++----- src/components/forms/Form/types.ts | 1 + src/components/forms/Form/useForm.tsx | 31 ++++++++++++++----- .../Bar/FloatingNotification.tsx | 2 +- .../NewNotifications/Bar/NotificationsBar.tsx | 2 +- .../NotificationCloseButton.tsx | 2 +- .../NotificationView/notification.test.tsx | 4 +-- .../Notifications.stories.tsx | 4 +-- .../NewNotifications/notification.test.tsx | 4 +-- .../overlays/Toasts/Toasts.stories.tsx | 2 +- src/components/overlays/Toasts/toast.test.tsx | 10 +++--- src/components/pickers/ComboBox/ComboBox.tsx | 2 ++ src/components/pickers/Menu/Menu.stories.tsx | 8 ++--- src/components/pickers/Menu/Menu.tsx | 5 ++- 16 files changed, 76 insertions(+), 37 deletions(-) create mode 100644 .changeset/orange-ties-itch.md diff --git a/.changeset/orange-ties-itch.md b/.changeset/orange-ties-itch.md new file mode 100644 index 00000000..778fa44a --- /dev/null +++ b/.changeset/orange-ties-itch.md @@ -0,0 +1,5 @@ +--- +"@cube-dev/ui-kit": patch +--- + +ComboBox now respects `onSelectionChange` event while working inside a form. diff --git a/.size-limit.js b/.size-limit.js index 0e03c830..34e36458 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -18,7 +18,7 @@ module.exports = [ }), ); }, - limit: '250kB', + limit: '200kB', }, { name: 'Tree shaking (just a Button)', diff --git a/src/components/forms/Form/Field.tsx b/src/components/forms/Form/Field.tsx index 9ab56c3b..0f508bea 100644 --- a/src/components/forms/Form/Field.tsx +++ b/src/components/forms/Form/Field.tsx @@ -54,7 +54,12 @@ function getDefaultValidateTrigger(type) { return type === 'Number' || type.includes('Text') ? 'onBlur' : 'onChange'; } -function getValueProps(type, value?, onChange?) { +function getValueProps( + type?: string, + value?, + onChange?, + allowsCustomValue?: boolean, +) { type = type || ''; if (type === 'Number') { @@ -84,7 +89,8 @@ function getValueProps(type, value?, onChange?) { } else if (type === 'ComboBox') { return { inputValue: value != null ? value : '', - onInputChange: onChange, + onInputChange: (val) => onChange(val, !allowsCustomValue), + onSelectionChange: onChange, }; } else if (type === 'Select') { return { @@ -110,7 +116,7 @@ export interface CubeFieldProps /** The id prefix for the field to avoid collisions between forms */ idPrefix?: string; children?: ReactElement | ((CubeFormInstance) => ReactElement); - /** Function that check whether to perform update of the form state. */ + /** Function that checks whether to perform update of the form state. */ shouldUpdate?: boolean | ((prevValues, nextValues) => boolean); /** Validation rules */ rules?: ValidationRule[]; @@ -290,6 +296,7 @@ export function Field(allProps: CubeFieldProps) { child, mergeProps(child.props, { ...getValueProps(inputType), + label: fieldName, name: fieldName, id: fieldId, }), @@ -300,7 +307,7 @@ export function Field(allProps: CubeFieldProps) { validateTrigger = defaultValidateTrigger; } - function onChangeHandler(val) { + function onChangeHandler(val, dontTouch) { const field = form.getFieldInstance(fieldName); if (shouldUpdate) { @@ -320,11 +327,12 @@ export function Field(allProps: CubeFieldProps) { } } - form.setFieldValue(fieldName, val, true); + form.setFieldValue(fieldName, val, !dontTouch, false, dontTouch); if ( - validateTrigger === 'onChange' || - (field && field.errors && field.errors.length) + !dontTouch && + (validateTrigger === 'onChange' || + (field && field.errors && field.errors.length)) ) { form.validateField(fieldName).catch(() => {}); // do nothing on fail } @@ -399,7 +407,12 @@ export function Field(allProps: CubeFieldProps) { Object.assign( newProps, - getValueProps(inputType, field?.value, onChangeHandler), + getValueProps( + inputType, + field?.inputValue, + onChangeHandler, + child.props.allowsCustomValue, + ), ); const { onChange, onSelectionChange, ...childProps } = child.props; diff --git a/src/components/forms/Form/types.ts b/src/components/forms/Form/types.ts index d1ea934f..b663603d 100644 --- a/src/components/forms/Form/types.ts +++ b/src/components/forms/Form/types.ts @@ -6,6 +6,7 @@ export type CubeFieldData = { readonly name: Name; errors: string[]; value?: Value; + inputValue?: Value; touched?: boolean; rules?: any[]; validating?: boolean; diff --git a/src/components/forms/Form/useForm.tsx b/src/components/forms/Form/useForm.tsx index 4f7be7cc..179baa6d 100644 --- a/src/components/forms/Form/useForm.tsx +++ b/src/components/forms/Form/useForm.tsx @@ -1,7 +1,7 @@ import { useRef, useState } from 'react'; import { dotize } from '../../../tasty'; import { applyRules } from './validation'; -import { FieldTypes, CubeFieldData, SetFieldsArrType } from './types'; +import { CubeFieldData, FieldTypes, SetFieldsArrType } from './types'; export type CubeFormData = { [K in keyof T]?: CubeFieldData; @@ -28,7 +28,7 @@ export class CubeFormInstance< TFormData extends CubeFormData = CubeFormData, > { public forceReRender: () => void = () => {}; - private initialFields = {}; + private initialFields: Partial = {}; private fields: TFormData = {} as TFormData; public ref = {}; public isSubmitting = false; @@ -64,6 +64,7 @@ export class CubeFormInstance< touched?: boolean, skipRender?: boolean, createFields = false, + inputOnly = false, ) => { let flag = false; @@ -85,7 +86,11 @@ export class CubeFormInstance< flag = true; - field.value = newData[name]; + if (!inputOnly) { + field.value = newData[name]; + } + + field.inputValue = newData[name]; if (touched === true) { field.touched = touched; @@ -98,7 +103,7 @@ export class CubeFormInstance< if (flag && !skipRender) { this.forceReRender(); - if (touched) { + if (touched && !inputOnly) { this.onValuesChange && this.onValuesChange(this.getFormData()); } } @@ -138,6 +143,7 @@ export class CubeFormInstance< value: T[Name], touched = false, skipRender = false, + inputOnly = false, ) { const field = this.fields[name]; @@ -145,7 +151,11 @@ export class CubeFormInstance< return; } - field.value = value; + if (!inputOnly) { + field.value = value; + } + + field.inputValue = value; if (touched) { field.touched = touched; @@ -157,7 +167,7 @@ export class CubeFormInstance< this.forceReRender(); } - if (touched) { + if (touched && !inputOnly) { this.onValuesChange && this.onValuesChange(this.getFormData()); } } @@ -309,13 +319,20 @@ export class CubeFormInstance< name: Name, data?: Data, ): Data { - return { + let obj = { name, validating: false, touched: false, errors: [], ...data, } as unknown as Data; + + if (obj) { + // condition is here to avoid typing issues + obj.inputValue = obj.value; + } + + return obj; } } diff --git a/src/components/overlays/NewNotifications/Bar/FloatingNotification.tsx b/src/components/overlays/NewNotifications/Bar/FloatingNotification.tsx index 7f3d4b92..b0d548eb 100644 --- a/src/components/overlays/NewNotifications/Bar/FloatingNotification.tsx +++ b/src/components/overlays/NewNotifications/Bar/FloatingNotification.tsx @@ -81,7 +81,7 @@ export const FloatingNotification = memo(function FloatingNotification( }} onDismiss={chainedOnDismiss} onClose={onCloseEvent} - qa="floating-notification" + qa="FloatingNotification" /> ); diff --git a/src/components/overlays/NewNotifications/Bar/NotificationsBar.tsx b/src/components/overlays/NewNotifications/Bar/NotificationsBar.tsx index c24ab4fb..7c1e50bd 100644 --- a/src/components/overlays/NewNotifications/Bar/NotificationsBar.tsx +++ b/src/components/overlays/NewNotifications/Bar/NotificationsBar.tsx @@ -71,7 +71,7 @@ export function NotificationsBar(props: NotificationsBarProps): JSX.Element { return ( ', () => { const notification = screen.getByTestId('notification'); - await userEvent.click( - getByTestId(notification, 'notification-close-button'), - ); + await userEvent.click(getByTestId(notification, 'NotificationCloseButton')); expect(onClose).toBeCalledTimes(1); }); diff --git a/src/components/overlays/NewNotifications/Notifications.stories.tsx b/src/components/overlays/NewNotifications/Notifications.stories.tsx index f2515427..03cbc89c 100644 --- a/src/components/overlays/NewNotifications/Notifications.stories.tsx +++ b/src/components/overlays/NewNotifications/Notifications.stories.tsx @@ -42,7 +42,7 @@ DefaultAction.play = async ({ canvasElement }) => { const button = getByRole('button'); await userEvent.click(button); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); await expect(notification).toBeInTheDocument(); }; @@ -389,7 +389,7 @@ WithLongActions.play = async ({ canvasElement }) => { const { getByRole, getByTestId } = within(canvasElement); await userEvent.click(getByRole('button')); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); await expect(notification).toBeInTheDocument(); }; diff --git a/src/components/overlays/NewNotifications/notification.test.tsx b/src/components/overlays/NewNotifications/notification.test.tsx index eedf6f8d..ce40eac6 100644 --- a/src/components/overlays/NewNotifications/notification.test.tsx +++ b/src/components/overlays/NewNotifications/notification.test.tsx @@ -13,7 +13,7 @@ describe('', () => { it('should unmount component by default', () => { const { getByTestId, rerender } = renderWithRoot(); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); rerender( , @@ -27,7 +27,7 @@ describe('', () => { , ); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); rerender( , diff --git a/src/components/overlays/Toasts/Toasts.stories.tsx b/src/components/overlays/Toasts/Toasts.stories.tsx index d3b38cbf..ec0c5415 100644 --- a/src/components/overlays/Toasts/Toasts.stories.tsx +++ b/src/components/overlays/Toasts/Toasts.stories.tsx @@ -28,7 +28,7 @@ UseToast.play = async ({ canvasElement }) => { const button = getByRole('button'); await userEvent.click(button); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); await expect(notification).toBeInTheDocument(); }; diff --git a/src/components/overlays/Toasts/toast.test.tsx b/src/components/overlays/Toasts/toast.test.tsx index 422d5e9b..53386b80 100644 --- a/src/components/overlays/Toasts/toast.test.tsx +++ b/src/components/overlays/Toasts/toast.test.tsx @@ -20,7 +20,7 @@ describe('useToastsApi', () => { }); expect( - screen.queryByTestId('floating-notification'), + screen.queryByTestId('FloatingNotification'), ).not.toBeInTheDocument(); expect(dismiss).toBeCalledTimes(1); @@ -37,7 +37,7 @@ describe('useToastsApi', () => { jest.runAllTimers(); }); - expect(screen.getByTestId('floating-notification')).toBeInTheDocument(); + expect(screen.getByTestId('FloatingNotification')).toBeInTheDocument(); expect(dismiss).not.toBeCalled(); }); @@ -55,14 +55,14 @@ describe('useToastsApi', () => { expect(dismiss).toBeCalled(); expect( - screen.queryByTestId('floating-notification'), + screen.queryByTestId('FloatingNotification'), ).not.toBeInTheDocument(); }); it('should unmount component by default', () => { const { getByTestId, rerender } = renderWithRoot(); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); rerender( , @@ -76,7 +76,7 @@ describe('useToastsApi', () => { , ); - const notification = getByTestId('floating-notification'); + const notification = getByTestId('FloatingNotification'); rerender( , diff --git a/src/components/pickers/ComboBox/ComboBox.tsx b/src/components/pickers/ComboBox/ComboBox.tsx index a1edfaf5..6b8f1587 100644 --- a/src/components/pickers/ComboBox/ComboBox.tsx +++ b/src/components/pickers/ComboBox/ComboBox.tsx @@ -160,6 +160,7 @@ function ComboBox(props: CubeComboBoxProps, ref) { labelSuffix, ...otherProps } = props; + let isAsync = loadingState != null; let { contains } = useFilter({ sensitivity: 'base' }); let [suffixWidth, setSuffixWidth] = useState(0); @@ -294,6 +295,7 @@ function ComboBox(props: CubeComboBoxProps, ref) { {suffix} {!hideTrigger ? ( {