Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/orange-ties-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@cube-dev/ui-kit": patch
---

ComboBox now respects `onSelectionChange` event while working inside a form.
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = [
}),
);
},
limit: '250kB',
limit: '200kB',
},
{
name: 'Tree shaking (just a Button)',
Expand Down
29 changes: 21 additions & 8 deletions src/components/forms/Form/Field.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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 {
Expand All @@ -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[];
Expand Down Expand Up @@ -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,
}),
Expand All @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/components/forms/Form/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
31 changes: 24 additions & 7 deletions src/components/forms/Form/useForm.tsx
Original file line number Diff line number Diff line change
@@ -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]>;
Expand All @@ -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;
Expand Down Expand Up @@ -64,6 +64,7 @@ export class CubeFormInstance<
touched?: boolean,
skipRender?: boolean,
createFields = false,
inputOnly = false,
) => {
let flag = false;

Expand All @@ -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;
Expand All @@ -98,7 +103,7 @@ export class CubeFormInstance<
if (flag && !skipRender) {
this.forceReRender();

if (touched) {
if (touched && !inputOnly) {
this.onValuesChange && this.onValuesChange(this.getFormData());
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -157,7 +167,7 @@ export class CubeFormInstance<
this.forceReRender();
}

if (touched) {
if (touched && !inputOnly) {
this.onValuesChange && this.onValuesChange(this.getFormData());
}
}
Expand Down Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export const FloatingNotification = memo(function FloatingNotification(
}}
onDismiss={chainedOnDismiss}
onClose={onCloseEvent}
qa="floating-notification"
qa="FloatingNotification"
/>
</NotificationContainer>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand Down Expand Up @@ -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();
};
Original file line number Diff line number Diff line change
Expand Up @@ -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} />,
Expand All @@ -27,7 +27,7 @@ describe('<Notification />', () => {
<TestComponent disableRemoveOnUnmount />,
);

const notification = getByTestId('floating-notification');
const notification = getByTestId('FloatingNotification');

rerender(
<TestComponent disableRemoveOnUnmount renderNotification={false} />,
Expand Down
2 changes: 1 addition & 1 deletion src/components/overlays/Toasts/Toasts.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
Expand Down
10 changes: 5 additions & 5 deletions src/components/overlays/Toasts/toast.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('useToastsApi', () => {
});

expect(
screen.queryByTestId('floating-notification'),
screen.queryByTestId('FloatingNotification'),
).not.toBeInTheDocument();

expect(dismiss).toBeCalledTimes(1);
Expand All @@ -37,7 +37,7 @@ describe('useToastsApi', () => {
jest.runAllTimers();
});

expect(screen.getByTestId('floating-notification')).toBeInTheDocument();
expect(screen.getByTestId('FloatingNotification')).toBeInTheDocument();
expect(dismiss).not.toBeCalled();
});

Expand All @@ -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} />,
Expand All @@ -76,7 +76,7 @@ describe('useToastsApi', () => {
<TestComponent disableRemoveOnUnmount />,
);

const notification = getByTestId('floating-notification');
const notification = getByTestId('FloatingNotification');

rerender(
<TestComponent disableRemoveOnUnmount renderNotification={false} />,
Expand Down
2 changes: 2 additions & 0 deletions src/components/pickers/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions src/components/pickers/Menu/Menu.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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());
};
Expand Down
Loading