-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Dashboard] [Controls] Add "Exists" functionality to options list #143762
Merged
Heenawter
merged 33 commits into
elastic:main
from
Heenawter:add-exists-to-options-list_2022-10-19
Nov 4, 2022
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
7a260b3
Mock first attempt at UI
Heenawter 98c7112
Add `exists` filter functionality
Heenawter efa5462
Make `exists` selection change button
Heenawter f5dcc03
Overwrite selections instead of disabling them
Heenawter 405434d
Add support for negate to exists
Heenawter 70a0e6f
Add to diffing system
Heenawter 721670d
Add toggle to disable `exists` query
Heenawter e11b760
Clear `exists` selection when toggle is disabled + fix mocks
Heenawter 92dab10
Switch to tooltip instead of docs link
Heenawter 865b066
Clean up popover logic
Heenawter a313b45
Fix rendering through memoization
Heenawter 166c369
Auto focus on search when popover opens
Heenawter ed77465
Added Jest unit tests
Heenawter 4eefecb
Beef up mock and add more Jest unit tests
Heenawter 72d595c
Add functional tests
Heenawter 4b875e3
Split up popover in to smaller components
Heenawter afc0bcf
Fix unit tests + functional test flakiness
Heenawter 52961b4
Fix flakiness a second time + add chaining tests
Heenawter 899a7f9
Clean up code
Heenawter 52ad6cf
Add `exists` selection to validation
Heenawter dcd5ca8
Fix invalid bug
Heenawter a45b451
Fix failing unit test
Heenawter 30c2a2a
More code clean up
Heenawter fd89c1e
Add another functional test
Heenawter 8af6c57
Apply styling changes
Heenawter 5927369
Fix tests
Heenawter b1bbfc8
Fix a11y issues
Heenawter 20a1dd2
Remove validation
Heenawter 1f3bd1b
Fix types
Heenawter 0318b0a
Clean up `a11y` fix
Heenawter 64c11c9
Fix jest test
Heenawter 7702df3
Address feedback
Heenawter a87f33d
Fix wording of tooltip
Heenawter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
60 changes: 60 additions & 0 deletions
60
src/plugins/controls/public/options_list/components/options_list_control.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
/* | ||
* 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. | ||
*/ | ||
|
||
import React from 'react'; | ||
|
||
import { mountWithIntl } from '@kbn/test-jest-helpers'; | ||
import { findTestSubject } from '@elastic/eui/lib/test'; | ||
|
||
import { OptionsListComponentState, OptionsListReduxState } from '../types'; | ||
import { ControlOutput, OptionsListEmbeddableInput } from '../..'; | ||
import { mockOptionsListReduxEmbeddableTools } from '../../../common/mocks'; | ||
import { OptionsListControl } from './options_list_control'; | ||
import { BehaviorSubject } from 'rxjs'; | ||
|
||
describe('Options list control', () => { | ||
ThomThomson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const defaultProps = { | ||
typeaheadSubject: new BehaviorSubject(''), | ||
}; | ||
|
||
interface MountOptions { | ||
componentState: Partial<OptionsListComponentState>; | ||
explicitInput: Partial<OptionsListEmbeddableInput>; | ||
output: Partial<ControlOutput>; | ||
} | ||
|
||
async function mountComponent(options?: Partial<MountOptions>) { | ||
const mockReduxEmbeddableTools = await mockOptionsListReduxEmbeddableTools({ | ||
componentState: options?.componentState ?? {}, | ||
explicitInput: options?.explicitInput ?? {}, | ||
output: options?.output ?? {}, | ||
} as Partial<OptionsListReduxState>); | ||
|
||
return mountWithIntl( | ||
<mockReduxEmbeddableTools.Wrapper> | ||
<OptionsListControl {...defaultProps} /> | ||
</mockReduxEmbeddableTools.Wrapper> | ||
); | ||
} | ||
|
||
test('if exclude = false and existsSelected = true, then the option should read "Exists"', async () => { | ||
const control = await mountComponent({ | ||
explicitInput: { id: 'testExists', exclude: false, existsSelected: true }, | ||
}); | ||
const existsOption = findTestSubject(control, 'optionsList-control-testExists'); | ||
expect(existsOption.text()).toBe('Exists'); | ||
}); | ||
|
||
test('if exclude = true and existsSelected = true, then the option should read "Does not exist"', async () => { | ||
const control = await mountComponent({ | ||
explicitInput: { id: 'testDoesNotExist', exclude: true, existsSelected: true }, | ||
}); | ||
const existsOption = findTestSubject(control, 'optionsList-control-testDoesNotExist'); | ||
expect(existsOption.text()).toBe('DOES NOT Exist'); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,7 @@ import classNames from 'classnames'; | |
import { debounce, isEmpty } from 'lodash'; | ||
import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react'; | ||
|
||
import { | ||
EuiFilterButton, | ||
EuiFilterGroup, | ||
EuiPopover, | ||
EuiTextColor, | ||
useResizeObserver, | ||
} from '@elastic/eui'; | ||
import { EuiFilterButton, EuiFilterGroup, EuiPopover, useResizeObserver } from '@elastic/eui'; | ||
import { useReduxEmbeddableContext } from '@kbn/presentation-util-plugin/public'; | ||
|
||
import { OptionsListStrings } from './options_list_strings'; | ||
|
@@ -46,10 +40,11 @@ export const OptionsListControl = ({ typeaheadSubject }: { typeaheadSubject: Sub | |
const validSelections = select((state) => state.componentState.validSelections); | ||
|
||
const selectedOptions = select((state) => state.explicitInput.selectedOptions); | ||
const existsSelected = select((state) => state.explicitInput.existsSelected); | ||
const controlStyle = select((state) => state.explicitInput.controlStyle); | ||
const singleSelect = select((state) => state.explicitInput.singleSelect); | ||
const id = select((state) => state.explicitInput.id); | ||
const exclude = select((state) => state.explicitInput.exclude); | ||
const id = select((state) => state.explicitInput.id); | ||
|
||
const loading = select((state) => state.output.loading); | ||
|
||
|
@@ -83,22 +78,34 @@ export const OptionsListControl = ({ typeaheadSubject }: { typeaheadSubject: Sub | |
selectionDisplayNode: ( | ||
<> | ||
{exclude && ( | ||
<EuiTextColor color="danger"> | ||
<b>{OptionsListStrings.control.getNegate()}</b>{' '} | ||
</EuiTextColor> | ||
)} | ||
{validSelections && ( | ||
<span>{validSelections?.join(OptionsListStrings.control.getSeparator())}</span> | ||
<> | ||
<span className="optionsList__negateLabel"> | ||
{existsSelected | ||
? OptionsListStrings.control.getExcludeExists() | ||
: OptionsListStrings.control.getNegate()} | ||
</span>{' '} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried to add this space using CSS |
||
</> | ||
)} | ||
{invalidSelections && ( | ||
<span className="optionsList__filterInvalid"> | ||
{invalidSelections.join(OptionsListStrings.control.getSeparator())} | ||
{existsSelected ? ( | ||
<span className={`optionsList__existsFilter`}> | ||
{OptionsListStrings.controlAndPopover.getExists(+Boolean(exclude))} | ||
</span> | ||
) : ( | ||
<> | ||
{validSelections && ( | ||
<span>{validSelections?.join(OptionsListStrings.control.getSeparator())}</span> | ||
)} | ||
{invalidSelections && ( | ||
<span className="optionsList__filterInvalid"> | ||
{invalidSelections.join(OptionsListStrings.control.getSeparator())} | ||
</span> | ||
)} | ||
</> | ||
)} | ||
</> | ||
), | ||
}; | ||
}, [exclude, validSelections, invalidSelections]); | ||
}, [exclude, existsSelected, validSelections, invalidSelections]); | ||
|
||
const button = ( | ||
<div className="optionsList--filterBtnWrapper" ref={resizeRef}> | ||
|
@@ -115,7 +122,9 @@ export const OptionsListControl = ({ typeaheadSubject }: { typeaheadSubject: Sub | |
numActiveFilters={validSelectionsCount} | ||
hasActiveFilters={Boolean(validSelectionsCount)} | ||
> | ||
{hasSelections ? selectionDisplayNode : OptionsListStrings.control.getPlaceholder()} | ||
{hasSelections || existsSelected | ||
? selectionDisplayNode | ||
: OptionsListStrings.control.getPlaceholder()} | ||
</EuiFilterButton> | ||
</div> | ||
); | ||
|
@@ -136,6 +145,7 @@ export const OptionsListControl = ({ typeaheadSubject }: { typeaheadSubject: Sub | |
className="optionsList__popoverOverride" | ||
closePopover={() => setIsPopoverOpen(false)} | ||
anchorClassName="optionsList__anchorOverride" | ||
aria-labelledby={`control-popover-${id}`} | ||
> | ||
<OptionsListPopover width={dimensions.width} updateSearchString={updateSearchString} /> | ||
</EuiPopover> | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that, rather than storing
existsSelected
as a separate property, I could have included this status in theselectedOptions
array. However, after further investigation, this added extra complexity to the code for, in my opinion, minimal benefit.While this change would clean up the
OptionsListEmbeddableInput
a bit, treating theexists
query the same as any other selection means that every selection/deselection action, including the validation, has two cases: one,exists
is selected/deselected or two, something other thanexists
is selected/deselected. This resulted in manyselectedOptions.contains(...)
checks in various selection related places.Therefore, because the logic for handling an
exists
selection is so different than any other selection, I chose to just keep it separate to avoid the expensivecontains
checks.