Web console: better json-input feedback (#8851)

* better json-input feedback * seamless Hjson * fix tests
2025-02-17 07:25:02 +00:00 · 2019-11-11 17:06:03 -08:00 · 2019-11-11 17:06:03 -08:00 · df2f77c58d
commit df2f77c58d
parent e9e1625e96
12 changed files with 242 additions and 149 deletions
--- a/web-console/src/components/auto-form/snapshots/auto-form.spec.tsx.snap
+++ b/web-console/src/components/auto-form/snapshots/auto-form.spec.tsx.snap
@ -319,8 +319,8 @@ exports[`auto-form snapshot matches snapshot 1`] = `
      class="bp3-form-content"
    >
      <div
-        class=" ace_editor ace-tm"
-        id="ace-editor"
+        class=" ace_editor ace-tm json-input"
+        id="brace-editor"
        style="width: 100%; height: 8vh;"
      >
        <textarea
@ -425,8 +425,8 @@ exports[`auto-form snapshot matches snapshot 1`] = `
      class="bp3-form-content"
    >
      <div
-        class=" ace_editor ace-tm"
-        id="ace-editor"
+        class=" ace_editor ace-tm json-input"
+        id="brace-editor"
        style="width: 100%; height: 8vh;"
      >
        <textarea
--- a/web-console/src/components/auto-form/auto-form.tsx
+++ b/web-console/src/components/auto-form/auto-form.tsx
@ -56,18 +56,10 @@ export interface AutoFormProps<T> {
  onChange: (newModel: T) => void;
  onFinalize?: () => void;
  showCustom?: (model: T) => boolean;
-  updateJsonValidity?: (jsonValidity: boolean) => void;
  large?: boolean;
 }

-export interface AutoFormState {
-  jsonInputsValidity: any;
-}
-
-export class AutoForm<T extends Record<string, any>> extends React.PureComponent<
-  AutoFormProps<T>,
-  AutoFormState
-> {
+export class AutoForm<T extends Record<string, any>> extends React.PureComponent<AutoFormProps<T>> {
  static REQUIRED_INTENT = Intent.PRIMARY;

  static makeLabelName(label: string): string {
@ -100,9 +92,7 @@ export class AutoForm<T extends Record<string, any>> extends React.PureComponent

  constructor(props: AutoFormProps<T>) {
    super(props);
-    this.state = {
-      jsonInputsValidity: {},
-    };
+    this.state = {};
  }

  private fieldChange = (field: Field<T>, newValue: any) => {
@ -250,27 +240,12 @@ export class AutoForm<T extends Record<string, any>> extends React.PureComponent
  }

  private renderJsonInput(field: Field<T>): JSX.Element {
-    const { model, updateJsonValidity } = this.props;
-    const { jsonInputsValidity } = this.state;
-
-    const updateInputValidity = (e: any) => {
-      if (updateJsonValidity) {
-        const newJsonInputValidity = Object.assign({}, jsonInputsValidity, { [field.name]: e });
-        this.setState({
-          jsonInputsValidity: newJsonInputValidity,
-        });
-        const allJsonValid: boolean = Object.keys(newJsonInputValidity).every(
-          property => newJsonInputValidity[property] === true,
-        );
-        updateJsonValidity(allJsonValid);
-      }
-    };
+    const { model } = this.props;

    return (
      <JsonInput
        value={deepGet(model as any, field.name)}
        onChange={(v: any) => this.fieldChange(field, v)}
-        updateInputValidity={updateInputValidity}
        placeholder={field.placeholder}
      />
    );
--- a/web-console/src/components/json-input/snapshots/json-input.spec.tsx.snap
+++ b/web-console/src/components/json-input/snapshots/json-input.spec.tsx.snap
@ -1,9 +1,101 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`json input matches snapshot 1`] = `
+exports[`json input matches snapshot (null) 1`] = `
 <div
-  class=" ace_editor ace-tm"
-  id="ace-editor"
+  class=" ace_editor ace-tm json-input"
+  id="brace-editor"
+  style="width: 100%; height: 8vh;"
+>
+  <textarea
+    autocapitalize="off"
+    autocorrect="off"
+    class="ace_text-input"
+    spellcheck="false"
+    style="opacity: 0;"
+    wrap="off"
+  />
+  <div
+    aria-hidden="true"
+    class="ace_gutter"
+    style="display: none;"
+  >
+    <div
+      class="ace_layer ace_gutter-layer ace_folding-enabled"
+    />
+    <div
+      class="ace_gutter-active-line"
+    />
+  </div>
+  <div
+    class="ace_scroller"
+  >
+    <div
+      class="ace_content"
+    >
+      <div
+        class="ace_layer ace_print-margin-layer"
+      >
+        <div
+          class="ace_print-margin"
+          style="left: 4px; visibility: hidden;"
+        />
+      </div>
+      <div
+        class="ace_layer ace_marker-layer"
+      />
+      <div
+        class="ace_layer ace_text-layer"
+        style="padding: 0px 4px;"
+      />
+      <div
+        class="ace_layer ace_marker-layer"
+      />
+      <div
+        class="ace_layer ace_cursor-layer ace_hidden-cursors"
+      >
+        <div
+          class="ace_cursor"
+        />
+      </div>
+    </div>
+  </div>
+  <div
+    class="ace_scrollbar ace_scrollbar-v"
+    style="display: none; width: 20px;"
+  >
+    <div
+      class="ace_scrollbar-inner"
+      style="width: 20px;"
+    />
+  </div>
+  <div
+    class="ace_scrollbar ace_scrollbar-h"
+    style="display: none; height: 20px;"
+  >
+    <div
+      class="ace_scrollbar-inner"
+      style="height: 20px;"
+    />
+  </div>
+  <div
+    style="height: auto; width: auto; top: 0px; left: 0px; visibility: hidden; position: absolute; white-space: pre; overflow: hidden;"
+  >
+    <div
+      style="height: auto; width: auto; top: 0px; left: 0px; visibility: hidden; position: absolute; white-space: pre; overflow: visible;"
+    />
+    <div
+      style="height: auto; width: auto; top: 0px; left: 0px; visibility: hidden; position: absolute; white-space: pre; overflow: visible;"
+    >
+      XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
+    </div>
+  </div>
+</div>
+`;
+
+exports[`json input matches snapshot (value) 1`] = `
+<div
+  class=" ace_editor ace-tm json-input"
+  id="brace-editor"
  style="width: 100%; height: 8vh;"
 >
  <textarea
--- a/web-console/src/components/json-input/json-input.scss
+++ b/web-console/src/components/json-input/json-input.scss
@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF 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.
+ */
+
+.json-input {
+  &.invalid::after {
+    content: '';
+    position: absolute;
+    top: 0;
+    bottom: 0;
+    left: 0;
+    right: 0;
+    border: 1px solid #ff4852;
+    pointer-events: none;
+  }
+}
--- a/web-console/src/components/json-input/json-input.spec.tsx
+++ b/web-console/src/components/json-input/json-input.spec.tsx
@ -22,8 +22,17 @@ import React from 'react';
 import { JsonInput } from './json-input';

 describe('json input', () => {
-  it('matches snapshot', () => {
-    const jsonCollapse = <JsonInput onChange={() => {}} value={'test'} />;
+  it('matches snapshot (null)', () => {
+    const jsonCollapse = <JsonInput onChange={() => {}} value={null} />;
+    const { container } = render(jsonCollapse);
+    expect(container.firstChild).toMatchSnapshot();
+  });
+
+  it('matches snapshot (value)', () => {
+    const value = {
+      hello: ['world', { a: 1, b: 2 }],
+    };
+    const jsonCollapse = <JsonInput onChange={() => {}} value={value} />;
    const { container } = render(jsonCollapse);
    expect(container.firstChild).toMatchSnapshot();
  });
--- a/web-console/src/components/json-input/json-input.tsx
+++ b/web-console/src/components/json-input/json-input.tsx
@ -16,82 +16,82 @@
 * limitations under the License.
 */

-import React from 'react';
+import classNames = require('classnames');
+import Hjson from 'hjson';
+import React, { useState } from 'react';
 import AceEditor from 'react-ace';

-import { parseStringToJson, stringifyJson, validJson } from '../../utils';
+import './json-input.scss';
+
+function parseHjson(str: string) {
+  return str === '' ? null : Hjson.parse(str);
+}
+
+function stringifyJson(item: any): string {
+  if (item != null) {
+    return JSON.stringify(item, null, 2);
+  } else {
+    return '';
+  }
+}

 interface JsonInputProps {
-  onChange: (newJSONValue: any) => void;
  value: any;
-  updateInputValidity?: (valueValid: boolean) => void;
+  onChange: (value: any) => void;
  placeholder?: string;
  focus?: boolean;
  width?: string;
  height?: string;
 }

-interface JsonInputState {
-  stringValue: string;
-}
+export const JsonInput = React.memo(function JsonInput(props: JsonInputProps) {
+  const { onChange, placeholder, focus, width, height, value } = props;
+  const stringifiedValue = stringifyJson(value);
+  const [stringValue, setStringValue] = useState(stringifiedValue);
+  const [blurred, setBlurred] = useState(false);

-export class JsonInput extends React.PureComponent<JsonInputProps, JsonInputState> {
-  constructor(props: JsonInputProps) {
-    super(props);
-    this.state = {
-      stringValue: '',
-    };
+  let parsedValue: any;
+  try {
+    parsedValue = parseHjson(stringValue);
+  } catch {}
+  if (typeof parsedValue !== 'object') parsedValue = undefined;
+
+  if (parsedValue !== undefined && stringifyJson(parsedValue) !== stringifiedValue) {
+    setStringValue(stringifiedValue);
  }

-  componentDidMount(): void {
-    const { value } = this.props;
-    const stringValue = stringifyJson(value);
-    this.setState({
-      stringValue,
-    });
-  }
-
-  componentWillReceiveProps(nextProps: JsonInputProps): void {
-    if (JSON.stringify(nextProps.value) !== JSON.stringify(this.props.value)) {
-      this.setState({
-        stringValue: stringifyJson(nextProps.value),
-      });
-    }
-  }
-
-  render(): JSX.Element {
-    const { onChange, updateInputValidity, placeholder, focus, width, height } = this.props;
-    const { stringValue } = this.state;
-    return (
-      <AceEditor
-        key="hjson"
-        mode="hjson"
-        theme="solarized_dark"
-        name="ace-editor"
-        onChange={(e: string) => {
-          this.setState({ stringValue: e });
-          if (validJson(e) || e === '') onChange(parseStringToJson(e));
-          if (updateInputValidity) updateInputValidity(validJson(e) || e === '');
-        }}
-        focus={focus}
-        fontSize={12}
-        width={width || '100%'}
-        height={height || '8vh'}
-        showPrintMargin={false}
-        showGutter={false}
-        value={stringValue}
-        placeholder={placeholder}
-        editorProps={{
-          $blockScrolling: Infinity,
-        }}
-        setOptions={{
-          enableBasicAutocompletion: false,
-          enableLiveAutocompletion: false,
-          showLineNumbers: false,
-          tabSize: 2,
-        }}
-        style={{}}
-      />
-    );
-  }
-}
+  return (
+    <AceEditor
+      className={classNames('json-input', { invalid: parsedValue === undefined && blurred })}
+      mode="hjson"
+      theme="solarized_dark"
+      onChange={(inputJson: string) => {
+        try {
+          const value = parseHjson(inputJson);
+          onChange(value);
+        } catch {}
+        setStringValue(inputJson);
+      }}
+      onFocus={() => setBlurred(false)}
+      onBlur={() => setBlurred(true)}
+      focus={focus}
+      fontSize={12}
+      width={width || '100%'}
+      height={height || '8vh'}
+      showPrintMargin={false}
+      showGutter={false}
+      value={stringValue}
+      placeholder={placeholder}
+      editorProps={{
+        $blockScrolling: Infinity,
+      }}
+      setOptions={{
+        enableBasicAutocompletion: false,
+        enableLiveAutocompletion: false,
+        showLineNumbers: false,
+        tabSize: 2,
+      }}
+      style={{}}
+    />
+  );
+});
--- a/web-console/src/dialogs/compaction-dialog/snapshots/compaction-dialog.spec.tsx.snap
+++ b/web-console/src/dialogs/compaction-dialog/snapshots/compaction-dialog.spec.tsx.snap
@ -379,8 +379,8 @@ exports[`compaction dialog matches snapshot 1`] = `
              class="bp3-form-content"
            >
              <div
-                class=" ace_editor ace-tm"
-                id="ace-editor"
+                class=" ace_editor ace-tm json-input"
+                id="brace-editor"
                style="width: 100%; height: 8vh;"
              >
                <textarea
@ -626,8 +626,8 @@ exports[`compaction dialog matches snapshot 1`] = `
              class="bp3-form-content"
            >
              <div
-                class=" ace_editor ace-tm"
-                id="ace-editor"
+                class=" ace_editor ace-tm json-input"
+                id="brace-editor"
                style="width: 100%; height: 8vh;"
              >
                <textarea
--- a/web-console/src/dialogs/compaction-dialog/compaction-dialog.tsx
+++ b/web-console/src/dialogs/compaction-dialog/compaction-dialog.tsx
@ -34,7 +34,6 @@ export interface CompactionDialogProps {

 export interface CompactionDialogState {
  currentConfig?: Record<string, any>;
-  allJsonValid: boolean;
 }

 export class CompactionDialog extends React.PureComponent<
@ -45,9 +44,7 @@ export class CompactionDialog extends React.PureComponent<

  constructor(props: CompactionDialogProps) {
    super(props);
-    this.state = {
-      allJsonValid: true,
-    };
+    this.state = {};
  }

  componentDidMount(): void {
@ -70,7 +67,7 @@ export class CompactionDialog extends React.PureComponent<

  render(): JSX.Element {
    const { onClose, onDelete, datasource, compactionConfig } = this.props;
-    const { currentConfig, allJsonValid } = this.state;
+    const { currentConfig } = this.state;

    return (
      <Dialog
@ -158,7 +155,6 @@ export class CompactionDialog extends React.PureComponent<
          ]}
          model={currentConfig}
          onChange={m => this.setState({ currentConfig: m })}
-          updateJsonValidity={e => this.setState({ allJsonValid: e })}
        />
        <div className={Classes.DIALOG_FOOTER}>
          <div className={Classes.DIALOG_FOOTER_ACTIONS}>
@ -173,7 +169,7 @@ export class CompactionDialog extends React.PureComponent<
              text="Submit"
              intent={Intent.PRIMARY}
              onClick={this.handleSubmit}
-              disabled={!currentConfig || !allJsonValid}
+              disabled={!currentConfig}
            />
          </div>
        </div>
--- a/web-console/src/dialogs/overlord-dynamic-config-dialog/overlord-dynamic-config-dialog.tsx
+++ b/web-console/src/dialogs/overlord-dynamic-config-dialog/overlord-dynamic-config-dialog.tsx
@ -35,7 +35,6 @@ export interface OverlordDynamicConfigDialogProps {

 export interface OverlordDynamicConfigDialogState {
  dynamicConfig?: Record<string, any>;
-  allJsonValid: boolean;
  historyRecords: any[];
 }

@ -48,7 +47,6 @@ export class OverlordDynamicConfigDialog extends React.PureComponent<
  constructor(props: OverlordDynamicConfigDialogProps) {
    super(props);
    this.state = {
-      allJsonValid: true,
      historyRecords: [],
    };

@ -116,7 +114,7 @@ export class OverlordDynamicConfigDialog extends React.PureComponent<

  render(): JSX.Element {
    const { onClose } = this.props;
-    const { dynamicConfig, allJsonValid, historyRecords } = this.state;
+    const { dynamicConfig, historyRecords } = this.state;

    return (
      <SnitchDialog
@ -124,7 +122,6 @@ export class OverlordDynamicConfigDialog extends React.PureComponent<
        onSave={this.saveConfig}
        onClose={onClose}
        title="Overlord dynamic config"
-        saveDisabled={!allJsonValid}
        historyRecords={historyRecords}
      >
        <p>
@ -150,7 +147,6 @@ export class OverlordDynamicConfigDialog extends React.PureComponent<
          ]}
          model={dynamicConfig}
          onChange={m => this.setState({ dynamicConfig: m })}
-          updateJsonValidity={e => this.setState({ allJsonValid: e })}
        />
      </SnitchDialog>
    );
--- a/web-console/src/utils/general.tsx
+++ b/web-console/src/utils/general.tsx
@ -260,24 +260,6 @@ export function validJson(json: string): boolean {
  }
 }

-// stringify JSON to string; if JSON is null, parse empty string ""
-export function stringifyJson(item: any): string {
-  if (item != null) {
-    return JSON.stringify(item, null, 2);
-  } else {
-    return '';
-  }
-}
-
-// parse string to JSON object; if string is empty, return null
-export function parseStringToJson(s: string): JSON | null {
-  if (s === '') {
-    return null;
-  } else {
-    return JSON.parse(s);
-  }
-}
-
 export function filterMap<T, Q>(xs: T[], f: (x: T, i: number) => Q | undefined): Q[] {
  return xs.map(f).filter((x: Q | undefined) => typeof x !== 'undefined') as Q[];
 }
--- a/web-console/src/visualization/snapshots/visualization.spec.tsx.snap
+++ b/web-console/src/visualization/snapshots/visualization.spec.tsx.snap
@ -1,18 +1,22 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

 exports[`Visualization BarUnit 1`] = `
-<rect
-  class="bar-chart-unit"
-  height="10"
-  width="10"
-  x="10"
-  y="10"
-/>
+<svg>
+  <rect
+    class="bar-chart-unit"
+    height="10"
+    width="10"
+    x="10"
+    y="10"
+  />
+</svg>
 `;

 exports[`Visualization action barGroup 1`] = `
-<g
-  class="chart-axis undefined"
-  transform="value"
-/>
+<svg>
+  <g
+    class="chart-axis undefined"
+    transform="value"
+  />
+</svg>
 `;
--- a/web-console/src/visualization/visualization.spec.tsx
+++ b/web-console/src/visualization/visualization.spec.tsx
@ -24,12 +24,21 @@ import { ChartAxis } from './chart-axis';

 describe('Visualization', () => {
  it('BarUnit', () => {
-    const barGroup = <BarUnit x={10} y={10} width={10} height={10} />;
+    const barGroup = (
+      <svg>
+        <BarUnit x={10} y={10} width={10} height={10} />
+      </svg>
+    );
    const { container } = render(barGroup);
    expect(container.firstChild).toMatchSnapshot();
  });
+
  it('action barGroup', () => {
-    const barGroup = <ChartAxis transform={'value'} scale={() => null} />;
+    const barGroup = (
+      <svg>
+        <ChartAxis transform={'value'} scale={() => null} />
+      </svg>
+    );
    const { container } = render(barGroup);
    expect(container.firstChild).toMatchSnapshot();
  });