Skip to content

Commit

Permalink
Remove now unnecessary update position logic
Browse files Browse the repository at this point in the history 
- underlying popover component takes care of that

+ remove `euiBody-hasPortalContent` logic (if underlying EuiPopover/EuiPortal components don't need it, combobox shouldn't either)

- note: type exported from index was not being used by any consumer-facing component, so I don't think it's a breaking change to remove it

- TODO: pass resize-observer `width` from EuiInputPopover to combobox options component
  • Loading branch information
@cee-chen
cee-chen committed Oct 2, 2023
1 parent 679bdbe commit e396a68
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 121 deletions.
63 changes: 1 addition & 62 deletions src/components/combo_box/combo_box.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import React, {
} from 'react';
import classNames from 'classnames';

import { findPopoverPosition, htmlIdGenerator, keys } from '../../services';
import { htmlIdGenerator, keys } from '../../services';
import { getElementZIndex } from '../../services/popover';
import { CommonProps } from '../common';
import { EuiInputPopover } from '../popover';
Expand All @@ -41,11 +41,9 @@ import {
} from './combo_box_input/combo_box_input';
import { EuiComboBoxOptionsListProps } from './combo_box_options_list/combo_box_options_list';
import {
UpdatePositionHandler,
OptionHandler,
RefInstance,
EuiComboBoxOptionOption,
EuiComboBoxOptionsListPosition,
EuiComboBoxSingleSelectionShape,
} from './types';
import { EuiComboBoxOptionsList } from './combo_box_options_list';
Expand Down Expand Up @@ -194,7 +192,6 @@ interface EuiComboBoxState<T> {
hasFocus: boolean;
isListOpen: boolean;
listElement?: RefInstance<HTMLDivElement>;
listPosition: EuiComboBoxOptionsListPosition;
listZIndex: number | undefined;
matchingOptions: Array<EuiComboBoxOptionOption<T>>;
searchValue: string;
Expand Down Expand Up @@ -225,7 +222,6 @@ export class EuiComboBox<T> extends Component<
hasFocus: false,
isListOpen: false,
listElement: null,
listPosition: 'bottom',
listZIndex: undefined,
matchingOptions: getMatchingOptions<T>({
options: this.props.options,
Expand Down Expand Up @@ -311,59 +307,6 @@ export class EuiComboBox<T> extends Component<
});
};

updatePosition: UpdatePositionHandler = (
listElement = this.state.listElement
) => {
if (!this._isMounted) {
return;
}

if (!this.state.isListOpen) {
return;
}

if (!listElement) {
return;
}

// it's possible that updateListPosition is called when listElement is becoming visible, but isn't yet
const listElementBounds = listElement.getBoundingClientRect();
if (listElementBounds.width === 0 || listElementBounds.height === 0) {
return;
}

if (!this.comboBoxRefInstance) {
return;
}

const comboBoxBounds = this.comboBoxRefInstance.getBoundingClientRect();

const { position, top } = findPopoverPosition({
allowCrossAxis: false,
anchor: this.comboBoxRefInstance,
popover: listElement,
position: 'bottom',
}) as { position: 'bottom'; top: number };

if (this.listRefInstance) {
this.listRefInstance.style.top = `${top}px`;
// listElement doesn't have its width set until after updating the position
// which means the popover service won't know about the correct width
// however, we already know where to position the element
this.listRefInstance.style.left = `${
comboBoxBounds.left + window.pageXOffset
}px`;
this.listRefInstance.style.width = `${comboBoxBounds.width}px`;
}

// Cache for future calls.
this.setState({
listElement,
listPosition: position,
width: comboBoxBounds.width,
});
};

incrementActiveOptionIndex = (amount: number) => {
// If there are no options available, do nothing.
if (!this.state.matchingOptions.length) {
Expand Down Expand Up @@ -927,7 +870,6 @@ export class EuiComboBox<T> extends Component<
activeOptionIndex,
hasFocus,
isListOpen,
listPosition,
searchValue,
width,
matchingOptions,
Expand Down Expand Up @@ -989,15 +931,13 @@ export class EuiComboBox<T> extends Component<
onScroll={this.onOptionListScroll}
optionRef={this.optionRefCallback}
options={options}
position={listPosition}
singleSelection={singleSelection}
renderOption={renderOption}
rootId={this.rootId}
rowHeight={rowHeight}
scrollToIndex={activeOptionIndex}
searchValue={searchValue}
selectedOptions={selectedOptions}
updatePosition={this.updatePosition}
width={width}
delimiter={delimiter}
getSelectedOptionForSearchValue={getSelectedOptionForSearchValue}
Expand Down Expand Up @@ -1063,7 +1003,6 @@ export class EuiComboBox<T> extends Component<
selectedOptions={selectedOptions}
singleSelection={singleSelection}
toggleButtonRef={this.toggleButtonRefCallback}
updatePosition={this.updatePosition}
value={value}
append={singleSelection ? append : undefined}
prepend={singleSelection ? prepend : undefined}
Expand Down
23 changes: 5 additions & 18 deletions src/components/combo_box/combo_box_input/combo_box_input.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
EuiComboBoxOptionOption,
EuiComboBoxSingleSelectionShape,
OptionHandler,
UpdatePositionHandler,
} from '../types';

export interface EuiComboBoxInputProps<T> extends CommonProps {
Expand All @@ -56,7 +55,6 @@ export interface EuiComboBoxInputProps<T> extends CommonProps {
selectedOptions: Array<EuiComboBoxOptionOption<T>>;
singleSelection?: boolean | EuiComboBoxSingleSelectionShape;
toggleButtonRef?: RefCallback<HTMLButtonElement | HTMLSpanElement>;
updatePosition: UpdatePositionHandler;
value?: string;
prepend?: EuiFormControlLayoutProps['prepend'];
append?: EuiFormControlLayoutProps['append'];
Expand Down Expand Up @@ -99,12 +97,11 @@ export class EuiComboBoxInput<T> extends Component<
this.setState({ inputWidth });
};

updatePosition = () => {
// Wait a beat for the DOM to update, since we depend on DOM elements' bounds.
requestAnimationFrame(() => {
this.props.updatePosition();
});
};
componentDidUpdate(prevProps: EuiComboBoxInputProps<T>) {
if (prevProps.searchValue !== this.props.searchValue) {
this.updateInputSize(this.props.searchValue);
}
}

onFocus: FocusEventHandler<HTMLInputElement> = (event) => {
this.props.onFocus(event);
Expand Down Expand Up @@ -145,16 +142,6 @@ export class EuiComboBoxInput<T> extends Component<
}
};

componentDidUpdate(prevProps: EuiComboBoxInputProps<T>) {
if (prevProps.searchValue !== this.props.searchValue) {
this.updateInputSize(this.props.searchValue);

// We need to update the position of everything if the user enters enough input to change
// the size of the input.
this.updatePosition();
}
}

render() {
const {
compressed,
Expand Down
36 changes: 1 addition & 35 deletions src/components/combo_box/combo_box_options_list/combo_box_options_list.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,9 @@ import { EuiTextTruncate } from '../../text_truncate';
import type { _EuiComboBoxProps } from '../combo_box';
import {
EuiComboBoxOptionOption,
EuiComboBoxOptionsListPosition,
EuiComboBoxSingleSelectionShape,
OptionHandler,
RefInstance,
UpdatePositionHandler,
} from '../types';

export type EuiComboBoxOptionsListProps<T> = CommonProps & {
Expand Down Expand Up @@ -76,7 +74,6 @@ export type EuiComboBoxOptionsListProps<T> = CommonProps & {
* Array of EuiComboBoxOptionOption objects. See #EuiComboBoxOptionOption
*/
options: Array<EuiComboBoxOptionOption<T>>;
position?: EuiComboBoxOptionsListPosition;
renderOption?: (
option: EuiComboBoxOptionOption<T>,
searchValue: string,
Expand All @@ -87,7 +84,6 @@ export type EuiComboBoxOptionsListProps<T> = CommonProps & {
scrollToIndex?: number;
searchValue: string;
selectedOptions: Array<EuiComboBoxOptionOption<T>>;
updatePosition: UpdatePositionHandler;
width: number;
singleSelection?: boolean | EuiComboBoxSingleSelectionShape;
delimiter?: string;
Expand Down Expand Up @@ -117,22 +113,7 @@ export class EuiComboBoxOptionsList<T> extends Component<
isCaseSensitive: false,
};

updatePosition = () => {
// Wait a beat for the DOM to update, since we depend on DOM elements' bounds.
requestAnimationFrame(() => {
this.props.updatePosition(this.listRefInstance);
});
};

componentDidMount() {
// Wait a frame, otherwise moving focus from one combo box to another will result in the class
// being removed from the body.
requestAnimationFrame(() => {
document.body.classList.add('euiBody-hasPortalContent');
});
this.updatePosition();
window.addEventListener('resize', this.updatePosition);

// Firefox will trigger a scroll event in many common situations when the options list div is appended
// to the DOM; in testing it was always within 100ms, but setting a timeout here for 500ms to be safe
setTimeout(() => {
Expand All @@ -144,17 +125,6 @@ export class EuiComboBoxOptionsList<T> extends Component<
}

componentDidUpdate(prevProps: EuiComboBoxOptionsListProps<T>) {
const { options, selectedOptions, searchValue } = prevProps;

// We don't compare matchingOptions because that will result in a loop.
if (
searchValue !== this.props.searchValue ||
options !== this.props.options ||
selectedOptions !== this.props.selectedOptions
) {
this.updatePosition();
}

if (
this.listRef &&
typeof this.props.activeOptionIndex !== 'undefined' &&
Expand All @@ -165,8 +135,6 @@ export class EuiComboBoxOptionsList<T> extends Component<
}

componentWillUnmount() {
document.body.classList.remove('euiBody-hasPortalContent');
window.removeEventListener('resize', this.updatePosition);
window.removeEventListener('scroll', this.closeListOnScroll, {
capture: true,
});
Expand Down Expand Up @@ -378,15 +346,13 @@ export class EuiComboBoxOptionsList<T> extends Component<
onScroll,
optionRef,
options,
position = 'bottom',
renderOption,
rootId,
rowHeight,
scrollToIndex,
searchValue,
selectedOptions,
singleSelection,
updatePosition,
width,
delimiter,
zIndex,
Expand Down Expand Up @@ -539,7 +505,7 @@ export class EuiComboBoxOptionsList<T> extends Component<
itemData={matchingOptions}
ref={this.setListRef}
innerRef={this.setListBoxRef}
width={width}
width={width} // TODO: Inherit width from EuiInputPopover instead of EuiComboBox
>
{this.ListRow}
</FixedSizeList>
Expand Down
1 change: 0 additions & 1 deletion src/components/combo_box/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,5 @@ export * from './combo_box_input';
export * from './combo_box_options_list';
export type {
EuiComboBoxOptionOption,
EuiComboBoxOptionsListPosition,
EuiComboBoxSingleSelectionShape,
} from './types';
5 changes: 0 additions & 5 deletions src/components/combo_box/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,10 @@ export interface EuiComboBoxOptionOption<
truncationProps?: _EuiComboBoxProps<T>['truncationProps'];
}

export type UpdatePositionHandler = (
listElement?: RefInstance<HTMLDivElement>
) => void;
export type OptionHandler<T> = (option: EuiComboBoxOptionOption<T>) => void;

export type RefInstance<T> = T | null;

export type EuiComboBoxOptionsListPosition = 'top' | 'bottom';

export interface EuiComboBoxSingleSelectionShape {
asPlainText?: boolean;
}

0 comments on commit e396a68

Please sign in to comment.