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

[Controls] Use new panelMinWidth prop in popovers #165397

Merged
Merged
9 changes: 9 additions & 0 deletions src/plugins/controls/public/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const MIN_POPOVER_WIDTH = 300;
Heenawter marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,6 @@ $controlMinWidth: $euiSize * 14;
&--small {
width: $smallControl;
min-width: $smallControl;

&:not(.controlFrameWrapper--grow) {
.controlFrame__labelToolTip {
max-width: 20%;
}
}
Comment on lines -127 to -131
Copy link
Contributor Author

@Heenawter Heenawter Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that the popover can extend past the input for range sliders, I don't think this specific styling is necessary anymore.

Before After
The control title takes up less space on the small size:

image
The control title takes up the same 40% even in the small size:

image

}

&--medium {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
.optionsList--filterGroup {
width: 100%;
height: $euiSizeXXL;
box-shadow: none;
background-color: transparent;

box-shadow: none;
border-top-left-radius: 0;
border-bottom-left-radius: 0;
border-top-right-radius: $euiBorderRadius - 1px;
border-bottom-right-radius: $euiBorderRadius - 1px;
.optionsList__inputButtonOverride {
max-inline-size: none; // overwrite the default `max-inline-size` that's coming from EUI
}

.optionsList--filterBtn {
border-radius: 0 !important;
height: $euiButtonHeight;

&.optionsList--filterBtnPlaceholder {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
import { Subject } from 'rxjs';
import classNames from 'classnames';
import { debounce, isEmpty } from 'lodash';
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';

import { EuiFilterButton, EuiFilterGroup, EuiPopover, useResizeObserver } from '@elastic/eui';
import { EuiFilterButton, EuiFilterGroup, EuiInputPopover } from '@elastic/eui';

import { MAX_OPTIONS_LIST_REQUEST_SIZE } from '../types';
import { OptionsListStrings } from './options_list_strings';
Expand All @@ -20,6 +20,7 @@ import { useOptionsList } from '../embeddable/options_list_embeddable';

import './options_list.scss';
import { ControlError } from '../../control_group/component/control_error_component';
import { MIN_POPOVER_WIDTH } from '../../constants';

export const OptionsListControl = ({
typeaheadSubject,
Expand All @@ -28,9 +29,7 @@ export const OptionsListControl = ({
typeaheadSubject: Subject<string>;
loadMoreSubject: Subject<number>;
}) => {
const resizeRef = useRef(null);
const optionsList = useOptionsList();
const dimensions = useResizeObserver(resizeRef.current);

const error = optionsList.select((state) => state.componentState.error);
const isPopoverOpen = optionsList.select((state) => state.componentState.popoverOpen);
Expand Down Expand Up @@ -124,26 +123,24 @@ export const OptionsListControl = ({
}, [exclude, existsSelected, validSelections, invalidSelections]);

const button = (
<div className="optionsList--filterBtnWrapper" ref={resizeRef}>
<EuiFilterButton
badgeColor="success"
iconType="arrowDown"
isLoading={debouncedLoading}
className={classNames('optionsList--filterBtn', {
'optionsList--filterBtnSingle': controlStyle !== 'twoLine',
'optionsList--filterBtnPlaceholder': !hasSelections,
})}
data-test-subj={`optionsList-control-${id}`}
onClick={() => optionsList.dispatch.setPopoverOpen(!isPopoverOpen)}
isSelected={isPopoverOpen}
numActiveFilters={validSelectionsCount}
hasActiveFilters={Boolean(validSelectionsCount)}
>
{hasSelections || existsSelected
? selectionDisplayNode
: placeholder ?? OptionsListStrings.control.getPlaceholder()}
</EuiFilterButton>
</div>
<EuiFilterButton
badgeColor="success"
iconType="arrowDown"
isLoading={debouncedLoading}
className={classNames('optionsList--filterBtn', {
'optionsList--filterBtnSingle': controlStyle !== 'twoLine',
'optionsList--filterBtnPlaceholder': !hasSelections,
})}
data-test-subj={`optionsList-control-${id}`}
onClick={() => optionsList.dispatch.setPopoverOpen(!isPopoverOpen)}
isSelected={isPopoverOpen}
numActiveFilters={validSelectionsCount}
hasActiveFilters={Boolean(validSelectionsCount)}
>
{hasSelections || existsSelected
? selectionDisplayNode
: placeholder ?? OptionsListStrings.control.getPlaceholder()}
</EuiFilterButton>
);

return error ? (
Expand All @@ -154,26 +151,26 @@ export const OptionsListControl = ({
'optionsList--filterGroupSingle': controlStyle !== 'twoLine',
})}
>
<EuiPopover
<EuiInputPopover
ownFocus
button={button}
input={button}
hasArrow={false}
repositionOnScroll
isOpen={isPopoverOpen}
panelPaddingSize="none"
anchorPosition="downCenter"
panelMinWidth={MIN_POPOVER_WIDTH}
className="optionsList__inputButtonOverride"
initialFocus={'[data-test-subj=optionsList-control-search-input]'}
closePopover={() => optionsList.dispatch.setPopoverOpen(false)}
aria-label={OptionsListStrings.popover.getAriaLabel(fieldName)}
panelClassName="optionsList__popoverOverride"
panelProps={{ 'aria-label': OptionsListStrings.popover.getAriaLabel(fieldName) }}
>
<OptionsListPopover
width={dimensions.width}
isLoading={debouncedLoading}
updateSearchString={updateSearchString}
loadMoreSuggestions={loadMoreSuggestions}
/>
</EuiPopover>
</EuiInputPopover>
</EuiFilterGroup>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,15 @@ import { mountWithIntl } from '@kbn/test-jest-helpers';
import { findTestSubject } from '@elastic/eui/lib/test';
import { FieldSpec } from '@kbn/data-views-plugin/common';

import { pluginServices } from '../../services';
import { mockOptionsListEmbeddable } from '../../../common/mocks';
import { ControlOutput, OptionsListEmbeddableInput } from '../..';
import { OptionsListComponentState, OptionsListReduxState } from '../types';
import { OptionsListEmbeddableContext } from '../embeddable/options_list_embeddable';
import { OptionsListPopover, OptionsListPopoverProps } from './options_list_popover';
import { pluginServices } from '../../services';

describe('Options list popover', () => {
const defaultProps = {
width: 500,
isLoading: false,
updateSearchString: jest.fn(),
loadMoreSuggestions: jest.fn(),
Expand Down Expand Up @@ -58,17 +57,6 @@ describe('Options list popover', () => {
showOnlySelectedButton.simulate('click');
};

test('available options list width responds to container size', async () => {
Copy link
Contributor Author

@Heenawter Heenawter Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The EUI panelMinWidth applies to the portal that is created for the popover, and not to the popover itself; this portal does not exist from within the popover component, so this test is no longer relevant.

let popover = await mountComponent({ popoverProps: { width: 301 } });
let popoverDiv = findTestSubject(popover, 'optionsList-control-popover');
expect(popoverDiv.getDOMNode().getAttribute('style')).toBe('width: 301px; min-width: 300px;');

// the div cannot be smaller than 301 pixels wide
popover = await mountComponent({ popoverProps: { width: 300 } });
popoverDiv = findTestSubject(popover, 'optionsList-control-available-options');
expect(popoverDiv.getDOMNode().getAttribute('style')).toBe('width: 100%; height: 100%;');
});

test('no available options', async () => {
const popover = await mountComponent({ componentState: { availableOptions: [] } });
const availableOptionsDiv = findTestSubject(popover, 'optionsList-control-available-options');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,19 @@
import { isEmpty } from 'lodash';
import React, { useState } from 'react';

import { OptionsListStrings } from './options_list_strings';
import { useOptionsList } from '../embeddable/options_list_embeddable';
import { OptionsListPopoverFooter } from './options_list_popover_footer';
import { OptionsListPopoverActionBar } from './options_list_popover_action_bar';
import { OptionsListPopoverSuggestions } from './options_list_popover_suggestions';
import { OptionsListPopoverInvalidSelections } from './options_list_popover_invalid_selections';

export interface OptionsListPopoverProps {
width: number;
isLoading: boolean;
loadMoreSuggestions: (cardinality: number) => void;
updateSearchString: (newSearchString: string) => void;
}

export const OptionsListPopover = ({
width,
isLoading,
updateSearchString,
loadMoreSuggestions,
Expand All @@ -36,43 +33,38 @@ export const OptionsListPopover = ({
const invalidSelections = optionsList.select((state) => state.componentState.invalidSelections);

const id = optionsList.select((state) => state.explicitInput.id);
const fieldName = optionsList.select((state) => state.explicitInput.fieldName);
const hideExclude = optionsList.select((state) => state.explicitInput.hideExclude);
const hideActionBar = optionsList.select((state) => state.explicitInput.hideActionBar);

const [showOnlySelected, setShowOnlySelected] = useState(false);

return (
<>
<div
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just removed the wrapping fragment, since it's unnecessary - no other changes were made here.

id={`control-popover-${id}`}
className={'optionsList__popover'}
data-test-subj={`optionsList-control-popover`}
>
{field?.type !== 'boolean' && !hideActionBar && (
<OptionsListPopoverActionBar
showOnlySelected={showOnlySelected}
updateSearchString={updateSearchString}
setShowOnlySelected={setShowOnlySelected}
/>
)}
<div
id={`control-popover-${id}`}
className={'optionsList__popover'}
style={{ width, minWidth: 300 }}
data-test-subj={`optionsList-control-popover`}
aria-label={OptionsListStrings.popover.getAriaLabel(fieldName)}
data-test-subj={`optionsList-control-available-options`}
data-option-count={isLoading ? 0 : Object.keys(availableOptions ?? {}).length}
style={{ width: '100%', height: '100%' }}
>
{field?.type !== 'boolean' && !hideActionBar && (
<OptionsListPopoverActionBar
showOnlySelected={showOnlySelected}
updateSearchString={updateSearchString}
setShowOnlySelected={setShowOnlySelected}
/>
<OptionsListPopoverSuggestions
loadMoreSuggestions={loadMoreSuggestions}
showOnlySelected={showOnlySelected}
/>
{!showOnlySelected && invalidSelections && !isEmpty(invalidSelections) && (
<OptionsListPopoverInvalidSelections />
)}
<div
data-test-subj={`optionsList-control-available-options`}
data-option-count={isLoading ? 0 : Object.keys(availableOptions ?? {}).length}
style={{ width: '100%', height: '100%' }}
>
<OptionsListPopoverSuggestions
loadMoreSuggestions={loadMoreSuggestions}
showOnlySelected={showOnlySelected}
/>
{!showOnlySelected && invalidSelections && !isEmpty(invalidSelections) && (
<OptionsListPopoverInvalidSelections />
)}
</div>
{!hideExclude && <OptionsListPopoverFooter isLoading={isLoading} />}
</div>
</>
{!hideExclude && <OptionsListPopoverFooter isLoading={isLoading} />}
</div>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useRangeSlider } from '../embeddable/range_slider_embeddable';
import { ControlError } from '../../control_group/component/control_error_component';

import './range_slider.scss';
import { MIN_POPOVER_WIDTH } from '../../constants';

export const RangeSliderControl: FC = () => {
/** Controls Services Context */
Expand Down Expand Up @@ -153,6 +154,9 @@ export const RangeSliderControl: FC = () => {
min={displayedMin}
max={displayedMax}
isLoading={isLoading}
inputPopoverProps={{
panelMinWidth: MIN_POPOVER_WIDTH,
}}
onMouseUp={() => {
// when the pin is dropped (on mouse up), cancel any pending debounced changes and force the change
// in value to happen instantly (which, in turn, will re-calculate the min/max for the slider due to
Expand Down
Loading