From fb2552661df91337465ea67bd4093d916110cef2 Mon Sep 17 00:00:00 2001 From: Jean-Louis Leysens Date: Mon, 15 Feb 2021 12:04:01 +0100 Subject: [PATCH 01/19] [ILM] Rollover min age tooltip and copy fixes (#91110) * removed an unnecessary tooltip in rollover field, added a tooltip to min age field when rollover is enabled * slight update to copy, added jest test and added comment about testing * page title and timeline title to sentence case * added link to learn more about timing to phase timeline component * fix jest test copy * remove unused import * fix i18n * remove unused translations * slight refactor to conditional for clarity * slight refactor to i18n text naming Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../client_integration/app/app.test.ts | 4 +- .../edit_policy/edit_policy.helpers.tsx | 6 +++ .../edit_policy/edit_policy.test.ts | 47 +++++++++++++++++++ .../sections/edit_policy/components/index.ts | 1 + .../components/phases/hot_phase/hot_phase.tsx | 26 +++------- .../min_age_field/min_age_field.tsx | 33 ++++++++++++- .../components/timeline/timeline.tsx | 22 +++++---- .../sections/edit_policy/edit_policy.tsx | 4 +- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 10 files changed, 108 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/app/app.test.ts b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/app/app.test.ts index 38cfad1a3b188..36f04be3b30b1 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/app/app.test.ts +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/app/app.test.ts @@ -22,8 +22,8 @@ const PERCENT_SIGN_NAME = 'test%'; const PERCENT_SIGN_WITH_OTHER_CHARS_NAME = 'test%#'; const PERCENT_SIGN_25_SEQUENCE = 'test%25'; -const createPolicyTitle = 'Create Policy'; -const editPolicyTitle = 'Edit Policy'; +const createPolicyTitle = 'Create policy'; +const editPolicyTitle = 'Edit policy'; window.scrollTo = jest.fn(); diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx index 7e1b7c5267a8b..83a13f0523a40 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.helpers.tsx @@ -271,6 +271,9 @@ export const setup = async (arg?: { appServicesContext: Partial (): boolean => + exists(`${phase}-rolloverMinAgeInputIconTip`); + return { ...testBed, actions: { @@ -306,6 +309,7 @@ export const setup = async (arg?: { appServicesContext: Partial exists('phaseErrorIndicator-warm'), + hasRolloverTipOnMinAge: hasRolloverTipOnMinAge('warm'), ...createShrinkActions('warm'), ...createForceMergeActions('warm'), setReadonly: setReadonly('warm'), @@ -321,11 +325,13 @@ export const setup = async (arg?: { appServicesContext: Partial exists('phaseErrorIndicator-cold'), + hasRolloverTipOnMinAge: hasRolloverTipOnMinAge('cold'), ...createIndexPriorityActions('cold'), ...createSearchableSnapshotActions('cold'), }, delete: { ...createToggleDeletePhaseActions(), + hasRolloverTipOnMinAge: hasRolloverTipOnMinAge('delete'), setMinAgeValue: setMinAgeValue('delete'), setMinAgeUnits: setMinAgeUnits('delete'), }, diff --git a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts index 6f325084938e8..f1a15d805faf8 100644 --- a/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts +++ b/x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/edit_policy.test.ts @@ -769,6 +769,38 @@ describe('', () => { }); }); }); + describe('with rollover', () => { + beforeEach(async () => { + httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]); + httpRequestsMockHelpers.setListNodes({ + isUsingDeprecatedDataRoleConfig: false, + nodesByAttributes: { test: ['123'] }, + nodesByRoles: { data: ['123'] }, + }); + httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['abc'] }); + httpRequestsMockHelpers.setLoadSnapshotPolicies([]); + + await act(async () => { + testBed = await setup(); + }); + + const { component } = testBed; + component.update(); + }); + + test('shows rollover tip on minimum age', async () => { + const { actions } = testBed; + + await actions.warm.enable(true); + await actions.cold.enable(true); + await actions.delete.enablePhase(); + + expect(actions.warm.hasRolloverTipOnMinAge()).toBeTruthy(); + expect(actions.cold.hasRolloverTipOnMinAge()).toBeTruthy(); + expect(actions.delete.hasRolloverTipOnMinAge()).toBeTruthy(); + }); + }); + describe('without rollover', () => { beforeEach(async () => { httpRequestsMockHelpers.setLoadPolicies([getDefaultHotPhasePolicy('my_policy')]); @@ -778,6 +810,7 @@ describe('', () => { nodesByRoles: { data: ['123'] }, }); httpRequestsMockHelpers.setListSnapshotRepos({ repositories: ['found-snapshots'] }); + httpRequestsMockHelpers.setLoadSnapshotPolicies([]); await act(async () => { testBed = await setup({ @@ -799,6 +832,20 @@ describe('', () => { expect(actions.hot.searchableSnapshotsExists()).toBeFalsy(); expect(actions.cold.searchableSnapshotDisabledDueToRollover()).toBeTruthy(); }); + + test('hiding rollover tip on minimum age', async () => { + const { actions } = testBed; + await actions.hot.toggleDefaultRollover(false); + await actions.hot.toggleRollover(false); + + await actions.warm.enable(true); + await actions.cold.enable(true); + await actions.delete.enablePhase(); + + expect(actions.warm.hasRolloverTipOnMinAge()).toBeFalsy(); + expect(actions.cold.hasRolloverTipOnMinAge()).toBeFalsy(); + expect(actions.delete.hasRolloverTipOnMinAge()).toBeFalsy(); + }); }); describe('policy timeline', () => { diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts index dc4f1e31d3696..ccc553c58e899 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts @@ -13,4 +13,5 @@ export { FieldLoadingError } from './field_loading_error'; export { Timeline } from './timeline'; export { FormErrorsCallout } from './form_errors_callout'; export { PhaseFooter } from './phase_footer'; +export { InfinityIcon } from './infinity_icon'; export * from './phases'; diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx index c77493476b929..6d4e2750bb2e8 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx @@ -16,7 +16,6 @@ import { EuiCallOut, EuiTextColor, EuiSwitch, - EuiIconTip, EuiText, } from '@elastic/eui'; @@ -121,25 +120,12 @@ export const HotPhase: FunctionComponent = () => {
path="_meta.hot.customRollover.enabled"> {(field) => ( - <> - field.setValue(e.target.checked)} - data-test-subj="rolloverSwitch" - /> -   - - } - /> - + field.setValue(e.target.checked)} + data-test-subj="rolloverSwitch" + /> )} {isUsingRollover && ( diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/min_age_field/min_age_field.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/min_age_field/min_age_field.tsx index 2f1a058f5a943..04b756dc23559 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/min_age_field/min_age_field.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/min_age_field/min_age_field.tsx @@ -17,11 +17,12 @@ import { EuiFormRow, EuiSelect, EuiText, + EuiIconTip, } from '@elastic/eui'; import { getFieldValidityAndErrorMessage } from '../../../../../../../shared_imports'; -import { UseField } from '../../../../form'; +import { UseField, useConfigurationIssues } from '../../../../form'; import { getUnitsAriaLabelForPhase, getTimingLabelForPhase } from './util'; @@ -62,6 +63,17 @@ const i18nTexts = { defaultMessage: 'nanoseconds', } ), + rolloverToolTipDescription: i18n.translate( + 'xpack.indexLifecycleMgmt.editPolicy.minimumAge.rolloverToolTipDescription', + { + defaultMessage: + 'Data age is calculated from rollover. Rollover is configured in the hot phase.', + } + ), + minAgeUnitFieldSuffix: i18n.translate( + 'xpack.indexLifecycleMgmt.editPolicy.minimumAge.minimumAgeFieldSuffixLabel', + { defaultMessage: 'old' } + ), }; interface Props { @@ -69,6 +81,7 @@ interface Props { } export const MinAgeField: FunctionComponent = ({ phase }): React.ReactElement => { + const { isUsingRollover } = useConfigurationIssues(); return ( {(field) => { @@ -110,6 +123,22 @@ export const MinAgeField: FunctionComponent = ({ phase }): React.ReactEle const { isInvalid: isUnitFieldInvalid } = getFieldValidityAndErrorMessage( unitField ); + const icon = ( + <> + {/* This element is rendered for testing purposes only */} +
+ + + ); + const selectAppendValue: Array< + string | React.ReactElement + > = isUsingRollover + ? [i18nTexts.minAgeUnitFieldSuffix, icon] + : [i18nTexts.minAgeUnitFieldSuffix]; return ( = ({ phase }): React.ReactEle unitField.setValue(e.target.value); }} isInvalid={isUnitFieldInvalid} - append={'old'} + append={selectAppendValue} data-test-subj={`${phase}-selectedMinimumAgeUnits`} aria-label={getUnitsAriaLabelForPhase(phase)} options={[ diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx index 8097ab51eb59e..c996c45171d2f 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx @@ -6,6 +6,7 @@ */ import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; import React, { FunctionComponent, memo } from 'react'; import { EuiFlexGroup, EuiFlexItem, EuiTitle, EuiText, EuiIconTip } from '@elastic/eui'; @@ -18,7 +19,7 @@ import { AbsoluteTimings, } from '../../lib'; -import { InfinityIcon } from '../infinity_icon'; +import { InfinityIcon, LearnMoreLink } from '..'; import { TimelinePhaseText } from './components'; @@ -47,7 +48,7 @@ const SCORE_BUFFER_AMOUNT = 50; const i18nTexts = { title: i18n.translate('xpack.indexLifecycleMgmt.timeline.title', { - defaultMessage: 'Policy Summary', + defaultMessage: 'Policy summary', }), description: i18n.translate('xpack.indexLifecycleMgmt.timeline.description', { defaultMessage: 'This policy moves data through the following phases.', @@ -55,13 +56,6 @@ const i18nTexts = { hotPhase: i18n.translate('xpack.indexLifecycleMgmt.timeline.hotPhaseSectionTitle', { defaultMessage: 'Hot phase', }), - rolloverTooltip: i18n.translate( - 'xpack.indexLifecycleMgmt.timeline.hotPhaseRolloverToolTipContent', - { - defaultMessage: - 'How long it takes to reach the rollover criteria in the hot phase can vary. Data moves to the next phase when the time since rollover reaches the minimum age.', - } - ), warmPhase: i18n.translate('xpack.indexLifecycleMgmt.timeline.warmPhaseSectionTitle', { defaultMessage: 'Warm phase', }), @@ -143,6 +137,16 @@ export const Timeline: FunctionComponent = memo( {i18nTexts.description} +   + + } + /> diff --git a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx index 0c7b5565372a5..637fbd893aaa0 100644 --- a/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx +++ b/x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx @@ -142,10 +142,10 @@ export const EditPolicy: React.FunctionComponent = ({ history }) => {

{isNewPolicy ? i18n.translate('xpack.indexLifecycleMgmt.editPolicy.createPolicyMessage', { - defaultMessage: 'Create Policy', + defaultMessage: 'Create policy', }) : i18n.translate('xpack.indexLifecycleMgmt.editPolicy.editPolicyMessage', { - defaultMessage: 'Edit Policy {originalPolicyName}', + defaultMessage: 'Edit policy {originalPolicyName}', values: { originalPolicyName }, })}

diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 4d94539a514d1..2d4584748c39d 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -9467,7 +9467,6 @@ "xpack.indexLifecycleMgmt.editPolicy.forcemerge.numberOfSegmentsRequiredError": "セグメント数の評価が必要です。", "xpack.indexLifecycleMgmt.editPolicy.formErrorsMessage": "このページのエラーを修正してください。", "xpack.indexLifecycleMgmt.editPolicy.hidePolicyJsonButto": "リクエストを非表示", - "xpack.indexLifecycleMgmt.editPolicy.hotPhase.enableRolloverTipContent": "現在のインデックスが定義された条件のいずれかを満たすときに、新しいインデックスにロールオーバーします。", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.learnAboutRolloverLinkText": "詳細", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.rolloverDefaultsTipContent": "インデックスが 30 日経過するか、50 GB に達したらロールオーバーします。", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.rolloverDescriptionMessage": "効率的なストレージと高いパフォーマンスのための時系列データの自動ロールアウト。", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index a35d4c67dde00..50e178527c3ec 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -9491,7 +9491,6 @@ "xpack.indexLifecycleMgmt.editPolicy.forcemerge.numberOfSegmentsRequiredError": "必须指定分段数的值。", "xpack.indexLifecycleMgmt.editPolicy.formErrorsMessage": "请修复此页面上的错误。", "xpack.indexLifecycleMgmt.editPolicy.hidePolicyJsonButto": "隐藏请求", - "xpack.indexLifecycleMgmt.editPolicy.hotPhase.enableRolloverTipContent": "在当前索引满足定义的条件之一时,滚动更新到新索引。", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.learnAboutRolloverLinkText": "了解详情", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.rolloverDefaultsTipContent": "当索引已存在 30 天或达到 50 GB 时滚动更新。", "xpack.indexLifecycleMgmt.editPolicy.hotPhase.rolloverDescriptionMessage": "自动滚动更新时间序列数据,以实现高效存储和更高性能。", From 75a7f78730b7672ae45894b9bd7537401f9a3923 Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Mon, 15 Feb 2021 12:25:19 +0100 Subject: [PATCH 02/19] [Lens] Improves ranking feature in Top values (#90749) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../operations/definitions/terms/index.tsx | 49 ++++++++++++++++--- .../translations/translations/ja-JP.json | 2 - .../translations/translations/zh-CN.json | 2 - 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx index 517cb941f2f67..3b0cb67cbce41 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/index.tsx @@ -16,6 +16,7 @@ import { EuiPopover, EuiButtonEmpty, EuiText, + EuiIconTip, } from '@elastic/eui'; import { AggFunctionsMapping } from '../../../../../../../../src/plugins/data/public'; import { buildExpressionFunction } from '../../../../../../../../src/plugins/expressions/public'; @@ -316,9 +317,25 @@ export const termsOperation: OperationDefinition )} + {i18n.translate('xpack.lens.indexPattern.terms.orderBy', { + defaultMessage: 'Rank by', + })}{' '} + + + } display="columnCompressed" fullWidth > @@ -338,14 +355,30 @@ export const termsOperation: OperationDefinition + {i18n.translate('xpack.lens.indexPattern.terms.orderDirection', { + defaultMessage: 'Rank direction', + })}{' '} + + + } display="columnCompressed" fullWidth > @@ -378,7 +411,7 @@ export const termsOperation: OperationDefinition diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 2d4584748c39d..ed11b669f231a 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -11326,9 +11326,7 @@ "xpack.lens.indexPattern.terms.missingLabel": "(欠落値)", "xpack.lens.indexPattern.terms.orderAlphabetical": "アルファベット順", "xpack.lens.indexPattern.terms.orderAscending": "昇順", - "xpack.lens.indexPattern.terms.orderBy": "並び順", "xpack.lens.indexPattern.terms.orderDescending": "降順", - "xpack.lens.indexPattern.terms.orderDirection": "全体的な方向", "xpack.lens.indexPattern.terms.otherBucketDescription": "他の値を「その他」としてグループ化", "xpack.lens.indexPattern.terms.otherLabel": "その他", "xpack.lens.indexPattern.terms.size": "値の数", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index 50e178527c3ec..c4c827ca7f55e 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -11354,9 +11354,7 @@ "xpack.lens.indexPattern.terms.missingLabel": "(缺失值)", "xpack.lens.indexPattern.terms.orderAlphabetical": "按字母顺序", "xpack.lens.indexPattern.terms.orderAscending": "升序", - "xpack.lens.indexPattern.terms.orderBy": "排序依据", "xpack.lens.indexPattern.terms.orderDescending": "降序", - "xpack.lens.indexPattern.terms.orderDirection": "排序方向", "xpack.lens.indexPattern.terms.otherBucketDescription": "将其他值分组为“其他”", "xpack.lens.indexPattern.terms.otherLabel": "其他", "xpack.lens.indexPattern.terms.size": "值数目", From 0ee7be1f0d9dfdb4c9880d74ba50bedffbd8a3db Mon Sep 17 00:00:00 2001 From: Marta Bondyra Date: Mon, 15 Feb 2021 12:48:06 +0100 Subject: [PATCH 03/19] [Lens] Keyboard-selected items follow user traversal of drop zones (#90546) --- .../__snapshots__/drag_drop.test.tsx.snap | 15 +- .../lens/public/drag_drop/drag_drop.scss | 36 +++- .../lens/public/drag_drop/drag_drop.test.tsx | 173 ++++++++++++++---- .../lens/public/drag_drop/drag_drop.tsx | 56 +++++- .../lens/public/drag_drop/providers.tsx | 19 +- .../config_panel/layer_panel.test.tsx | 26 ++- .../editor_frame/editor_frame.test.tsx | 10 +- .../workspace_panel_wrapper.scss | 5 +- .../dimension_panel/droppable.test.ts | 7 +- .../indexpattern_datasource/field_item.scss | 7 + 10 files changed, 285 insertions(+), 69 deletions(-) diff --git a/x-pack/plugins/lens/public/drag_drop/__snapshots__/drag_drop.test.tsx.snap b/x-pack/plugins/lens/public/drag_drop/__snapshots__/drag_drop.test.tsx.snap index b3b695b22ad71..e5594bb0bb769 100644 --- a/x-pack/plugins/lens/public/drag_drop/__snapshots__/drag_drop.test.tsx.snap +++ b/x-pack/plugins/lens/public/drag_drop/__snapshots__/drag_drop.test.tsx.snap @@ -1,12 +1,16 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`DragDrop defined dropType is reflected in the className 1`] = ` - + +
`; exports[`DragDrop items that has dropType=undefined get special styling when another item is dragged 1`] = ` @@ -23,6 +27,7 @@ exports[`DragDrop items that has dropType=undefined get special styling when ano exports[`DragDrop renders if nothing is being dragged 1`] = `
+ ); @@ -96,7 +97,7 @@ describe('DragDrop', () => { jest.runAllTimers(); expect(dataTransfer.setData).toBeCalledWith('text', 'hello'); - expect(setDragging).toBeCalledWith(value); + expect(setDragging).toBeCalledWith({ ...value }); expect(setA11yMessage).toBeCalledWith('Lifted hello'); }); @@ -175,7 +176,7 @@ describe('DragDrop', () => { test('items that has dropType=undefined get special styling when another item is dragged', () => { const component = mount( - + @@ -198,7 +199,6 @@ describe('DragDrop', () => { const getAdditionalClassesOnEnter = jest.fn().mockReturnValue('additional'); const getAdditionalClassesOnDroppable = jest.fn().mockReturnValue('droppable'); const setA11yMessage = jest.fn(); - let activeDropTarget; const component = mount( { setDragging={() => { dragging = { id: '1', humanData: { label: 'label1' } }; }} - setActiveDropTarget={(val) => { - activeDropTarget = { activeDropTarget: val }; - }} - activeDropTarget={activeDropTarget} > { , style: {} } }, setActiveDropTarget, setA11yMessage, activeDropTarget: { @@ -376,21 +372,115 @@ describe('DragDrop', () => { .simulate('focus'); act(() => { keyboardHandler.simulate('keydown', { key: 'ArrowRight' }); - expect(setActiveDropTarget).toBeCalledWith({ - ...items[2].value, - onDrop, - dropType: items[2].dropType, - }); - keyboardHandler.simulate('keydown', { key: 'Enter' }); - expect(setA11yMessage).toBeCalledWith( - 'Selected label3 in group at position 1. Press space or enter to replace label3 with label1.' - ); - expect(setActiveDropTarget).toBeCalledWith(undefined); - expect(onDrop).toBeCalledWith( - { humanData: { label: 'label1', position: 1 }, id: '1' }, - 'move_compatible' - ); }); + expect(setActiveDropTarget).toBeCalledWith({ + ...items[2].value, + onDrop, + dropType: items[2].dropType, + }); + keyboardHandler.simulate('keydown', { key: 'Enter' }); + expect(setA11yMessage).toBeCalledWith( + 'Selected label3 in group at position 1. Press space or enter to replace label3 with label1.' + ); + expect(setActiveDropTarget).toBeCalledWith(undefined); + expect(onDrop).toBeCalledWith( + { humanData: { label: 'label1', position: 1 }, id: '1' }, + 'move_compatible' + ); + }); + + test('Keyboard navigation: dragstart sets dragging in the context and calls it with proper params', async () => { + const setDragging = jest.fn(); + + const setA11yMessage = jest.fn(); + const component = mount( + + + + + + ); + + const keyboardHandler = component + .find('[data-test-subj="lnsDragDrop-keyboardHandler"]') + .first() + .simulate('focus'); + + keyboardHandler.simulate('keydown', { key: 'Enter' }); + jest.runAllTimers(); + + expect(setDragging).toBeCalledWith({ + ...value, + ghost: { + children: , + style: { + height: 0, + width: 0, + }, + }, + }); + expect(setA11yMessage).toBeCalledWith('Lifted hello'); + }); + + test('Keyboard navigation: ActiveDropTarget gets ghost image', () => { + const onDrop = jest.fn(); + const setActiveDropTarget = jest.fn(); + const setA11yMessage = jest.fn(); + const items = [ + { + draggable: true, + value: { + id: '1', + humanData: { label: 'label1', position: 1 }, + }, + children: '1', + order: [2, 0, 0, 0], + }, + { + draggable: true, + dragType: 'move' as 'copy' | 'move', + + value: { + id: '2', + + humanData: { label: 'label2', position: 1 }, + }, + onDrop, + dropType: 'move_compatible' as DropType, + order: [2, 0, 1, 0], + }, + ]; + const component = mount( + Hello
, style: {} } }, + setActiveDropTarget, + setA11yMessage, + activeDropTarget: { + activeDropTarget: { ...items[1].value, onDrop, dropType: 'move_compatible' }, + dropTargetsByOrder: { + '2,0,1,0': { ...items[1].value, onDrop, dropType: 'move_compatible' }, + }, + }, + keyboardMode: true, + }} + > + {items.map((props) => ( + +
+ + ))} + + ); + + expect(component.find(DragDrop).at(1).find('.lnsDragDrop_ghost').text()).toEqual('Hello'); }); describe('reordering', () => { @@ -427,7 +517,7 @@ describe('DragDrop', () => { const registerDropTarget = jest.fn(); const baseContext = { dragging, - setDragging: (val?: DragDropIdentifier) => { + setDragging: (val?: DraggingIdentifier) => { dragging = val; }, keyboardMode, @@ -479,7 +569,11 @@ describe('DragDrop', () => { test(`Reorderable group with lifted element renders properly`, () => { const setA11yMessage = jest.fn(); const setDragging = jest.fn(); - const component = mountComponent({ dragging: items[0], setDragging, setA11yMessage }); + const component = mountComponent({ + dragging: { ...items[0] }, + setDragging, + setA11yMessage, + }); act(() => { component .find('[data-test-subj="lnsDragDrop"]') @@ -488,7 +582,7 @@ describe('DragDrop', () => { jest.runAllTimers(); }); - expect(setDragging).toBeCalledWith(items[0]); + expect(setDragging).toBeCalledWith({ ...items[0] }); expect(setA11yMessage).toBeCalledWith('Lifted label1'); expect( component @@ -498,7 +592,7 @@ describe('DragDrop', () => { }); test(`Reordered elements get extra styles to show the reorder effect when dragging`, () => { - const component = mountComponent({ dragging: items[0] }); + const component = mountComponent({ dragging: { ...items[0] } }); act(() => { component @@ -545,7 +639,11 @@ describe('DragDrop', () => { const setA11yMessage = jest.fn(); const setDragging = jest.fn(); - const component = mountComponent({ dragging: items[0], setDragging, setA11yMessage }); + const component = mountComponent({ + dragging: { ...items[0] }, + setDragging, + setA11yMessage, + }); component .find('[data-test-subj="lnsDragDrop-reorderableDropLayer"]') @@ -558,14 +656,14 @@ describe('DragDrop', () => { ); expect(preventDefault).toBeCalled(); expect(stopPropagation).toBeCalled(); - expect(onDrop).toBeCalledWith(items[0], 'reorder'); + expect(onDrop).toBeCalledWith({ ...items[0] }, 'reorder'); }); test(`Keyboard Navigation: User cannot move an element outside of the group`, () => { const setA11yMessage = jest.fn(); const setActiveDropTarget = jest.fn(); const component = mountComponent({ - dragging: items[0], + dragging: { ...items[0] }, keyboardMode: true, activeDropTarget: { activeDropTarget: undefined, @@ -594,7 +692,7 @@ describe('DragDrop', () => { }); test(`Keyboard navigation: user can drop element to an activeDropTarget`, () => { const component = mountComponent({ - dragging: items[0], + dragging: { ...items[0] }, activeDropTarget: { activeDropTarget: { ...items[2], dropType: 'reorder', onDrop }, dropTargetsByOrder: { @@ -621,7 +719,10 @@ describe('DragDrop', () => { test(`Keyboard Navigation: Doesn't call onDrop when movement is cancelled`, () => { const setA11yMessage = jest.fn(); const onDropHandler = jest.fn(); - const component = mountComponent({ dragging: items[0], setA11yMessage }, onDropHandler); + const component = mountComponent( + { dragging: { ...items[0] }, setA11yMessage }, + onDropHandler + ); const keyboardHandler = component.find('[data-test-subj="lnsDragDrop-keyboardHandler"]'); keyboardHandler.simulate('keydown', { key: 'Space' }); keyboardHandler.simulate('keydown', { key: 'Escape' }); @@ -640,7 +741,7 @@ describe('DragDrop', () => { test(`Keyboard Navigation: Reordered elements get extra styles to show the reorder effect`, () => { const setA11yMessage = jest.fn(); const component = mountComponent({ - dragging: items[0], + dragging: { ...items[0] }, keyboardMode: true, activeDropTarget: { activeDropTarget: undefined, @@ -704,7 +805,7 @@ describe('DragDrop', () => { '2,0,1,1': { ...items[1], onDrop, dropType: 'reorder' }, }, }} - dragging={items[0]} + dragging={{ ...items[0] }} setActiveDropTarget={setActiveDropTarget} setA11yMessage={setA11yMessage} > diff --git a/x-pack/plugins/lens/public/drag_drop/drag_drop.tsx b/x-pack/plugins/lens/public/drag_drop/drag_drop.tsx index 07c1368e53456..4b25064320327 100644 --- a/x-pack/plugins/lens/public/drag_drop/drag_drop.tsx +++ b/x-pack/plugins/lens/public/drag_drop/drag_drop.tsx @@ -177,6 +177,7 @@ export const DragDrop = (props: BaseProps) => { ); const dropProps = { ...props, + keyboardMode, setKeyboardMode, dragging, setDragging, @@ -219,7 +220,10 @@ const DragInner = memo(function DragInner({ ariaDescribedBy, setA11yMessage, }: DragInnerProps) { - const dragStart = (e?: DroppableEvent | React.KeyboardEvent) => { + const dragStart = ( + e: DroppableEvent | React.KeyboardEvent, + keyboardModeOn?: boolean + ) => { // Setting stopPropgagation causes Chrome failures, so // we are manually checking if we've already handled this // in a nested child, and doing nothing if so... @@ -237,9 +241,21 @@ const DragInner = memo(function DragInner({ // dragStart event, so we drop a setTimeout to avoid that. const currentTarget = e?.currentTarget; + setTimeout(() => { - setDragging(value); + setDragging({ + ...value, + ghost: keyboardModeOn + ? { + children, + style: { width: currentTarget.offsetWidth, height: currentTarget.offsetHeight }, + } + : undefined, + }); setA11yMessage(announce.lifted(value.humanData)); + if (keyboardModeOn) { + setKeyboardMode(true); + } if (onDragStart) { onDragStart(currentTarget); } @@ -284,8 +300,19 @@ const DragInner = memo(function DragInner({ : announce.noTarget() ); }; + const shouldShowGhostImageInstead = + isDragging && + dragType === 'move' && + keyboardMode && + activeDropTarget?.activeDropTarget && + activeDropTarget?.activeDropTarget.dropType !== 'reorder'; return ( -
+
, + style: {}, + }, }; const component = mountWithIntl( @@ -463,7 +467,7 @@ describe('LayerPanel', () => { }) ); - component.find('[data-test-subj="lnsGroup"] DragDrop').first().simulate('drop'); + component.find('[data-test-subj="lnsGroup"] DragDrop .lnsDragDrop').first().simulate('drop'); expect(mockDatasource.onDrop).toHaveBeenCalledWith( expect.objectContaining({ @@ -497,6 +501,10 @@ describe('LayerPanel', () => { indexPatternId: 'a', id: '1', humanData: { label: 'Label' }, + ghost: { + children: , + style: {}, + }, }; const component = mountWithIntl( @@ -554,6 +562,10 @@ describe('LayerPanel', () => { groupId: 'a', id: 'a', humanData: { label: 'Label' }, + ghost: { + children: , + style: {}, + }, }; const component = mountWithIntl( @@ -571,7 +583,7 @@ describe('LayerPanel', () => { ); // Simulate drop on the pre-populated dimension - component.find('[data-test-subj="lnsGroupB"] DragDrop').at(0).simulate('drop'); + component.find('[data-test-subj="lnsGroupB"] DragDrop .lnsDragDrop').at(0).simulate('drop'); expect(mockDatasource.onDrop).toHaveBeenCalledWith( expect.objectContaining({ columnId: 'b', @@ -582,7 +594,7 @@ describe('LayerPanel', () => { ); // Simulate drop on the empty dimension - component.find('[data-test-subj="lnsGroupB"] DragDrop').at(1).simulate('drop'); + component.find('[data-test-subj="lnsGroupB"] DragDrop .lnsDragDrop').at(1).simulate('drop'); expect(mockDatasource.onDrop).toHaveBeenCalledWith( expect.objectContaining({ columnId: 'newid', @@ -613,6 +625,10 @@ describe('LayerPanel', () => { groupId: 'a', id: 'a', humanData: { label: 'Label' }, + ghost: { + children: , + style: {}, + }, }; const component = mountWithIntl( @@ -659,6 +675,10 @@ describe('LayerPanel', () => { groupId: 'a', id: 'a', humanData: { label: 'Label' }, + ghost: { + children: , + style: {}, + }, }; const component = mountWithIntl( diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx index da1d7f6eacd02..108e4aa84418f 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/editor_frame.test.tsx @@ -1323,7 +1323,10 @@ describe('editor_frame', () => { getDatasourceSuggestionsForVisualizeField: () => [generateSuggestion()], renderDataPanel: (_element, { dragDropContext: { setDragging, dragging } }) => { if (!dragging || dragging.id !== 'draggedField') { - setDragging({ id: 'draggedField', humanData: { label: 'draggedField' } }); + setDragging({ + id: 'draggedField', + humanData: { label: 'draggedField' }, + }); } }, }, @@ -1425,7 +1428,10 @@ describe('editor_frame', () => { getDatasourceSuggestionsForVisualizeField: () => [generateSuggestion()], renderDataPanel: (_element, { dragDropContext: { setDragging, dragging } }) => { if (!dragging || dragging.id !== 'draggedField') { - setDragging({ id: 'draggedField', humanData: { label: '1' } }); + setDragging({ + id: 'draggedField', + humanData: { label: '1' }, + }); } }, }, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss index ae9294c474b42..0ace88b3d3ab7 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel_wrapper.scss @@ -2,7 +2,6 @@ .lnsWorkspacePanelWrapper { @include euiScrollBar; - overflow: hidden; // Override panel size padding padding: 0 !important; // sass-lint:disable-line no-important margin-bottom: $euiSize; @@ -10,6 +9,7 @@ flex-direction: column; position: relative; // For positioning the dnd overlay min-height: $euiSizeXXL * 10; + overflow: visible; .lnsWorkspacePanelWrapper__pageContentBody { @include euiScrollBar; @@ -17,7 +17,6 @@ display: flex; align-items: stretch; justify-content: stretch; - overflow: auto; > * { flex: 1 1 100%; @@ -34,6 +33,8 @@ // Color the whole panel instead background-color: transparent !important; // sass-lint:disable-line no-important border: none !important; // sass-lint:disable-line no-important + width: 100%; + height: 100%; } .lnsExpressionRenderer { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts index b374be98748f0..1f0381d92ce64 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts @@ -4,7 +4,6 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; import { IndexPatternDimensionEditorProps } from './dimension_panel'; import { onDrop, getDropTypes } from './droppable'; @@ -187,7 +186,11 @@ describe('IndexPatternDimensionEditorPanel', () => { groupId, dragDropContext: { ...dragDropContext, - dragging: { name: 'bar', id: 'bar', humanData: { label: 'Label' } }, + dragging: { + name: 'bar', + id: 'bar', + humanData: { label: 'Label' }, + }, }, }) ).toBe(undefined); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/field_item.scss b/x-pack/plugins/lens/public/indexpattern_datasource/field_item.scss index 8a6e10c8be6e4..19f5b91975202 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/field_item.scss +++ b/x-pack/plugins/lens/public/indexpattern_datasource/field_item.scss @@ -29,6 +29,13 @@ } } +.kbnFieldButton.lnsDragDrop_ghost { + .lnsFieldItem__infoIcon { + visibility: hidden; + opacity: 0; + } +} + .kbnFieldButton__name { transition: background-color $euiAnimSpeedFast ease-in-out; } From e9e7453e1dd6785cebf53fd344830f73dadab42c Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Mon, 15 Feb 2021 13:00:31 +0100 Subject: [PATCH 04/19] [Lens] Improves error messages when in Dashboard (#90668) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../visualization.test.tsx | 16 +- .../datatable_visualization/visualization.tsx | 2 +- .../editor_frame/state_helpers.ts | 55 +++- .../workspace_panel/workspace_panel.tsx | 5 +- .../embeddable/embeddable.test.tsx | 173 ++++++---- .../embeddable/embeddable.tsx | 12 +- .../embeddable/embeddable_factory.ts | 5 +- .../embeddable/expression_wrapper.tsx | 61 +++- .../editor_frame_service/error_helper.ts | 12 + .../public/editor_frame_service/mocks.tsx | 2 +- .../public/editor_frame_service/service.tsx | 6 +- .../lens/public/editor_frame_service/types.ts | 5 + .../visualization.test.ts | 18 +- .../metric_visualization/visualization.tsx | 2 +- .../lens/public/pie_visualization/toolbar.tsx | 9 +- .../pie_visualization/visualization.test.ts | 30 +- .../pie_visualization/visualization.tsx | 4 +- x-pack/plugins/lens/public/types.ts | 7 +- .../lens/public/visualization_container.scss | 8 + .../xy_visualization/color_assignment.ts | 2 +- .../xy_visualization/visualization.test.ts | 308 ++++++++---------- .../public/xy_visualization/visualization.tsx | 2 +- .../xy_visualization/xy_config_panel.tsx | 2 +- x-pack/test/functional/apps/lens/dashboard.ts | 34 ++ 24 files changed, 447 insertions(+), 333 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx index 2a6228f16867d..ad5e1e552ccd2 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.test.tsx @@ -459,10 +459,10 @@ describe('Datatable Visualization', () => { label: 'label', }); - const error = datatableVisualization.getErrorMessages( - { layerId: 'a', columns: [{ columnId: 'b' }, { columnId: 'c' }] }, - frame - ); + const error = datatableVisualization.getErrorMessages({ + layerId: 'a', + columns: [{ columnId: 'b' }, { columnId: 'c' }], + }); expect(error).toBeUndefined(); }); @@ -478,10 +478,10 @@ describe('Datatable Visualization', () => { label: 'label', }); - const error = datatableVisualization.getErrorMessages( - { layerId: 'a', columns: [{ columnId: 'b' }, { columnId: 'c' }] }, - frame - ); + const error = datatableVisualization.getErrorMessages({ + layerId: 'a', + columns: [{ columnId: 'b' }, { columnId: 'c' }], + }); expect(error).toBeUndefined(); }); diff --git a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx index 9625a814c7958..47f8ce09aea68 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/visualization.tsx @@ -276,7 +276,7 @@ export const datatableVisualization: Visualization }; }, - getErrorMessages(state, frame) { + getErrorMessages(state) { return undefined; }, diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts index 559e773dbc167..9c7ef19132c46 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/state_helpers.ts @@ -18,6 +18,9 @@ import { import { buildExpression } from './expression_helpers'; import { Document } from '../../persistence/saved_object_store'; import { VisualizeFieldContext } from '../../../../../../src/plugins/ui_actions/public'; +import { getActiveDatasourceIdFromDoc } from './state_management'; +import { ErrorMessage } from '../types'; +import { getMissingCurrentDatasource, getMissingVisualizationTypeError } from '../error_helper'; export async function initializeDatasources( datasourceMap: Record, @@ -72,7 +75,7 @@ export async function persistedStateToExpression( datasources: Record, visualizations: Record, doc: Document -): Promise { +): Promise<{ ast: Ast | null; errors: ErrorMessage[] | undefined }> { const { state: { visualization: visualizationState, datasourceStates: persistedDatasourceStates }, visualizationType, @@ -80,7 +83,12 @@ export async function persistedStateToExpression( title, description, } = doc; - if (!visualizationType) return null; + if (!visualizationType) { + return { + ast: null, + errors: [{ shortMessage: '', longMessage: getMissingVisualizationTypeError() }], + }; + } const visualization = visualizations[visualizationType!]; const datasourceStates = await initializeDatasources( datasources, @@ -97,15 +105,33 @@ export async function persistedStateToExpression( const datasourceLayers = createDatasourceLayers(datasources, datasourceStates); - return buildExpression({ - title, - description, + const datasourceId = getActiveDatasourceIdFromDoc(doc); + if (datasourceId == null) { + return { + ast: null, + errors: [{ shortMessage: '', longMessage: getMissingCurrentDatasource() }], + }; + } + const validationResult = validateDatasourceAndVisualization( + datasources[datasourceId], + datasourceStates[datasourceId].state, visualization, visualizationState, - datasourceMap: datasources, - datasourceStates, - datasourceLayers, - }); + { datasourceLayers } + ); + + return { + ast: buildExpression({ + title, + description, + visualization, + visualizationState, + datasourceMap: datasources, + datasourceStates, + datasourceLayers, + }), + errors: validationResult, + }; } export const validateDatasourceAndVisualization = ( @@ -113,13 +139,8 @@ export const validateDatasourceAndVisualization = ( currentDatasourceState: unknown | null, currentVisualization: Visualization | null, currentVisualizationState: unknown | undefined, - frameAPI: FramePublicAPI -): - | Array<{ - shortMessage: string; - longMessage: string; - }> - | undefined => { + frameAPI: Pick +): ErrorMessage[] | undefined => { const layersGroups = currentVisualizationState ? currentVisualization ?.getLayerIds(currentVisualizationState) @@ -141,7 +162,7 @@ export const validateDatasourceAndVisualization = ( : undefined; const visualizationValidationErrors = currentVisualizationState - ? currentVisualization?.getErrorMessages(currentVisualizationState, frameAPI) + ? currentVisualization?.getErrorMessages(currentVisualizationState) : undefined; if (datasourceValidationErrors?.length || visualizationValidationErrors?.length) { diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index 2c4cecd356ced..219c0638f9b56 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -155,10 +155,7 @@ export const WorkspacePanel = React.memo(function WorkspacePanel({ datasourceLayers: framePublicAPI.datasourceLayers, }); } catch (e) { - const buildMessages = activeVisualization?.getErrorMessages( - visualizationState, - framePublicAPI - ); + const buildMessages = activeVisualization?.getErrorMessages(visualizationState); const defaultMessage = { shortMessage: i18n.translate('xpack.lens.editorFrame.buildExpressionError', { defaultMessage: 'An unexpected error occurred while preparing the chart', diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx index d2085a4cc8a8b..227c8b4741501 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.test.tsx @@ -116,11 +116,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { @@ -140,6 +143,36 @@ describe('embeddable', () => { | expression`); }); + it('should not render the visualization if any error arises', async () => { + const embeddable = new Embeddable( + { + timefilter: dataPluginMock.createSetupContract().query.timefilter.timefilter, + attributeService, + expressionRenderer, + basePath, + indexPatternService: {} as IndexPatternsContract, + editable: true, + getTrigger, + documentToExpression: () => + Promise.resolve({ + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: [{ shortMessage: '', longMessage: 'my validation error' }], + }), + }, + {} as LensEmbeddableInput + ); + await embeddable.initializeSavedVis({} as LensEmbeddableInput); + embeddable.render(mountpoint); + + expect(expressionRenderer).toHaveBeenCalledTimes(0); + }); + it('should initialize output with deduped list of index patterns', async () => { attributeService = attributeServiceMockFromSavedVis({ ...savedVis, @@ -162,11 +195,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, {} as LensEmbeddableInput @@ -194,11 +230,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123' } as LensEmbeddableInput @@ -232,11 +271,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123' } as LensEmbeddableInput @@ -265,11 +307,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123' } as LensEmbeddableInput @@ -312,11 +357,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, input @@ -359,11 +407,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, input @@ -405,11 +456,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, input @@ -440,11 +494,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123' } as LensEmbeddableInput @@ -475,11 +532,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123' } as LensEmbeddableInput @@ -510,11 +570,14 @@ describe('embeddable', () => { getTrigger, documentToExpression: () => Promise.resolve({ - type: 'expression', - chain: [ - { type: 'function', function: 'my', arguments: {} }, - { type: 'function', function: 'expression', arguments: {} }, - ], + ast: { + type: 'expression', + chain: [ + { type: 'function', function: 'my', arguments: {} }, + { type: 'function', function: 'expression', arguments: {} }, + ], + }, + errors: undefined, }), }, { id: '123', timeRange, query, filters } as LensEmbeddableInput diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx index dc5f9b366e6b5..ef265881f6eb3 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx @@ -51,6 +51,7 @@ import { IndexPatternsContract } from '../../../../../../src/plugins/data/public import { getEditPath, DOC_TYPE } from '../../../common'; import { IBasePath } from '../../../../../../src/core/public'; import { LensAttributeService } from '../../lens_attribute_service'; +import type { ErrorMessage } from '../types'; export type LensSavedObjectAttributes = Omit; @@ -77,7 +78,9 @@ export interface LensEmbeddableOutput extends EmbeddableOutput { export interface LensEmbeddableDeps { attributeService: LensAttributeService; - documentToExpression: (doc: Document) => Promise; + documentToExpression: ( + doc: Document + ) => Promise<{ ast: Ast | null; errors: ErrorMessage[] | undefined }>; editable: boolean; indexPatternService: IndexPatternsContract; expressionRenderer: ReactExpressionRendererType; @@ -99,6 +102,7 @@ export class Embeddable private subscription: Subscription; private isInitialized = false; private activeData: Partial | undefined; + private errors: ErrorMessage[] | undefined; private externalSearchContext: { timeRange?: TimeRange; @@ -225,8 +229,9 @@ export class Embeddable type: this.type, savedObjectId: (input as LensByReferenceInput)?.savedObjectId, }; - const expression = await this.deps.documentToExpression(this.savedVis); - this.expression = expression ? toExpression(expression) : null; + const { ast, errors } = await this.deps.documentToExpression(this.savedVis); + this.errors = errors; + this.expression = ast ? toExpression(ast) : null; await this.initializeOutput(); this.isInitialized = true; } @@ -279,6 +284,7 @@ export class Embeddable Promise; + documentToExpression: ( + doc: Document + ) => Promise<{ ast: Ast | null; errors: ErrorMessage[] | undefined }>; } export class EmbeddableFactory implements EmbeddableFactoryDefinition { diff --git a/x-pack/plugins/lens/public/editor_frame_service/embeddable/expression_wrapper.tsx b/x-pack/plugins/lens/public/editor_frame_service/embeddable/expression_wrapper.tsx index 8873388633552..a559e6a02419d 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/embeddable/expression_wrapper.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/embeddable/expression_wrapper.tsx @@ -8,7 +8,7 @@ import React from 'react'; import { I18nProvider } from '@kbn/i18n/react'; import { FormattedMessage } from '@kbn/i18n/react'; -import { EuiFlexGroup, EuiFlexItem, EuiText, EuiIcon } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiText, EuiIcon, EuiEmptyPrompt } from '@elastic/eui'; import { ExpressionRendererEvent, ReactExpressionRendererType, @@ -18,10 +18,12 @@ import { ExecutionContextSearch } from 'src/plugins/data/public'; import { DefaultInspectorAdapters, RenderMode } from 'src/plugins/expressions'; import classNames from 'classnames'; import { getOriginalRequestErrorMessage } from '../error_helper'; +import { ErrorMessage } from '../types'; export interface ExpressionWrapperProps { ExpressionRenderer: ReactExpressionRendererType; expression: string | null; + errors: ErrorMessage[] | undefined; variables?: Record; searchContext: ExecutionContextSearch; searchSessionId?: string; @@ -37,6 +39,46 @@ export interface ExpressionWrapperProps { className?: string; } +interface VisualizationErrorProps { + errors: ExpressionWrapperProps['errors']; +} + +export function VisualizationErrorPanel({ errors }: VisualizationErrorProps) { + return ( +
+ + {errors ? ( + <> +

{errors[0].longMessage}

+ {errors.length > 1 ? ( +

+ +

+ ) : null} + + ) : ( +

+ +

+ )} + + } + /> +
+ ); +} + export function ExpressionWrapper({ ExpressionRenderer: ExpressionRendererComponent, expression, @@ -50,23 +92,12 @@ export function ExpressionWrapper({ hasCompatibleActions, style, className, + errors, }: ExpressionWrapperProps) { return ( - {expression === null || expression === '' ? ( - - - - - - - - - - + {errors || expression === null || expression === '' ? ( + ) : (
{ setDimension: jest.fn(), removeDimension: jest.fn(), - getErrorMessages: jest.fn((_state, _frame) => undefined), + getErrorMessages: jest.fn((_state) => undefined), }; } diff --git a/x-pack/plugins/lens/public/editor_frame_service/service.tsx b/x-pack/plugins/lens/public/editor_frame_service/service.tsx index 9e54a4d630dc2..8769aceca3bfd 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/service.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/service.tsx @@ -72,7 +72,7 @@ export class EditorFrameService { * This is an asynchronous process and should only be triggered once for a saved object. * @param doc parsed Lens saved object */ - private async documentToExpression(doc: Document) { + private documentToExpression = async (doc: Document) => { const [resolvedDatasources, resolvedVisualizations] = await Promise.all([ collectAsyncDefinitions(this.datasources), collectAsyncDefinitions(this.visualizations), @@ -81,7 +81,7 @@ export class EditorFrameService { const { persistedStateToExpression } = await import('../async_services'); return await persistedStateToExpression(resolvedDatasources, resolvedVisualizations, doc); - } + }; public setup( core: CoreSetup, @@ -98,7 +98,7 @@ export class EditorFrameService { coreHttp: coreStart.http, timefilter: deps.data.query.timefilter.timefilter, expressionRenderer: deps.expressions.ReactExpressionRenderer, - documentToExpression: this.documentToExpression.bind(this), + documentToExpression: this.documentToExpression, indexPatternService: deps.data.indexPatterns, uiActions: deps.uiActions, }; diff --git a/x-pack/plugins/lens/public/editor_frame_service/types.ts b/x-pack/plugins/lens/public/editor_frame_service/types.ts index dc5a4aa0e234b..6043e96343899 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/types.ts +++ b/x-pack/plugins/lens/public/editor_frame_service/types.ts @@ -8,3 +8,8 @@ import { Datatable } from 'src/plugins/expressions'; export type TableInspectorAdapter = Record; + +export interface ErrorMessage { + shortMessage: string; + longMessage: string; +} diff --git a/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts b/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts index 84abc38bf4106..66e524435ebc8 100644 --- a/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/metric_visualization/visualization.test.ts @@ -197,23 +197,7 @@ describe('metric_visualization', () => { describe('#getErrorMessages', () => { it('returns undefined if no error is raised', () => { - const datasource: DatasourcePublicAPI = { - ...createMockDatasource('l1').publicAPIMock, - getOperationForColumnId(_: string) { - return { - id: 'a', - dataType: 'number', - isBucketed: false, - label: 'shazm', - }; - }, - }; - const frame = { - ...mockFrame(), - datasourceLayers: { l1: datasource }, - }; - - const error = metricVisualization.getErrorMessages(exampleState(), frame); + const error = metricVisualization.getErrorMessages(exampleState()); expect(error).not.toBeDefined(); }); diff --git a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx index b86ba71083440..91516b7b7319b 100644 --- a/x-pack/plugins/lens/public/metric_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/metric_visualization/visualization.tsx @@ -117,7 +117,7 @@ export const metricVisualization: Visualization = { return { ...prevState, accessor: undefined }; }, - getErrorMessages(state, frame) { + getErrorMessages(state) { // Is it possible to break it? return undefined; }, diff --git a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx index 5ec97e90e57d9..e3bd54032a93c 100644 --- a/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/toolbar.tsx @@ -17,6 +17,7 @@ import { EuiHorizontalRule, } from '@elastic/eui'; import { Position } from '@elastic/charts'; +import { PaletteRegistry } from 'src/plugins/charts/public'; import { DEFAULT_PERCENT_DECIMALS } from './constants'; import { PieVisualizationState, SharedPieLayerState } from './types'; import { VisualizationDimensionEditorProps, VisualizationToolbarProps } from '../types'; @@ -250,11 +251,15 @@ const DecimalPlaceSlider = ({ ); }; -export function DimensionEditor(props: VisualizationDimensionEditorProps) { +export function DimensionEditor( + props: VisualizationDimensionEditorProps & { + paletteService: PaletteRegistry; + } +) { return ( <> { props.setState({ ...props.state, palette: newPalette }); diff --git a/x-pack/plugins/lens/public/pie_visualization/visualization.test.ts b/x-pack/plugins/lens/public/pie_visualization/visualization.test.ts index 52fd4daac63c5..0cdeaa8c043d8 100644 --- a/x-pack/plugins/lens/public/pie_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/pie_visualization/visualization.test.ts @@ -7,8 +7,6 @@ import { getPieVisualization } from './visualization'; import { PieVisualizationState } from './types'; -import { createMockDatasource, createMockFramePublicAPI } from '../editor_frame_service/mocks'; -import { DatasourcePublicAPI, FramePublicAPI } from '../types'; import { chartPluginMock } from '../../../../../src/plugins/charts/public/mocks'; jest.mock('../id_generator'); @@ -36,37 +34,11 @@ function exampleState(): PieVisualizationState { }; } -function mockFrame(): FramePublicAPI { - return { - ...createMockFramePublicAPI(), - addNewLayer: () => LAYER_ID, - datasourceLayers: { - [LAYER_ID]: createMockDatasource(LAYER_ID).publicAPIMock, - }, - }; -} - // Just a basic bootstrap here to kickstart the tests describe('pie_visualization', () => { describe('#getErrorMessages', () => { it('returns undefined if no error is raised', () => { - const datasource: DatasourcePublicAPI = { - ...createMockDatasource('l1').publicAPIMock, - getOperationForColumnId(_: string) { - return { - id: 'a', - dataType: 'number', - isBucketed: false, - label: 'shazm', - }; - }, - }; - const frame = { - ...mockFrame(), - datasourceLayers: { l1: datasource }, - }; - - const error = pieVisualization.getErrorMessages(exampleState(), frame); + const error = pieVisualization.getErrorMessages(exampleState()); expect(error).not.toBeDefined(); }); diff --git a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx index 6408d7496d332..683acc49859b6 100644 --- a/x-pack/plugins/lens/public/pie_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/pie_visualization/visualization.tsx @@ -227,7 +227,7 @@ export const getPieVisualization = ({ renderDimensionEditor(domElement, props) { render( - + , domElement ); @@ -274,7 +274,7 @@ export const getPieVisualization = ({ )); }, - getErrorMessages(state, frame) { + getErrorMessages(state) { // not possible to break it? return undefined; }, diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index cccc35acb3fca..ba02a3376bae7 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -358,7 +358,7 @@ export interface LensMultiTable { export interface VisualizationConfigProps { layerId: string; - frame: FramePublicAPI; + frame: Pick; state: T; } @@ -631,10 +631,7 @@ export interface Visualization { * The frame will call this function on all visualizations at few stages (pre-build/build error) in order * to provide more context to the error and show it to the user */ - getErrorMessages: ( - state: T, - frame: FramePublicAPI - ) => Array<{ shortMessage: string; longMessage: string }> | undefined; + getErrorMessages: (state: T) => Array<{ shortMessage: string; longMessage: string }> | undefined; /** * The frame calls this function to display warnings about visualization diff --git a/x-pack/plugins/lens/public/visualization_container.scss b/x-pack/plugins/lens/public/visualization_container.scss index a67aa50127c81..d40a0b48ab40e 100644 --- a/x-pack/plugins/lens/public/visualization_container.scss +++ b/x-pack/plugins/lens/public/visualization_container.scss @@ -15,3 +15,11 @@ position: static; // Let the progress indicator position itself against the outer parent } } + +.lnsEmbeddedError { + flex-grow: 1; + display: flex; + align-items: center; + justify-content: center; + overflow: auto; +} diff --git a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts index 27cc16ebf862b..d2e87ece5b5ec 100644 --- a/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts +++ b/x-pack/plugins/lens/public/xy_visualization/color_assignment.ts @@ -95,7 +95,7 @@ export function getColorAssignments( export function getAccessorColorConfig( colorAssignments: ColorAssignments, - frame: FramePublicAPI, + frame: Pick, layer: XYLayerConfig, paletteService: PaletteRegistry ): AccessorConfig[] { diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts index cdb7f452cf7cf..c244fa7fdfc89 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.test.ts @@ -589,137 +589,119 @@ describe('xy_visualization', () => { describe('#getErrorMessages', () => { it("should not return an error when there's only one dimension (X or Y)", () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: [], - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: [], + }, + ], + }) ).not.toBeDefined(); }); it("should not return an error when there's only one dimension on multiple layers (same axis everywhere)", () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: [], - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: 'a', - accessors: [], - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: [], + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: 'a', + accessors: [], + }, + ], + }) ).not.toBeDefined(); }); it('should not return an error when mixing different valid configurations in multiple layers', () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: ['a'], - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: undefined, - accessors: ['a'], - splitAccessor: 'a', - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: ['a'], + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: undefined, + accessors: ['a'], + splitAccessor: 'a', + }, + ], + }) ).not.toBeDefined(); }); it("should not return an error when there's only one splitAccessor dimension configured", () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: undefined, - accessors: [], - splitAccessor: 'a', - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: undefined, + accessors: [], + splitAccessor: 'a', + }, + ], + }) ).not.toBeDefined(); expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: undefined, - accessors: [], - splitAccessor: 'a', - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: undefined, - accessors: [], - splitAccessor: 'a', - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: undefined, + accessors: [], + splitAccessor: 'a', + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: undefined, + accessors: [], + splitAccessor: 'a', + }, + ], + }) ).not.toBeDefined(); }); it('should return an error when there are multiple layers, one axis configured for each layer (but different axis from each other)', () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: [], - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: undefined, - accessors: ['a'], - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: [], + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: undefined, + accessors: ['a'], + }, + ], + }) ).toEqual([ { shortMessage: 'Missing Vertical axis.', @@ -729,34 +711,31 @@ describe('xy_visualization', () => { }); it('should return an error with batched messages for the same error with multiple layers', () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: ['a'], - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: undefined, - accessors: [], - splitAccessor: 'a', - }, - { - layerId: 'third', - seriesType: 'area', - xAccessor: undefined, - accessors: [], - splitAccessor: 'a', - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: ['a'], + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: undefined, + accessors: [], + splitAccessor: 'a', + }, + { + layerId: 'third', + seriesType: 'area', + xAccessor: undefined, + accessors: [], + splitAccessor: 'a', + }, + ], + }) ).toEqual([ { shortMessage: 'Missing Vertical axis.', @@ -766,32 +745,29 @@ describe('xy_visualization', () => { }); it("should return an error when some layers are complete but other layers aren't", () => { expect( - xyVisualization.getErrorMessages( - { - ...exampleState(), - layers: [ - { - layerId: 'first', - seriesType: 'area', - xAccessor: 'a', - accessors: [], - }, - { - layerId: 'second', - seriesType: 'area', - xAccessor: 'a', - accessors: ['a'], - }, - { - layerId: 'third', - seriesType: 'area', - xAccessor: 'a', - accessors: ['a'], - }, - ], - }, - createMockFramePublicAPI() - ) + xyVisualization.getErrorMessages({ + ...exampleState(), + layers: [ + { + layerId: 'first', + seriesType: 'area', + xAccessor: 'a', + accessors: [], + }, + { + layerId: 'second', + seriesType: 'area', + xAccessor: 'a', + accessors: ['a'], + }, + { + layerId: 'third', + seriesType: 'area', + xAccessor: 'a', + accessors: ['a'], + }, + ], + }) ).toEqual([ { shortMessage: 'Missing Vertical axis.', diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index a4dc7a91822bd..1ee4b2e050f3e 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -340,7 +340,7 @@ export const getXyVisualization = ({ toExpression(state, layers, paletteService, attributes), toPreviewExpression: (state, layers) => toPreviewExpression(state, layers, paletteService), - getErrorMessages(state, frame) { + getErrorMessages(state) { // Data error handling below here const hasNoAccessors = ({ accessors }: XYLayerConfig) => accessors == null || accessors.length === 0; diff --git a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx index dd5fff3c49f4f..14733ad0f613b 100644 --- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel.tsx @@ -508,7 +508,7 @@ export function DimensionEditor( return ( <> { setState(updateLayer(state, { ...layer, palette: newPalette }, index)); diff --git a/x-pack/test/functional/apps/lens/dashboard.ts b/x-pack/test/functional/apps/lens/dashboard.ts index 5cbd5dff45e1e..0d2db53ba73af 100644 --- a/x-pack/test/functional/apps/lens/dashboard.ts +++ b/x-pack/test/functional/apps/lens/dashboard.ts @@ -194,5 +194,39 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await dashboardAddPanel.filterEmbeddableNames('lnsPieVis'); await find.existsByLinkText('lnsPieVis'); }); + + it('should show validation messages if any error appears', async () => { + await PageObjects.common.navigateToApp('dashboard'); + await PageObjects.dashboard.clickNewDashboard(); + + await dashboardAddPanel.clickCreateNewLink(); + await dashboardAddPanel.clickVisType('lens'); + await PageObjects.header.waitUntilLoadingHasFinished(); + await PageObjects.lens.goToTimeRange(); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension', + operation: 'date_histogram', + field: '@timestamp', + }); + + await PageObjects.lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'moving_average', + keepOpen: true, + }); + await PageObjects.lens.configureReference({ + operation: 'sum', + field: 'bytes', + }); + await PageObjects.lens.closeDimensionEditor(); + + // remove the x dimension to trigger the validation error + await PageObjects.lens.removeDimension('lnsXY_xDimensionPanel'); + await PageObjects.lens.saveAndReturn(); + + await PageObjects.header.waitUntilLoadingHasFinished(); + await testSubjects.existOrFail('embeddable-lens-failure'); + }); }); } From 2f7689882fa43c0b25ff850ad54a1f4556b3aacb Mon Sep 17 00:00:00 2001 From: Marco Liberati Date: Mon, 15 Feb 2021 13:01:23 +0100 Subject: [PATCH 05/19] [Lens] Fix empty display name issue in XY chart (#91132) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../dimension_panel/dimension_editor.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 6dffeb351d260..8f5da64fcc9a8 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -59,6 +59,9 @@ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: stri const [inputValue, setInputValue] = useState(value); const unflushedChanges = useRef(false); + // Save the initial value + const initialValue = useRef(value); + const onChangeDebounced = useMemo(() => { const callback = _.debounce((val: string) => { onChange(val); @@ -79,7 +82,7 @@ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: stri const handleInputChange = (e: React.ChangeEvent) => { const val = String(e.target.value); setInputValue(val); - onChangeDebounced(val); + onChangeDebounced(val || initialValue.current); }; return ( @@ -96,6 +99,7 @@ const LabelInput = ({ value, onChange }: { value: string; onChange: (value: stri data-test-subj="indexPattern-label-edit" value={inputValue} onChange={handleInputChange} + placeholder={initialValue.current} /> ); From bac1d13df664ab988f7d08a1d1c77c9ac52922f5 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Mon, 15 Feb 2021 13:16:20 +0100 Subject: [PATCH 06/19] [ML] Unskip test. Fix modelMemoryLimit value. (#91280) Unskips test and updates modelMemoryLimit value. --- x-pack/test/api_integration/apis/ml/modules/setup_module.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/test/api_integration/apis/ml/modules/setup_module.ts b/x-pack/test/api_integration/apis/ml/modules/setup_module.ts index c4b3a4ed0adcf..8e5da7c56bb64 100644 --- a/x-pack/test/api_integration/apis/ml/modules/setup_module.ts +++ b/x-pack/test/api_integration/apis/ml/modules/setup_module.ts @@ -277,7 +277,7 @@ export default ({ getService }: FtrProviderContext) => { jobId: 'pf7_log-entry-categories-count', jobState: JOB_STATE.CLOSED, datafeedState: DATAFEED_STATE.STOPPED, - modelMemoryLimit: '26mb', + modelMemoryLimit: '41mb', }, ], searches: [] as string[], @@ -713,8 +713,7 @@ export default ({ getService }: FtrProviderContext) => { return successObjects; } - // blocks ES snapshot promotion: https://github.com/elastic/kibana/issues/91224 - describe.skip('module setup', function () { + describe('module setup', function () { before(async () => { await ml.testResources.setKibanaTimeZoneToUTC(); }); From 5a5b8ad9a89ce39274bc241a47c8fc0692e229cc Mon Sep 17 00:00:00 2001 From: Joe Reuter Date: Mon, 15 Feb 2021 13:17:31 +0100 Subject: [PATCH 07/19] [Lens] Adjust new copy for 7.12 (#90413) --- .../__snapshots__/table_basic.test.tsx.snap | 24 +++++++++---------- .../components/columns.tsx | 4 ++-- .../workspace_panel/workspace_panel.tsx | 21 +--------------- .../calculations/moving_average.tsx | 11 ++++----- .../definitions/calculations/utils.ts | 2 +- .../operations/definitions/date_histogram.tsx | 6 ++--- .../definitions/ranges/range_editor.tsx | 8 +++---- .../xy_visualization/xy_config_panel.tsx | 2 +- .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - .../test/functional/apps/lens/smokescreen.ts | 6 ++--- x-pack/test/functional/apps/lens/table.ts | 6 ++--- .../test/functional/page_objects/lens_page.ts | 2 +- 13 files changed, 34 insertions(+), 60 deletions(-) diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap b/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap index d69af298018e7..158c7fa4aeec3 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap +++ b/x-pack/plugins/lens/public/datatable_visualization/components/__snapshots__/table_basic.test.tsx.snap @@ -107,10 +107,10 @@ exports[`DatatableComponent it renders actions column when there are row actions "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, @@ -144,10 +144,10 @@ exports[`DatatableComponent it renders actions column when there are row actions "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, @@ -181,10 +181,10 @@ exports[`DatatableComponent it renders actions column when there are row actions "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, @@ -329,10 +329,10 @@ exports[`DatatableComponent it renders the title and value 1`] = ` "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, @@ -366,10 +366,10 @@ exports[`DatatableComponent it renders the title and value 1`] = ` "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, @@ -403,10 +403,10 @@ exports[`DatatableComponent it renders the title and value 1`] = ` "showMoveLeft": false, "showMoveRight": false, "showSortAsc": Object { - "label": "Sort asc", + "label": "Sort ascending", }, "showSortDesc": Object { - "label": "Sort desc", + "label": "Sort descending", }, }, "cellActions": undefined, diff --git a/x-pack/plugins/lens/public/datatable_visualization/components/columns.tsx b/x-pack/plugins/lens/public/datatable_visualization/components/columns.tsx index 5ff1e84276ba7..fdb05599c38e9 100644 --- a/x-pack/plugins/lens/public/datatable_visualization/components/columns.tsx +++ b/x-pack/plugins/lens/public/datatable_visualization/components/columns.tsx @@ -152,14 +152,14 @@ export const createGridColumns = ( ? false : { label: i18n.translate('xpack.lens.table.sort.ascLabel', { - defaultMessage: 'Sort asc', + defaultMessage: 'Sort ascending', }), }, showSortDesc: isReadOnly ? false : { label: i18n.translate('xpack.lens.table.sort.descLabel', { - defaultMessage: 'Sort desc', + defaultMessage: 'Sort descending', }), }, additional: isReadOnly diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx index 219c0638f9b56..83d2100a832cf 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/workspace_panel/workspace_panel.tsx @@ -10,16 +10,7 @@ import classNames from 'classnames'; import { FormattedMessage } from '@kbn/i18n/react'; import { Ast } from '@kbn/interpreter/common'; import { i18n } from '@kbn/i18n'; -import { - EuiFlexGroup, - EuiFlexItem, - EuiIcon, - EuiText, - EuiTextColor, - EuiButtonEmpty, - EuiLink, - EuiTitle, -} from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiText, EuiButtonEmpty, EuiLink } from '@elastic/eui'; import { CoreStart, CoreSetup } from 'kibana/public'; import { DataPublicPluginStart, @@ -420,16 +411,6 @@ export const InnerVisualizationWrapper = ({ - - - - - - - {localState.configurationValidationError[0].longMessage} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx index 7fdc58b74e509..bc361973bb62c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/calculations/moving_average.tsx @@ -192,22 +192,21 @@ const MovingAveragePopup = () => {

@@ -227,7 +226,7 @@ const MovingAveragePopup = () => {