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

[Discover] Create field_button and add popovers to sidebar #73226

Merged
merged 53 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
19747a5
cleanup of fielditem with accordions
andreadelrio Jul 7, 2020
19ccf91
clean sass and update tests
andreadelrio Jul 8, 2020
447c72d
bring back useShortDots, accidentally deleted
andreadelrio Jul 8, 2020
0350653
first pass on making the switch to popovers
andreadelrio Jul 9, 2020
21d9901
popover title
andreadelrio Jul 13, 2020
2d489fc
fix conflicts
andreadelrio Jul 16, 2020
550869f
Merge remote-tracking branch 'upstream/master' into discover-popovers
andreadelrio Jul 21, 2020
4ff26c8
field button in progress
andreadelrio Jul 23, 2020
eec4031
moving styles around
andreadelrio Jul 23, 2020
7a6bfad
will add size to field_button
andreadelrio Jul 23, 2020
c2d8ea2
added size to field_button
andreadelrio Jul 23, 2020
e72dfb1
some progress on tests
andreadelrio Jul 24, 2020
b02a777
more tests
andreadelrio Jul 25, 2020
bec896f
cleanup
andreadelrio Jul 27, 2020
81a70ea
fixing some tests
andreadelrio Jul 27, 2020
388ff29
add EuiKeyboardAccessible to Discover's sidebar
andreadelrio Jul 29, 2020
9306d83
Merge branch 'master' into discover-popovers
elasticmachine Jul 30, 2020
ddd1739
moving interactive content out of interactive content
Jul 31, 2020
ba9ba6a
Merge branch 'master' into discover-popovers
elasticmachine Aug 3, 2020
583e22d
Fixing lens popover focus
Aug 3, 2020
a358c04
adding allowing both icon and action
Aug 3, 2020
aeefaa2
keep focus ring open on popover open
Aug 3, 2020
f960dc8
fix overlap of infoIcon
andreadelrio Aug 4, 2020
49dffc6
Merge pull request #3 from andreadelrio/button-append
myasonik Aug 4, 2020
2dae400
Merge pull request #2 from myasonik/pr/73226
andreadelrio Aug 4, 2020
f779e56
snaps and delete extra css line
andreadelrio Aug 4, 2020
90504df
snaps and delete extra css line
andreadelrio Aug 4, 2020
1c79015
pr feedback
andreadelrio Aug 5, 2020
332283f
adjust alignment
andreadelrio Aug 5, 2020
fec2bd5
Merge branch 'discover-popovers' of https://github.com/andreadelrio/k…
Aug 5, 2020
08c66f9
showing buttons on discover sidebar when tabbing
andreadelrio Aug 5, 2020
6a06de0
Moving `kbnFieldButton__infoIcon` into `<button>`
Aug 5, 2020
19f739b
improve popover in Discover sidebar
andreadelrio Aug 5, 2020
f09432e
Merge branch 'master' into discover-popovers
elasticmachine Aug 6, 2020
3cbd327
Merge pull request #3 from cchaos/andreadelrio-discover-popovers
andreadelrio Aug 6, 2020
eafc748
fix 1 test and update snaps
andreadelrio Aug 6, 2020
4c72d5a
fix typo
andreadelrio Aug 6, 2020
243bcff
cleanup
andreadelrio Aug 6, 2020
68251ef
fix field_button test
andreadelrio Aug 10, 2020
9ce49a6
fix discover_sidebar test
andreadelrio Aug 10, 2020
af06e49
fix Lens test
flash1293 Aug 10, 2020
f68adc9
Merge branch 'master' into discover-popovers
elasticmachine Aug 10, 2020
299ea36
use button icons for add/remove in discover sidebar
andreadelrio Aug 11, 2020
c4350f9
Merge branch 'discover-popovers' of https://github.com/andreadelrio/k…
andreadelrio Aug 11, 2020
c390b71
pass type check
andreadelrio Aug 11, 2020
eda738e
Merge branch 'master' into discover-popovers
elasticmachine Aug 13, 2020
eee6c07
fix aria-labels and add tooltips
andreadelrio Aug 14, 2020
6aa13ee
Merge remote-tracking branch 'upstream/master' into discover-popovers
andreadelrio Aug 14, 2020
bc516d4
show add icon for active items in Discover
andreadelrio Aug 16, 2020
cb2a892
Merge branch 'master' into discover-popovers
elasticmachine Aug 16, 2020
e1bbcdb
show DiscoverFieldDetails only when infoIsOpen
andreadelrio Aug 17, 2020
26c46af
Merge branch 'discover-popovers' of https://github.com/andreadelrio/k…
andreadelrio Aug 17, 2020
c2de18d
remove extra markup
andreadelrio Aug 18, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.dscSidebarItem__fieldPopoverPanel {
min-width: 260px;
max-width: 300px;
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import { EuiButton } from '@elastic/eui';
import React, { useState } from 'react';
import { EuiButton, EuiPopover, EuiPopoverTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { DiscoverFieldDetails } from './discover_field_details';
import { FieldIcon } from '../../../../../kibana_react/public';
import { FieldIcon, FieldButton } from '../../../../../kibana_react/public';
import { FieldDetails } from './types';
import { IndexPatternField, IndexPattern } from '../../../../../data/public';
import { shortenDottedString } from '../../helpers';
import { getFieldTypeName } from './lib/get_field_type_name';
import './discover_field.scss';

export interface DiscoverFieldProps {
/**
Expand Down Expand Up @@ -100,6 +101,8 @@ export function DiscoverField({
}
);

const [infoIsOpen, setOpen] = useState(false);
andreadelrio marked this conversation as resolved.
Show resolved Hide resolved

const toggleDisplay = (f: IndexPatternField) => {
if (selected) {
onRemoveField(f.name);
Expand All @@ -108,70 +111,105 @@ export function DiscoverField({
}
};

function togglePopover() {
setOpen(!infoIsOpen);
}

function wrapOnDot(str?: string) {
// u200B is a non-width white-space character, which allows
// the browser to efficiently word-wrap right after the dot
// without us having to draw a lot of extra DOM elements, etc
return str ? str.replace(/\./g, '.\u200B') : '';
}

const dscFieldIcon = (
<FieldIcon type={field.type} label={getFieldTypeName(field.type)} scripted={field.scripted} />
);

const fieldName = (
<span
data-test-subj={`field-${field.name}`}
title={field.name}
className="dscSidebarField__name"
>
{useShortDots ? wrapOnDot(shortenDottedString(field.name)) : wrapOnDot(field.displayName)}
</span>
);

let actionButton;
if (field.name !== '_source' && !selected) {
kertal marked this conversation as resolved.
Show resolved Hide resolved
actionButton = (
<EuiButton
fill
size="s"
className="dscSidebarItem__action"
onClick={(ev: React.MouseEvent<HTMLButtonElement>) => {
ev.preventDefault();
ev.stopPropagation();
toggleDisplay(field);
}}
data-test-subj={`fieldToggle-${field.name}`}
arial-label={addLabelAria}
>
{addLabel}
</EuiButton>
);
} else if (field.name !== '_source' && selected) {
actionButton = (
<EuiButton
color="danger"
className="dscSidebarItem__action"
onClick={(ev: React.MouseEvent<HTMLButtonElement>) => {
ev.preventDefault();
ev.stopPropagation();
toggleDisplay(field);
}}
data-test-subj={`fieldToggle-${field.name}`}
arial-label={removeLabelAria}
>
{removeLabel}
</EuiButton>
);
}

return (
<>
andreadelrio marked this conversation as resolved.
Show resolved Hide resolved
<div
className={`dscSidebarField dscSidebarItem ${showDetails ? 'dscSidebarItem--active' : ''}`}
tabIndex={0}
onClick={() => onShowDetails(!showDetails, field)}
onKeyPress={() => onShowDetails(!showDetails, field)}
data-test-subj={`field-${field.name}-showDetails`}
>
<span className="dscSidebarField__fieldIcon">
<FieldIcon
type={field.type}
label={getFieldTypeName(field.type)}
scripted={field.scripted}
<EuiPopover
ownFocus
display="block"
button={
<FieldButton
size="s"
className="dscSidebarItem"
isActive={infoIsOpen}
onClick={() => {
togglePopover();
}}
data-test-subj={`field-${field.name}-showDetails`}
fieldIcon={dscFieldIcon}
fieldAction={actionButton}
fieldName={fieldName}
/>
</span>
<span
data-test-subj={`field-${field.name}`}
title={field.name}
className="dscSidebarField__name"
>
{useShortDots ? wrapOnDot(shortenDottedString(field.name)) : wrapOnDot(field.displayName)}
</span>
<span>
{field.name !== '_source' && !selected && (
<EuiButton
fill
size="s"
className="dscSidebarItem__action"
onClick={(ev: React.MouseEvent<HTMLButtonElement>) => {
ev.preventDefault();
ev.stopPropagation();
toggleDisplay(field);
}}
data-test-subj={`fieldToggle-${field.name}`}
arial-label={addLabelAria}
>
{addLabel}
</EuiButton>
)}
{field.name !== '_source' && selected && (
<EuiButton
color="danger"
className="dscSidebarItem__action"
onClick={(ev: React.MouseEvent<HTMLButtonElement>) => {
ev.preventDefault();
ev.stopPropagation();
toggleDisplay(field);
}}
data-test-subj={`fieldToggle-${field.name}`}
arial-label={removeLabelAria}
>
{removeLabel}
</EuiButton>
)}
</span>
</div>
}
isOpen={infoIsOpen}
closePopover={() => setOpen(false)}
anchorPosition="rightUp"
panelClassName="dscSidebarItem__fieldPopoverPanel"
>
<EuiPopoverTitle>
{' '}
{i18n.translate('discover.fieldChooser.discoverField.fieldTopValuesLabel', {
defaultMessage: 'Top values',
})}
</EuiPopoverTitle>
<DiscoverFieldDetails
indexPattern={indexPattern}
field={field}
details={getDetails(field)}
onAddFilter={onAddFilter}
/>
</EuiPopover>

{showDetails && (
<DiscoverFieldDetails
indexPattern={indexPattern}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
.dscFieldDetails__visualizeBtn {
@include euiFontSizeXS;
height: $euiSizeL !important;
min-width: $euiSize * 4;
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
* under the License.
*/
import React from 'react';
import { EuiLink, EuiIconTip, EuiText } from '@elastic/eui';
import { EuiLink, EuiIconTip, EuiText, EuiPopoverFooter, EuiButton, EuiSpacer } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n/react';
import { DiscoverFieldBucket } from './discover_field_bucket';
import { getWarnings } from './lib/get_warnings';
import { Bucket, FieldDetails } from './types';
import { getServices } from '../../../kibana_services';
import { IndexPatternField, IndexPattern } from '../../../../../data/public';
import './discover_field_details.scss';

interface DiscoverFieldDetailsProps {
field: IndexPatternField;
Expand All @@ -41,62 +42,68 @@ export function DiscoverFieldDetails({
const warnings = getWarnings(field);

return (
<div className="dscFieldDetails">
{!details.error && (
<EuiText size="xs">
<FormattedMessage
id="discover.fieldChooser.detailViews.topValuesInRecordsDescription"
defaultMessage="Top 5 values in"
/>{' '}
{!indexPattern.metaFields.includes(field.name) && !field.scripted ? (
<EuiLink onClick={() => onAddFilter('_exists_', field.name, '+')}>
{details.exists}
</EuiLink>
) : (
<span>{details.exists}</span>
)}{' '}
/ {details.total}{' '}
<FormattedMessage
id="discover.fieldChooser.detailViews.recordsText"
defaultMessage="records"
/>
</EuiText>
)}
{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>
)}
<>
<div className="dscFieldDetails">
{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>
)}

{details.visualizeUrl && (
<>
<EuiLink
onClick={() => {
getServices().core.application.navigateToApp(details.visualizeUrl.app, {
path: details.visualizeUrl.path,
});
}}
className="kuiButton kuiButton--secondary kuiButton--small kuiVerticalRhythmSmall"
data-test-subj={`fieldVisualize-${field.name}`}
>
<FormattedMessage
id="discover.fieldChooser.detailViews.visualizeLinkText"
defaultMessage="Visualize"
/>
{details.visualizeUrl && (
<>
<EuiSpacer size="xs" />
<EuiButton
onClick={() => {
getServices().core.application.navigateToApp(details.visualizeUrl.app, {
path: details.visualizeUrl.path,
});
}}
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(' ')} />
)}
</EuiLink>
</>
</>
)}
</div>
{!details.error && (
<EuiPopoverFooter>
<EuiText size="xs" textAlign="center">
<FormattedMessage
id="discover.fieldChooser.detailViews.topValuesInRecordsDescription"
defaultMessage="Top 5 values in"
/>{' '}
{!indexPattern.metaFields.includes(field.name) && !field.scripted ? (
<EuiLink onClick={() => onAddFilter('_exists_', field.name, '+')}>
{details.exists}
</EuiLink>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm very confused by what this link does.

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'm also not sure what that link does. @kertal do you know?

Copy link
Member

Choose a reason for hiding this comment

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

it creates an exists filter, I don't know if anybody used it because it's quite invisible. however in the last testing session somebody found it and used it. so it's displaying the number of records that have this field. and if you click on it, you get those records, maybe a tooltip here would be useful ... or a more obvious solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you two work together to maybe find a better solution? I don't think using this number as a link is right because to me it looks like it should be working like pagination.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Came up with this proposal. @cchaos what do you think?

Group 3

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch on reducing the duplicate "Top values" text. I still don't think this clears up the question "Why is the number 74 blue, and what is going to happen when I click on it?"

I would make the behavior of this link a completely separate link from this number. What it's doing is adding filter to the global search bar with the name of the field. For example, this is what gets created when clicking that 74:

Screen Shot 2020-08-05 at 17 26 52 PM

So it's similar to those +/- buttons, but instead of filtering on those particular values of the field, it just filters down the results to only those documents that the field exists in. So basically it is narrowing down to those 74 documents.

Or even something like <Link>Exists in 74</Link> / 91 records with a tooltip explaining that it will add a filter to only those documents that contain the field.

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 think we can go with this for now
no0tooltip

There's no tooltip in it... I tried adding a tooltip but it always shows up when the popover opens and I don't think that's ideal.
tooltip

Copy link
Member

Choose a reason for hiding this comment

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

@andreadelrio I like this solution, for me it's much clearer this way

) : (
<span>{details.exists}</span>
)}{' '}
/ {details.total}{' '}
<FormattedMessage
id="discover.fieldChooser.detailViews.recordsText"
defaultMessage="records"
/>
</EuiText>
</EuiPopoverFooter>
)}
</div>
</>
);
}
Loading