-
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
Improved Visualize button in field popover #103099
Changes from 3 commits
279b72d
7223bc5
52766d6
fd9ed8e
19366b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,18 +25,19 @@ const indexPattern = getStubIndexPattern( | |
); | ||
|
||
describe('discover sidebar field details', function () { | ||
const onAddFilter = jest.fn(); | ||
const defaultProps = { | ||
indexPattern, | ||
details: { buckets: [], error: '', exists: 1, total: 2, columns: [] }, | ||
onAddFilter: jest.fn(), | ||
onAddFilter, | ||
}; | ||
|
||
function mountComponent(field: IndexPatternField) { | ||
const compProps = { ...defaultProps, field }; | ||
return mountWithIntl(<DiscoverFieldDetails {...compProps} />); | ||
} | ||
|
||
it('should enable the visualize link for a number field', function () { | ||
it('click on addFilter calls the function', function () { | ||
const visualizableField = new IndexPatternField({ | ||
name: 'bytes', | ||
type: 'number', | ||
|
@@ -47,37 +48,9 @@ describe('discover sidebar field details', function () { | |
aggregatable: true, | ||
readFromDocValues: true, | ||
}); | ||
const comp = mountComponent(visualizableField); | ||
expect(findTestSubject(comp, 'fieldVisualize-bytes')).toBeTruthy(); | ||
}); | ||
|
||
it('should disable the visualize link for an _id field', function () { | ||
const conflictField = new IndexPatternField({ | ||
name: '_id', | ||
type: 'string', | ||
esTypes: ['_id'], | ||
count: 0, | ||
scripted: false, | ||
searchable: true, | ||
aggregatable: true, | ||
readFromDocValues: true, | ||
}); | ||
const comp = mountComponent(conflictField); | ||
expect(findTestSubject(comp, 'fieldVisualize-_id')).toEqual({}); | ||
}); | ||
|
||
it('should disable the visualize link for an unknown field', function () { | ||
const unknownField = new IndexPatternField({ | ||
name: 'test', | ||
type: 'unknown', | ||
esTypes: ['double'], | ||
count: 0, | ||
scripted: false, | ||
searchable: true, | ||
aggregatable: true, | ||
readFromDocValues: true, | ||
}); | ||
const comp = mountComponent(unknownField); | ||
expect(findTestSubject(comp, 'fieldVisualize-test')).toEqual({}); | ||
const component = mountComponent(visualizableField); | ||
const onAddButton = findTestSubject(component, 'onAddFilterButton'); | ||
onAddButton.simulate('click'); | ||
expect(onAddFilter).toHaveBeenCalledWith('_exists_', visualizableField.name, '+'); | ||
Comment on lines
+51
to
+54
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. ℹ️ This test got moved from the now deleted "Footer" component. |
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,105 +6,69 @@ | |
* Side Public License, v 1. | ||
*/ | ||
|
||
import React, { useState, useEffect } from 'react'; | ||
import { EuiIconTip, EuiText, EuiButton, EuiSpacer } from '@elastic/eui'; | ||
import React from 'react'; | ||
import { EuiText, EuiSpacer, EuiLink } from '@elastic/eui'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { METRIC_TYPE, UiCounterMetricType } from '@kbn/analytics'; | ||
import { DiscoverFieldBucket } from './discover_field_bucket'; | ||
import { getWarnings } from './lib/get_warnings'; | ||
import { | ||
triggerVisualizeActions, | ||
isFieldVisualizable, | ||
getVisualizeHref, | ||
} from './lib/visualize_trigger_utils'; | ||
import { Bucket, FieldDetails } from './types'; | ||
import { IndexPatternField, IndexPattern } from '../../../../../../../data/public'; | ||
import './discover_field_details.scss'; | ||
|
||
interface DiscoverFieldDetailsProps { | ||
field: IndexPatternField; | ||
indexPattern: IndexPattern; | ||
details: FieldDetails; | ||
onAddFilter: (field: IndexPatternField | string, value: string, type: '+' | '-') => void; | ||
trackUiMetric?: (metricType: UiCounterMetricType, eventName: string | string[]) => void; | ||
} | ||
|
||
export function DiscoverFieldDetails({ | ||
field, | ||
indexPattern, | ||
details, | ||
onAddFilter, | ||
trackUiMetric, | ||
}: DiscoverFieldDetailsProps) { | ||
const warnings = getWarnings(field); | ||
const [showVisualizeLink, setShowVisualizeLink] = useState<boolean>(false); | ||
const [visualizeLink, setVisualizeLink] = useState<string>(''); | ||
|
||
useEffect(() => { | ||
isFieldVisualizable(field, indexPattern.id, details.columns).then( | ||
(flag) => { | ||
setShowVisualizeLink(flag); | ||
// get href only if Visualize button is enabled | ||
getVisualizeHref(field, indexPattern.id, details.columns).then( | ||
(uri) => { | ||
if (uri) setVisualizeLink(uri); | ||
}, | ||
() => { | ||
setVisualizeLink(''); | ||
} | ||
); | ||
}, | ||
() => { | ||
setShowVisualizeLink(false); | ||
} | ||
); | ||
}, [field, indexPattern.id, details.columns]); | ||
|
||
const handleVisualizeLinkClick = (event: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => { | ||
// regular link click. let the uiActions code handle the navigation and show popup if needed | ||
event.preventDefault(); | ||
if (trackUiMetric) { | ||
trackUiMetric(METRIC_TYPE.CLICK, 'visualize_link_click'); | ||
} | ||
triggerVisualizeActions(field, indexPattern.id, details.columns); | ||
}; | ||
|
||
return ( | ||
<> | ||
<div className="dscFieldDetails"> | ||
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. ℹ️ This CSS class just added a couple of spacing (I replaced that by some well placed |
||
<div className=""> | ||
{details.error && <EuiText size="xs">{details.error}</EuiText>} | ||
{!details.error && ( | ||
<div style={{ marginTop: '4px' }}> | ||
{details.buckets.map((bucket: Bucket, idx: number) => ( | ||
<DiscoverFieldBucket | ||
key={`bucket${idx}`} | ||
bucket={bucket} | ||
field={field} | ||
onAddFilter={onAddFilter} | ||
/> | ||
))} | ||
</div> | ||
)} | ||
|
||
{showVisualizeLink && ( | ||
<> | ||
<div style={{ marginTop: '4px' }}> | ||
{details.buckets.map((bucket: Bucket, idx: number) => ( | ||
<DiscoverFieldBucket | ||
key={`bucket${idx}`} | ||
bucket={bucket} | ||
field={field} | ||
onAddFilter={onAddFilter} | ||
/> | ||
))} | ||
</div> | ||
<EuiSpacer size="xs" /> | ||
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */} | ||
<EuiButton | ||
onClick={(e) => handleVisualizeLinkClick(e)} | ||
href={visualizeLink} | ||
size="s" | ||
className="dscFieldDetails__visualizeBtn" | ||
data-test-subj={`fieldVisualize-${field.name}`} | ||
> | ||
<FormattedMessage | ||
id="discover.fieldChooser.detailViews.visualizeLinkText" | ||
defaultMessage="Visualize" | ||
/> | ||
</EuiButton> | ||
{warnings.length > 0 && ( | ||
<EuiIconTip type="alert" color="warning" content={warnings.join(' ')} /> | ||
)} | ||
<EuiText size="xs"> | ||
{!indexPattern.metaFields.includes(field.name) && !field.scripted ? ( | ||
<EuiLink | ||
onClick={() => onAddFilter('_exists_', field.name, '+')} | ||
data-test-subj="onAddFilterButton" | ||
> | ||
<FormattedMessage | ||
id="discover.fieldChooser.detailViews.existsInRecordsText" | ||
defaultMessage="Exists in {value} / {totalValue} records" | ||
values={{ | ||
value: details.exists, | ||
totalValue: details.total, | ||
}} | ||
/> | ||
</EuiLink> | ||
) : ( | ||
<FormattedMessage | ||
id="discover.fieldChooser.detailViews.valueOfRecordsText" | ||
defaultMessage="{value} / {totalValue} records" | ||
values={{ | ||
value: details.exists, | ||
totalValue: details.total, | ||
}} | ||
/> | ||
)} | ||
</EuiText> | ||
</> | ||
)} | ||
</div> | ||
|
This file was deleted.
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.
ℹ️ I've removed all those tests, since they were anyway not working. The only test that checked if the button was visible, did not do that correctly (since the result of
findTestSubject
will always be truthy and a.exists()
check is required). If that test would have been tested properly it would have always failed, due to the async nature of the component. I instead decided to replace this by unit tests for the utility class to load that visualization information.