fix(Field): respect onSelectionChange event of ComboBox (#177)

cube-js · Aug 18, 2022 · 16a213a · 16a213a
1 parent 4f2d9d5
commit 16a213a
Show file tree

Hide file tree

Showing 16 changed files with 76 additions and 37 deletions.
diff --git a/.changeset/orange-ties-itch.md 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
@@ -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
@@ -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<T extends FieldTypes>
   /** 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<T extends FieldTypes>(allProps: CubeFieldProps<T>) {
       child,
       mergeProps(child.props, {
         ...getValueProps(inputType),
+        label: fieldName,
         name: fieldName,
         id: fieldId,
       }),
@@ -300,7 +307,7 @@ export function Field<T extends FieldTypes>(allProps: CubeFieldProps<T>) {
     validateTrigger = defaultValidateTrigger;
   }
 
-  function onChangeHandler(val) {
+  function onChangeHandler(val, dontTouch) {
     const field = form.getFieldInstance(fieldName);
 
     if (shouldUpdate) {
@@ -320,11 +327,12 @@ export function Field<T extends FieldTypes>(allProps: CubeFieldProps<T>) {
       }
     }
 
-    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<T extends FieldTypes>(allProps: CubeFieldProps<T>) {
 
   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
@@ -6,6 +6,7 @@ export type CubeFieldData<Name extends string | number | symbol, Value> = {
   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
@@ -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<T extends FieldTypes> = {
   [K in keyof T]?: CubeFieldData<K, T[K]>;
@@ -28,7 +28,7 @@ export class CubeFormInstance<
   TFormData extends CubeFormData<T> = CubeFormData<T>,
 > {
   public forceReRender: () => void = () => {};
-  private initialFields = {};
+  private initialFields: Partial<T> = {};
   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,14 +143,19 @@ export class CubeFormInstance<
     value: T[Name],
     touched = false,
     skipRender = false,
+    inputOnly = false,
   ) {
     const field = this.fields[name];
 
     if (!field || isEqual(value, field.value)) {
       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
@@ -81,7 +81,7 @@ export const FloatingNotification = memo(function FloatingNotification(
         }}
         onDismiss={chainedOnDismiss}
         onClose={onCloseEvent}
-        qa="floating-notification"
+        qa="FloatingNotification"
       />
     </NotificationContainer>
   );

diff --git 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 (
     <NotificationsContainer
       ref={ref}
-      data-qa="notifications-bar"
+      qa="NotificationsBar"
       role="region"
       aria-live="polite"
       {...mergeProps(listProps, hoverProps, focusProps)}

diff --git a/src/components/overlays/NewNotifications/NotificationView/NotificationCloseButton.tsx b/src/components/overlays/NewNotifications/NotificationView/NotificationCloseButton.tsx
@@ -36,7 +36,7 @@ export const NotificationCloseButton = memo(function NotificationCloseButton(
 
   return (
     <CloseButton
-      qa="notification-close-button"
+      qa="NotificationCloseButton"
       type="neutral"
       mods={{ show: isHovered || isFocused }}
       onPress={onPress}

diff --git a/src/components/overlays/NewNotifications/NotificationView/notification.test.tsx b/src/components/overlays/NewNotifications/NotificationView/notification.test.tsx
@@ -41,9 +41,7 @@ describe('<Notification />', () => {
 
     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
@@ -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
@@ -13,7 +13,7 @@ describe('<Notification />', () => {
   it('should unmount component by default', () => {
     const { getByTestId, rerender } = renderWithRoot(<TestComponent />);
 
-    const notification = getByTestId('floating-notification');
+    const notification = getByTestId('FloatingNotification');
 
     rerender(
       <TestComponent disableRemoveOnUnmount renderNotification={false} />,
@@ -27,7 +27,7 @@ describe('<Notification />', () => {
       <TestComponent disableRemoveOnUnmount />,
     );
 
-    const notification = getByTestId('floating-notification');
+    const notification = getByTestId('FloatingNotification');
 
     rerender(
       <TestComponent disableRemoveOnUnmount renderNotification={false} />,

diff --git 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
@@ -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(<TestComponent />);
 
-    const notification = getByTestId('floating-notification');
+    const notification = getByTestId('FloatingNotification');
 
     rerender(
       <TestComponent disableRemoveOnUnmount renderNotification={false} />,
@@ -76,7 +76,7 @@ describe('useToastsApi', () => {
       <TestComponent disableRemoveOnUnmount />,
     );
 
-    const notification = getByTestId('floating-notification');
+    const notification = getByTestId('FloatingNotification');
 
     rerender(
       <TestComponent disableRemoveOnUnmount renderNotification={false} />,

diff --git a/src/components/pickers/ComboBox/ComboBox.tsx b/src/components/pickers/ComboBox/ComboBox.tsx
@@ -160,6 +160,7 @@ function ComboBox<T extends object>(props: CubeComboBoxProps<T>, ref) {
     labelSuffix,
     ...otherProps
   } = props;
+
   let isAsync = loadingState != null;
   let { contains } = useFilter({ sensitivity: 'base' });
   let [suffixWidth, setSuffixWidth] = useState(0);
@@ -294,6 +295,7 @@ function ComboBox<T extends object>(props: CubeComboBoxProps<T>, ref) {
         {suffix}
         {!hideTrigger ? (
           <TriggerElement
+            qa="ComboBoxTrigger"
             {...mergeProps(buttonProps, triggerFocusProps, triggerHoverProps)}
             {...modAttrs({
               pressed: isTriggerPressed,

diff --git a/src/components/pickers/Menu/Menu.stories.tsx b/src/components/pickers/Menu/Menu.stories.tsx
@@ -104,11 +104,11 @@ export const InsideModal = () => {
             <Button
               size="small"
               icon={<MoreOutlined />}
-              data-qa="contextMenuButton"
+              qa="ContextMenuButton"
               aria-label="Open Context Menu"
             />
             <Menu
-              data-qa="contextMenuList"
+              qa="ContextMenuList"
               id="menu"
               width="220px"
               selectionMode="multiple"
@@ -132,11 +132,11 @@ export const InsideModal = () => {
 
 InsideModal.play = async ({ canvasElement }) => {
   const canvas = within(canvasElement);
-  const button = await canvas.findByTestId('contextMenuButton');
+  const button = await canvas.findByTestId('ContextMenuButton');
 
   await userEvent.click(button);
 
-  const list = await canvas.findByTestId('contextMenuList');
+  const list = await canvas.findByTestId('ContextMenuList');
 
   await waitFor(() => expect(list).toBeInTheDocument());
 };