From 5d0969feb84e8de667f9a06c5ae6a2767b46f582 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Wed, 26 Feb 2020 17:46:56 -0500 Subject: [PATCH 1/8] in progress. switched out textfield for code editor component --- .../public/components/controls/raw_json.tsx | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 32939c420155f..3fab746fb9034 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -19,7 +19,7 @@ import React, { useEffect } from 'react'; -import { EuiFormRow, EuiIconTip, EuiTextArea } from '@elastic/eui'; +import { EuiFormRow, EuiIconTip, EuiCodeEditor } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -49,10 +49,9 @@ function RawJsonParamEditor({ ); const isValid = isValidJson(value); - const onChange = (ev: React.ChangeEvent) => { - const textValue = ev.target.value; - setValue(textValue); - setValidity(isValidJson(textValue)); + const onChange = (newValue: string) => { + setValue(newValue); + setValidity(isValidJson(newValue)); }; useEffect(() => { @@ -66,15 +65,20 @@ function RawJsonParamEditor({ fullWidth={true} compressed > - ); From 2349141c7e3215a77da94c3f74d6cab40a64c6b0 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 27 Feb 2020 12:10:55 -0500 Subject: [PATCH 2/8] removed legacy import isValidJson in favor of using Ace Editor's default json validation --- src/legacy/core_plugins/data/public/index.ts | 1 - .../data/public/search/aggs/agg_types.ts | 2 +- .../data/public/search/aggs/index.ts | 2 +- .../data/public/search/aggs/utils.test.tsx | 51 ------------------- .../data/public/search/aggs/utils.ts | 31 +---------- .../public/components/controls/raw_json.tsx | 16 +++--- .../public/legacy_imports.ts | 2 +- 7 files changed, 13 insertions(+), 92 deletions(-) delete mode 100644 src/legacy/core_plugins/data/public/search/aggs/utils.test.tsx diff --git a/src/legacy/core_plugins/data/public/index.ts b/src/legacy/core_plugins/data/public/index.ts index 8cde5d0a1fc11..65931064f7735 100644 --- a/src/legacy/core_plugins/data/public/index.ts +++ b/src/legacy/core_plugins/data/public/index.ts @@ -71,7 +71,6 @@ export { isStringType, isType, isValidInterval, - isValidJson, METRIC_TYPES, OptionedParamType, parentPipelineType, diff --git a/src/legacy/core_plugins/data/public/search/aggs/agg_types.ts b/src/legacy/core_plugins/data/public/search/aggs/agg_types.ts index c16eb06eeb116..391edb2a63020 100644 --- a/src/legacy/core_plugins/data/public/search/aggs/agg_types.ts +++ b/src/legacy/core_plugins/data/public/search/aggs/agg_types.ts @@ -108,7 +108,7 @@ export { convertDateRangeToString } from './buckets/date_range'; export { convertIPRangeToString } from './buckets/ip_range'; export { aggTypeFilters, propFilter } from './filter'; export { OptionedParamType } from './param_types/optioned'; -export { isValidJson, isValidInterval } from './utils'; +export { isValidInterval } from './utils'; export { BUCKET_TYPES } from './buckets/bucket_agg_types'; export { METRIC_TYPES } from './metrics/metric_agg_types'; export { ISchemas, Schema, Schemas } from './schemas'; diff --git a/src/legacy/core_plugins/data/public/search/aggs/index.ts b/src/legacy/core_plugins/data/public/search/aggs/index.ts index 0bdb92b8de65e..06ba2b33a0e68 100644 --- a/src/legacy/core_plugins/data/public/search/aggs/index.ts +++ b/src/legacy/core_plugins/data/public/search/aggs/index.ts @@ -46,7 +46,7 @@ export { convertDateRangeToString } from './buckets/date_range'; export { convertIPRangeToString } from './buckets/ip_range'; export { aggTypeFilters, propFilter } from './filter'; export { OptionedParamType } from './param_types/optioned'; -export { isValidJson, isValidInterval } from './utils'; +export { isValidInterval } from './utils'; export { BUCKET_TYPES } from './buckets/bucket_agg_types'; export { METRIC_TYPES } from './metrics/metric_agg_types'; export { ISchemas, Schema, Schemas } from './schemas'; diff --git a/src/legacy/core_plugins/data/public/search/aggs/utils.test.tsx b/src/legacy/core_plugins/data/public/search/aggs/utils.test.tsx deleted file mode 100644 index a3c7f24f3927d..0000000000000 --- a/src/legacy/core_plugins/data/public/search/aggs/utils.test.tsx +++ /dev/null @@ -1,51 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { isValidJson } from './utils'; - -jest.mock('ui/new_platform'); - -const input = { - valid: '{ "test": "json input" }', - invalid: 'strings are not json', -}; - -describe('AggType utils', () => { - describe('isValidJson', () => { - it('should return true when empty string', () => { - expect(isValidJson('')).toBeTruthy(); - }); - - it('should return true when undefine', () => { - expect(isValidJson(undefined as any)).toBeTruthy(); - }); - - it('should return false when invalid string', () => { - expect(isValidJson(input.invalid)).toBeFalsy(); - }); - - it('should return true when valid string', () => { - expect(isValidJson(input.valid)).toBeTruthy(); - }); - - it('should return false if a number', () => { - expect(isValidJson('0')).toBeFalsy(); - }); - }); -}); diff --git a/src/legacy/core_plugins/data/public/search/aggs/utils.ts b/src/legacy/core_plugins/data/public/search/aggs/utils.ts index 62f07ce44ab46..9a344ff2c060d 100644 --- a/src/legacy/core_plugins/data/public/search/aggs/utils.ts +++ b/src/legacy/core_plugins/data/public/search/aggs/utils.ts @@ -20,35 +20,6 @@ import { leastCommonInterval } from 'ui/vis/lib/least_common_interval'; import { isValidEsInterval } from '../../../common'; -/** - * Check a string if it's a valid JSON. - * - * @param {string} value a string that should be validated - * @returns {boolean} true if value is a valid JSON or if value is an empty string, or a string with whitespaces, otherwise false - */ -function isValidJson(value: string): boolean { - if (!value || value.length === 0) { - return true; - } - - const trimmedValue = value.trim(); - - if (trimmedValue.length === 0) { - return true; - } - - if (trimmedValue[0] === '{' || trimmedValue[0] === '[') { - try { - JSON.parse(trimmedValue); - return true; - } catch (e) { - return false; - } - } else { - return false; - } -} - function isValidInterval(value: string, baseInterval?: string) { if (baseInterval) { return _parseWithBase(value, baseInterval); @@ -69,4 +40,4 @@ function _parseWithBase(value: string, baseInterval: string) { } } -export { isValidJson, isValidInterval }; +export { isValidInterval }; diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 3fab746fb9034..37ffbf4939d16 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -17,13 +17,12 @@ * under the License. */ -import React, { useEffect } from 'react'; +import React, { useEffect, useState } from 'react'; import { EuiFormRow, EuiIconTip, EuiCodeEditor } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; -import { isValidJson } from '../../legacy_imports'; import { AggParamEditorProps } from '../agg_param_props'; function RawJsonParamEditor({ @@ -34,6 +33,8 @@ function RawJsonParamEditor({ setValue, setTouched, }: AggParamEditorProps) { + const [isFormValid, setFormValidity] = useState(false); + const label = ( <> {' '} @@ -47,21 +48,19 @@ function RawJsonParamEditor({ /> ); - const isValid = isValidJson(value); const onChange = (newValue: string) => { setValue(newValue); - setValidity(isValidJson(newValue)); }; useEffect(() => { - setValidity(isValid); - }, [isValid]); + setValidity(isFormValid); + }, [isFormValid]); return ( @@ -72,6 +71,9 @@ function RawJsonParamEditor({ width="100%" height="250px" value={value} + onValidate={(annotations: any[]) => { + setFormValidity(!annotations || annotations.length === 0); + }} setOptions={{ fontSize: '14px', }} diff --git a/src/legacy/core_plugins/vis_default_editor/public/legacy_imports.ts b/src/legacy/core_plugins/vis_default_editor/public/legacy_imports.ts index 5e547eed1c957..8e201fedd5c54 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/legacy_imports.ts +++ b/src/legacy/core_plugins/vis_default_editor/public/legacy_imports.ts @@ -44,7 +44,7 @@ export { parentPipelineType } from 'ui/agg_types'; export { siblingPipelineType } from 'ui/agg_types'; export { isType, isStringType } from 'ui/agg_types'; export { OptionedValueProp, OptionedParamEditorProps, OptionedParamType } from 'ui/agg_types'; -export { isValidJson, isValidInterval } from 'ui/agg_types'; +export { isValidInterval } from 'ui/agg_types'; export { AggParamOption } from 'ui/agg_types'; export { CidrMask } from 'ui/agg_types'; From 7d1ca89b3d37a536c45929e6f4817a9d54f03664 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 28 Feb 2020 11:19:08 -0500 Subject: [PATCH 3/8] Changed the raw_json component to disregard the first validation, which will be a false positive. Removed isValidJson from agg_types --- .../public/components/controls/raw_json.tsx | 22 ++++++++++++------- src/legacy/ui/public/agg_types/index.ts | 1 - 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 37ffbf4939d16..ca8d69e765a4b 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useEffect, useState } from 'react'; +import React, { useState } from 'react'; import { EuiFormRow, EuiIconTip, EuiCodeEditor } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -33,7 +33,8 @@ function RawJsonParamEditor({ setValue, setTouched, }: AggParamEditorProps) { - const [isFormValid, setFormValidity] = useState(false); + const [isFormValid, setFormValidity] = useState(true); + const [editorReady, setEditorReady] = useState(false); const label = ( <> @@ -53,9 +54,16 @@ function RawJsonParamEditor({ setValue(newValue); }; - useEffect(() => { - setValidity(isFormValid); - }, [isFormValid]); + const onEditorValidate = (annotations: any[]) => { + //The first onValidate returned from EuiCodeEditor is a false positive + if (editorReady) { + const validity = annotations.length === 0; + setFormValidity(validity); + setValidity(validity); + } else { + setEditorReady(true); + } + }; return ( { - setFormValidity(!annotations || annotations.length === 0); - }} + onValidate={onEditorValidate} setOptions={{ fontSize: '14px', }} diff --git a/src/legacy/ui/public/agg_types/index.ts b/src/legacy/ui/public/agg_types/index.ts index ac5d0bed7ef15..fdea427166175 100644 --- a/src/legacy/ui/public/agg_types/index.ts +++ b/src/legacy/ui/public/agg_types/index.ts @@ -74,7 +74,6 @@ export { isStringType, isType, isValidInterval, - isValidJson, OptionedParamType, parentPipelineType, propFilter, From aa8823e8cb8182bb6feac5a571020d50d6380d08 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Fri, 28 Feb 2020 11:53:21 -0500 Subject: [PATCH 4/8] Fixed eslint error... --- .../vis_default_editor/public/components/controls/raw_json.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index ca8d69e765a4b..7996ea7001f53 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -55,7 +55,7 @@ function RawJsonParamEditor({ }; const onEditorValidate = (annotations: any[]) => { - //The first onValidate returned from EuiCodeEditor is a false positive + // The first onValidate returned from EuiCodeEditor is a false positive if (editorReady) { const validity = annotations.length === 0; setFormValidity(validity); From 2e1403244155082f730f4a072b3f26bf9d8bf23d Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Tue, 3 Mar 2020 18:15:16 -0500 Subject: [PATCH 5/8] Made a11y changes, replaced any with unknown --- .../public/components/controls/raw_json.tsx | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 7996ea7001f53..5ff6f7682f1e5 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -19,9 +19,8 @@ import React, { useState } from 'react'; -import { EuiFormRow, EuiIconTip, EuiCodeEditor } from '@elastic/eui'; +import { EuiFormRow, EuiIconTip, EuiCodeEditor, EuiScreenReaderOnly } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; import { AggParamEditorProps } from '../agg_param_props'; @@ -36,17 +35,19 @@ function RawJsonParamEditor({ const [isFormValid, setFormValidity] = useState(true); const [editorReady, setEditorReady] = useState(false); + const editorTooltipText = i18n.translate('visDefaultEditor.controls.jsonInputTooltip', { + defaultMessage: + "Any JSON formatted properties you add here will be merged with the elasticsearch aggregation definition for this section. For example 'shard_size' on a terms aggregation.", + }); + + const jsonEditorLabelText = i18n.translate('visDefaultEditor.controls.jsonInputLabel', { + defaultMessage: 'JSON input', + }); + const label = ( <> - {' '} - + {jsonEditorLabelText}{' '} + ); @@ -54,7 +55,7 @@ function RawJsonParamEditor({ setValue(newValue); }; - const onEditorValidate = (annotations: any[]) => { + const onEditorValidate = (annotations: unknown[]) => { // The first onValidate returned from EuiCodeEditor is a false positive if (editorReady) { const validity = annotations.length === 0; @@ -72,22 +73,28 @@ function RawJsonParamEditor({ fullWidth={true} compressed > - + <> + + +

{editorTooltipText}

+ + ); } From 37e8cf9cbc0e7dd6569540688d66d9c6094a670a Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 5 Mar 2020 17:08:15 -0500 Subject: [PATCH 6/8] Added suggestions from PR --- .../public/components/controls/raw_json.tsx | 70 +++++++++++-------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 5ff6f7682f1e5..602f1e4f85414 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useState } from 'react'; +import React, { useState, useMemo, useCallback } from 'react'; import { EuiFormRow, EuiIconTip, EuiCodeEditor, EuiScreenReaderOnly } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -32,44 +32,54 @@ function RawJsonParamEditor({ setValue, setTouched, }: AggParamEditorProps) { - const [isFormValid, setFormValidity] = useState(true); + const [isFieldValid, setFieldValidity] = useState(true); const [editorReady, setEditorReady] = useState(false); - const editorTooltipText = i18n.translate('visDefaultEditor.controls.jsonInputTooltip', { - defaultMessage: - "Any JSON formatted properties you add here will be merged with the elasticsearch aggregation definition for this section. For example 'shard_size' on a terms aggregation.", - }); - - const jsonEditorLabelText = i18n.translate('visDefaultEditor.controls.jsonInputLabel', { - defaultMessage: 'JSON input', - }); + const editorTooltipText = useMemo( + () => + i18n.translate('visDefaultEditor.controls.jsonInputTooltip', { + defaultMessage: + "Any JSON formatted properties you add here will be merged with the elasticsearch aggregation definition for this section. For example 'shard_size' on a terms aggregation.", + }), + [] + ); - const label = ( - <> - {jsonEditorLabelText}{' '} - - + const jsonEditorLabelText = useMemo( + () => + i18n.translate('visDefaultEditor.controls.jsonInputLabel', { + defaultMessage: 'JSON input', + }), + [] ); - const onChange = (newValue: string) => { - setValue(newValue); - }; + const label = useMemo( + () => ( + <> + {jsonEditorLabelText}{' '} + + + ), + [] + ); - const onEditorValidate = (annotations: unknown[]) => { - // The first onValidate returned from EuiCodeEditor is a false positive - if (editorReady) { - const validity = annotations.length === 0; - setFormValidity(validity); - setValidity(validity); - } else { - setEditorReady(true); - } - }; + const onEditorValidate = useCallback( + (annotations: unknown[]) => { + // The first onValidate returned from EuiCodeEditor is a false negative + if (editorReady) { + const validity = annotations.length === 0; + setFieldValidity(validity); + setValidity(validity); + } else { + setEditorReady(true); + } + }, + [setValidity] + ); return ( @@ -85,7 +95,7 @@ function RawJsonParamEditor({ setOptions={{ fontSize: '14px', }} - onChange={onChange} + onChange={setValue} fullWidth={true} onBlur={setTouched} aria-label={jsonEditorLabelText} From e0780aeb88c16c6a1fe9b6c8b3e8aac1e6e9cc35 Mon Sep 17 00:00:00 2001 From: Devon A Thomson Date: Thu, 5 Mar 2020 17:34:02 -0500 Subject: [PATCH 7/8] updated to EUI 20.0.2 --- .../vis_default_editor/public/components/controls/raw_json.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 602f1e4f85414..8e6900cc33ac5 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -85,7 +85,6 @@ function RawJsonParamEditor({ > <> Date: Fri, 6 Mar 2020 12:51:31 -0500 Subject: [PATCH 8/8] added dependencies to hooks --- .../public/components/controls/raw_json.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx index 8e6900cc33ac5..b6a6da09fb20e 100644 --- a/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx +++ b/src/legacy/core_plugins/vis_default_editor/public/components/controls/raw_json.tsx @@ -25,7 +25,6 @@ import { i18n } from '@kbn/i18n'; import { AggParamEditorProps } from '../agg_param_props'; function RawJsonParamEditor({ - agg, showValidation, value = '', setValidity, @@ -59,7 +58,7 @@ function RawJsonParamEditor({ ), - [] + [jsonEditorLabelText, editorTooltipText] ); const onEditorValidate = useCallback( @@ -73,7 +72,7 @@ function RawJsonParamEditor({ setEditorReady(true); } }, - [setValidity] + [setValidity, editorReady] ); return (