Skip to content
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

fix(ComboBox): fix mobile overlay not closing when using public methods #3268

Merged
merged 11 commits into from
Oct 5, 2023
69 changes: 69 additions & 0 deletions packages/react-ui/components/ComboBox/__tests__/ComboBox-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import React, { useState } from 'react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { MobilePopupDataTids } from '../../../internal/MobilePopup';
import { DEFAULT_THEME } from '../../../lib/theming/themes/DefaultTheme';
import { HTMLProps } from '../../../typings/html';
import { InputDataTids } from '../../../components/Input';
import { MenuMessageDataTids } from '../../../internal/MenuMessage';
Expand Down Expand Up @@ -1303,3 +1305,70 @@ describe('ComboBox', () => {
expect(screen.getByTestId(InputLikeTextDataTids.root)).not.toHaveFocus();
});
});

describe('mobile comboBox', () => {
const calcMatches = (query: string) => query === DEFAULT_THEME.mobileMediaQuery;
const oldMatchMedia = window.matchMedia;
const matchMediaMock = jest.fn().mockImplementation((query) => {
return {
matches: calcMatches(query),
media: query,
onchange: null,
addListener: jest.fn(),
removeListener: jest.fn(),
addEventListener: jest.fn(),
removeEventListener: jest.fn(),
};
});

const comboBoxRef = React.createRef<ComboBox<{ value: number; label: string }>>();
const state = { value: 1, label: 'First' };

const items = [
{ value: 1, label: 'First' },
{ value: 2, label: 'Second' },
{ value: 3, label: 'Third' },
];

const getItems = (query: string) =>
Promise.resolve(
items.filter((x) => x.label.toLowerCase().includes(query.toLowerCase()) || x.value.toString(10) === query),
);

const close = () => comboBoxRef.current?.close();
const menuItemsCount = items.length;

beforeEach(() => {
window.matchMedia = matchMediaMock;
});

afterEach(() => {
window.matchMedia = oldMatchMedia;
});

const TestComponent = () => <ComboBox ref={comboBoxRef} getItems={getItems} value={state} />;

it('should fully close by method', async () => {
render(<TestComponent />);
userEvent.click(screen.getByTestId(InputLikeTextDataTids.root));
await delay(500);

close();
expect(screen.queryByTestId(MobilePopupDataTids.root)).not.toBeInTheDocument();
});

it('should close and open again after being closed by public method', async () => {
render(<TestComponent />);

userEvent.click(screen.getByTestId(InputLikeTextDataTids.root));
await delay(500);

close();

userEvent.click(screen.getByTestId(InputDataTids.root));
await delay(500);

expect(screen.getByTestId(MobilePopupDataTids.root)).toBeInTheDocument();
expect(await screen.findAllByRole('button')).toHaveLength(menuItemsCount);
});
});
63 changes: 22 additions & 41 deletions packages/react-ui/internal/CustomComboBox/ComboBoxView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ interface ComboBoxViewProps<T>
onValueChange?: (value: T) => void;
onClickOutside?: (e: Event) => void;
onFocus?: () => void;
onMobileClose?: () => void;
onFocusOutside?: () => void;
onInputBlur?: () => void;
onInputValueChange?: (value: string) => void;
Expand All @@ -88,10 +89,6 @@ interface ComboBoxViewProps<T>
refInputLikeText?: (inputLikeText: Nullable<InputLikeText>) => void;
}

interface ComboBoxViewState {
isMobileOpened: boolean;
}

type DefaultProps<T> = Required<
Pick<
ComboBoxViewProps<T>,
Expand All @@ -113,7 +110,7 @@ export const ComboBoxViewIds = {

@responsiveLayout
@rootNode
export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, ComboBoxViewState> {
export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>> {
public static __KONTUR_REACT_UI__ = 'ComboBoxView';

public static defaultProps: DefaultProps<unknown> = {
Expand Down Expand Up @@ -149,10 +146,6 @@ export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, Combo
this.props.opened && this.dropdownContainerRef.current?.position();
}

public state: ComboBoxViewState = {
isMobileOpened: false,
};

public componentDidUpdate(prevProps: ComboBoxViewProps<T>) {
const { input, props } = this;

Expand Down Expand Up @@ -247,42 +240,34 @@ export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, Combo
private renderMobileMenu = () => {
let rightIcon = null;

const { loading, items } = this.props;
const { loading, items, opened, onFocus, onInputValueChange, placeholder, textValue } = this.props;
if (loading && items && !!items.length) {
rightIcon = this.renderSpinner();
}

const inputProps: InputProps = {
autoFocus: true,
width: '100%',
onFocus: this.props.onFocus,
onValueChange: this.props.onInputValueChange,
value: this.props.textValue,
placeholder: this.props.placeholder,
onFocus,
onValueChange: onInputValueChange,
value: textValue,
placeholder,
rightIcon,
};

return (
<MobilePopup
headerChildComponent={<Input ref={this.refMobileInput} {...inputProps} />}
onCloseRequest={this.handleCloseMobile}
opened={this.state.isMobileOpened}
>
{this.getComboBoxMenu()}
</MobilePopup>
opened && (
<MobilePopup
headerChildComponent={<Input ref={this.refMobileInput} {...inputProps} />}
onCloseRequest={this.props.onMobileClose}
opened
>
{this.getComboBoxMenu()}
</MobilePopup>
)
);
};

private handleCloseMobile = () => {
this.setState({
isMobileOpened: false,
});

if (this.props.onInputBlur) {
this.props.onInputBlur();
}
zhzz marked this conversation as resolved.
Show resolved Hide resolved
};

private getParent = () => {
return getRootNode(this);
};
Expand Down Expand Up @@ -331,8 +316,8 @@ export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, Combo
maxLength={this.props.maxLength}
onBlur={isMobile ? undefined : onInputBlur}
onValueChange={onInputValueChange}
onFocus={isMobile ? this.handleFocusMobile : onInputFocus}
onClick={isMobile ? this.handleFocusMobile : onInputClick}
onFocus={onInputFocus}
onClick={isMobile ? this.handleMobileFocus : onInputClick}
leftIcon={leftIcon}
rightIcon={rightIcon}
value={textValue || ''}
Expand Down Expand Up @@ -372,14 +357,10 @@ export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, Combo
);
}

private handleFocusMobile = () => {
this.setState({
isMobileOpened: true,
});
private handleMobileFocus = () => {
this.props.onInputClick?.();

if (this.mobileInput) {
this.mobileInput.focus();
}
this.mobileInput?.focus();
};

private handleItemSelect = (item: T) => {
Expand All @@ -388,7 +369,7 @@ export class ComboBoxView<T> extends React.Component<ComboBoxViewProps<T>, Combo
}

if (this.isMobileLayout) {
this.handleCloseMobile();
this.props.onMobileClose?.();
}
};

Expand Down
9 changes: 6 additions & 3 deletions packages/react-ui/internal/CustomComboBox/CustomComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ export class CustomComboBox<T> extends React.PureComponent<CustomComboBoxProps<T
onValueChange: this.handleValueChange,
onClickOutside: this.handleClickOutside,
onFocus: this.handleFocus,
onMobileClose: this.handleMobileClose,
onFocusOutside: this.handleBlur,
onInputBlur: this.handleInputBlur,
onInputValueChange: (value: string) => this.dispatch({ type: 'TextChange', value }),
Expand Down Expand Up @@ -383,6 +384,10 @@ export class CustomComboBox<T> extends React.PureComponent<CustomComboBoxProps<T
this.dispatch({ type: 'Focus' });
};

private handleMobileClose = () => {
this.handleInputBlur();
};

private handleClickOutside = (e: Event) => {
fixClickFocusIE(e);
this.handleBlur();
Expand Down Expand Up @@ -410,9 +415,7 @@ export class CustomComboBox<T> extends React.PureComponent<CustomComboBoxProps<T
// it would call handleFocusOutside
// In that way handleBlur would be called

// TODO: add check for mobile layout, to call `handleBlur`

if (this.state.opened) {
if (this.state.opened && !this.isMobileLayout) {
return;
}
this.handleBlur();
Expand Down