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.

Sign up for GitHub

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

Jump to bottom

[Lens] Filters aggregation #75635

Merged
merged 43 commits into from
Sep 10, 2020
Merged

[Lens] Filters aggregation #75635

Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
e82e536
checkpoint
mbondyra Aug 21, 2020
47ba9df
filters aggregation tweaks and types
mbondyra Aug 24, 2020
38fb236
refactoring
mbondyra Aug 24, 2020
be5ae8d
style fixes
mbondyra Aug 24, 2020
8d1227d
addressing feedback
mbondyra Aug 25, 2020
66aada5
tests added
mbondyra Aug 26, 2020
030b5f7
types
mbondyra Aug 26, 2020
f21b11f
some of easier feedback addressed
mbondyra Aug 27, 2020
9f3f4a8
Merge commit '03002453d3d90831670c5215a613a52bd0306f00' into lens/fil…
mbondyra Sep 2, 2020
0c85af1
change of the popovers behavior
mbondyra Sep 2, 2020
07ce55a
error validation
mbondyra Sep 3, 2020
6fbac67
continue with feedback
mbondyra Sep 4, 2020
f97e5f6
after meeting fixes
mbondyra Sep 4, 2020
ccd9ed7
update tests
mbondyra Sep 4, 2020
becb89a
Merge commit 'fc22c7d9fb8aa767dcb5136f11451da0561a50a7' into lens/fil…
mbondyra Sep 4, 2020
c659183
added termstofilters behavior
mbondyra Sep 4, 2020
9537d67
Merge commit '05ac18a4a222e491c555a70f642787907b042dbc' into lens/fil…
mbondyra Sep 7, 2020
2503ba0
Merge commit '5a1d0753c07c64717f77a6f3c2004eabf285bd0a' into lens/fil…
mbondyra Sep 7, 2020
c3f3ded
fixing types
mbondyra Sep 7, 2020
9f2bd40
tests fixes
mbondyra Sep 8, 2020
e1fe770
Merge commit 'eecb0e59220c7c29811842a7911b6b2a3b872b5e' into lens/fil…
mbondyra Sep 8, 2020
fdbf1ae
remove magic mapping type
flash1293 Sep 8, 2020
acbffdb
Merge pull request #3 from flash1293/lens/operation-type-cleanup
mbondyra Sep 8, 2020
5c4c1d5
revert (simplify) if statements to ternary
mbondyra Sep 8, 2020
6e0b3f3
types again
mbondyra Sep 8, 2020
c0d36e6
defaultFilter fix
mbondyra Sep 8, 2020
a931828
Merge commit 'e827a6761e1667ea8b9ff1f10603849bc7219f91' into lens/fil…
mbondyra Sep 8, 2020
98da724
Merge commit '728dfb4b6bd6fd9ee1c736132b4b7f96fcccb70e' into lens/fil…
mbondyra Sep 8, 2020
2f6339d
Fixing up KWL bar to support `isInvalid`
Sep 8, 2020
03c00d8
Fixing up layout of filter agg button
Sep 8, 2020
3216553
types fixes
mbondyra Sep 9, 2020
c3de990
Merge branch 'master' into lens/filters_aggregation
mbondyra Sep 9, 2020
127e539
refactor: complicated condition
mbondyra Sep 9, 2020
81c4080
Merge branch 'lens/filters_aggregation' of github.com:mbondyra/kibana…
mbondyra Sep 9, 2020
3a29c0f
Fixing word-wrap and adding more titles
Sep 9, 2020
b539a0f
Cleanup
Sep 9, 2020
c18c658
Update x-pack/plugins/lens/public/editor_frame_service/editor_frame/c…
mbondyra Sep 9, 2020
871ec5b
Revert "Update x-pack/plugins/lens/public/editor_frame_service/editor…
Sep 9, 2020
5cf7490
refactor: copy
mbondyra Sep 10, 2020
e95af12
added tests
mbondyra Sep 10, 2020
eebbbdb
type
mbondyra Sep 10, 2020
e0058e2
Merge commit '100afab3085e735fa0624a07a54a7f752d92935f' into lens/fil…
mbondyra Sep 10, 2020
2978769
useDebounce properly this time
mbondyra Sep 10, 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
2 changes: 2 additions & 0 deletions src/plugins/data/public/ui/query_string_input/query_string_input.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import React, { Component, RefObject, createRef } from 'react';
import { i18n } from '@kbn/i18n';

import classNames from 'classnames';
import {
EuiTextArea,
Expand Down Expand Up @@ -591,6 +592,7 @@ export class QueryStringInputUI extends Component<Props, State> {
'euiFormControlLayout euiFormControlLayout--group kbnQueryBar__wrap',
this.props.className
);

return (
<div className={className}>
{this.props.prepend}
Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/public/ui/typeahead/suggestions_component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export class SuggestionsComponent extends Component<Props> {
const StyledSuggestionsListDiv = styled.div`
${(props: { queryBarRect: DOMRect; verticalListPosition: string }) => `
position: absolute;
z-index: 4001;
left: ${props.queryBarRect.left}px;
width: ${props.queryBarRect.width}px;
${props.verticalListPosition}`}
Expand Down
15 changes: 15 additions & 0 deletions x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/_layer_panel.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,21 @@
min-height: $euiSizeXXL;
}

.lnsLayerPanel__anchor {
width: 100%;
}

.lnsLayerPanel__dndGrab {
padding: $euiSizeS;
}

.lnsLayerPanel__filterLink {
min-height: $euiSizeL;
width: 100%;
align-self: flex-start;
word-break: break-word;
}

.lnsLayerPanel__styleEditor {
width: $euiSize * 30;
padding: $euiSizeS;
Expand Down
1 change: 1 addition & 0 deletions .../plugins/lens/public/editor_frame_service/editor_frame/config_panel/dimension_popover.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function DimensionPopover({
<EuiPopover
className="lnsDimensionPopover"
anchorClassName="lnsDimensionPopover__trigger"
panelClassName="lnsDimensionPanel__fixTranslateDnd"
isOpen={
popoverState.isOpen &&
(popoverState.openId === accessor || (noMatch && popoverState.addingToGroupId === groupId))
Expand Down
51 changes: 30 additions & 21 deletions x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/bucket_nesting_editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,34 @@ export function BucketNestingEditor({
}
}

const topLevelCopy: Record<string, string> = {
mbondyra marked this conversation as resolved.
Show resolved Hide resolved
terms: i18n.translate('xpack.lens.indexPattern.groupingOverallTerms', {
defaultMessage: 'Overall top {field}',
values: { field: fieldName },
}),
filters: i18n.translate('xpack.lens.indexPattern.groupingOverallFilters', {
defaultMessage: 'Search query overall',
}),
date_histogram: i18n.translate('xpack.lens.indexPattern.groupingOverallDateHistogram', {
defaultMessage: 'Dates overall',
}),
};

const bottomLevelCopy: Record<string, string> = {
terms: i18n.translate('xpack.lens.indexPattern.groupingSecondTerms', {
defaultMessage: 'Top values for each {target}',
values: { target: target.fieldName },
}),
filters: i18n.translate('xpack.lens.indexPattern.groupingSecondFilters', {
defaultMessage: 'Search query for each {target}',
values: { target: target.fieldName },
}),
date_histogram: i18n.translate('xpack.lens.indexPattern.groupingSecondDateHistogram', {
defaultMessage: 'Dates for each {target}',
values: { target: target.fieldName },
Copy link
Contributor

Choose a reason for hiding this comment

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

This phrasing has bothered me for a long time, and if you feel like it's related feel free to make a change here. It's probably a separate PR though. #76038

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'll open a PR for this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, this whole grouping section's copy is not just confusing to read by the grammar is not great and I know that's because of the re-use of the field name, but I think we need phrasing or formatting that considers this better. Also the option for this type of agg shows as "Top values of each Records". I think "Records" here is incorrect?

}),
};

return (
<>
<EuiHorizontalRule margin="m" />
Expand All @@ -73,33 +101,14 @@ export function BucketNestingEditor({
<EuiRadio
id={generator('topLevel')}
data-test-subj="indexPattern-nesting-topLevel"
label={
column.operationType === 'terms'
? i18n.translate('xpack.lens.indexPattern.groupingOverallTerms', {
defaultMessage: 'Overall top {field}',
values: { field: fieldName },
})
: i18n.translate('xpack.lens.indexPattern.groupingOverallDateHistogram', {
defaultMessage: 'Dates overall',
})
}
label={topLevelCopy[column.operationType]}
checked={!prevColumn}
onChange={toggleNesting}
/>
<EuiRadio
id={generator('bottomLevel')}
data-test-subj="indexPattern-nesting-bottomLevel"
label={
column.operationType === 'terms'
? i18n.translate('xpack.lens.indexPattern.groupingSecondTerms', {
defaultMessage: 'Top values for each {target}',
values: { target: target.fieldName },
})
: i18n.translate('xpack.lens.indexPattern.groupingSecondDateHistogram', {
defaultMessage: 'Dates for each {target}',
values: { target: target.fieldName },
})
}
label={bottomLevelCopy[column.operationType]}
checked={!!prevColumn}
onChange={toggleNesting}
/>
Expand Down
19 changes: 19 additions & 0 deletions x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,22 @@
padding-top: 0;
padding-bottom: 0;
}

.lnsIndexPatternDimensionEditor__filtersEditor {
max-width: 100%;
width: $euiSize * 60;
}

.lnsIndexPatternDimensionEditor__queryInput--invalid {
textarea {
// sass-lint:disable-block no-important
background-image: linear-gradient(to top, $euiColorDanger, $euiColorDanger 2px, transparent 2px, transparent 100%) !important;
background-size: 100%;
}
}

// todo: remove after closing https://github.com/elastic/eui/issues/3548
.lnsDimensionPanel__fixTranslateDnd {
// sass-lint:disable-block no-important
transform: none !important;
}
7 changes: 6 additions & 1 deletion x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/popover_editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ export function PopoverEditor(props: PopoverEditorProps) {
compatibleWithCurrentField ? '' : ' incompatible'
}`,
onClick() {
// todo: when moving from terms agg to filters, we want to create a filter `$field.name : *`
Copy link
Contributor

Choose a reason for hiding this comment

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

A quick option to do this in a clean way is to have a property on the operation definition acceptsColumn(otherColumn: IndexPatternColumn): boolean which is called instead of putting that logic here. The specific implementation would be part of the filters operation definition.

I'm fine with not doing that now, in that case let's create a technical debt issue for it.

Copy link
Contributor Author

@mbondyra mbondyra Sep 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The problem here is that other columns are already accepted if the field is compatible with them (the fact they are not accepted was the misconception I got from checking the previousColumn type we talked about async). In our case, top values and filters have totally non-compatible fields set. I think this has to be completely changed anyway once we allow creating fieldless columns. I'll create an issue, but without proposal for implementation, because it depends on fieldless column. Feel free to add some details if you have any ideas.
#77017

// it probably has to be re-thought when removing the field name.
const isTermsToFilters =
selectedColumn?.operationType === 'terms' && operationType === 'filters';

if (!selectedColumn || !compatibleWithCurrentField) {
const possibleFields = fieldByOperation[operationType] || [];

Expand All @@ -186,7 +191,7 @@ export function PopoverEditor(props: PopoverEditorProps) {
trackUiEvent(`indexpattern_dimension_operation_${operationType}`);
return;
}
if (incompatibleSelectedOperationType) {
if (incompatibleSelectedOperationType && !isTermsToFilters) {
setInvalidOperationType(null);
}
if (selectedColumn.operationType === operationType) {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ export function getIndexPatternDatasource({
data,
savedObjects: core.savedObjects,
docLinks: core.docLinks,
http: core.http,
}}
>
<IndexPatternDimensionEditor
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ export const cardinalityOperation: OperationDefinition<CardinalityIndexPatternCo
sourceField: field.name,
isBucketed: IS_BUCKETED,
params:
previousColumn && previousColumn.dataType === 'number' ? previousColumn.params : undefined,
previousColumn?.dataType === 'number' &&
mbondyra marked this conversation as resolved.
Show resolved Hide resolved
previousColumn.params &&
'format' in previousColumn.params
? previousColumn.params
: undefined,
};
},
toEsAggsConfig: (column, columnId) => ({
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn> = {
scale: 'ratio',
sourceField: field.name,
params:
previousColumn && previousColumn.dataType === 'number' ? previousColumn.params : undefined,
previousColumn?.dataType === 'number' &&
previousColumn.params &&
'format' in previousColumn.params
? previousColumn.params
: undefined,
};
},
toEsAggsConfig: (column, columnId) => ({
Expand Down
Loading