Repository: electron/fiddle
Pull request #678 (2021-04-30 16:43:28)

feat: better execution settings UI
Refs https://github.com/electron/fiddle/pull/675#pullrequestreview-648760711.

This reworks the execution settings UI for execution flags and environment variables to something a bit more user-friendly. 

<img width="889" alt="Screen Shot 2021-05-03 at 10 41 21 AM" src="https://user-images.githubusercontent.com/2036040/116856872-21ac9980-abfc-11eb-99df-c0b534c6381e.png">



coveralls: 
[![Coverage Status](https://coveralls.io/builds/39269137/badge)](https://coveralls.io/builds/39269137)

Coverage decreased (-0.5%) to 95.402% when pulling **c409f0fc0e71ca80ef19d5b46c25dd44234340bf on better-settings-ui** into **55ca77750807e268e556b2e1314c9f4c42b40327 on master**.


Patch:
diff --git a/src/less/components/settings.less b/src/less/components/settings.less
index 5bee2be3..fcf27a9e 100644
--- a/src/less/components/settings.less
+++ b/src/less/components/settings.less
@@ -35,6 +35,10 @@
     width: 250px;
   }
 
+ .bp3-input-group {
+    margin-bottom: 15px;
+  }
+
   .bp3-divider {
     margin-top: 1rem;
     margin-bottom: 1rem;
diff --git a/src/renderer/components/settings-execution.tsx b/src/renderer/components/settings-execution.tsx
index c3bef3f4..93668022 100644
--- a/src/renderer/components/settings-execution.tsx
+++ b/src/renderer/components/settings-execution.tsx
@@ -1,4 +1,6 @@
 import {
+  Button,
+  ButtonGroup,
   Callout,
   Checkbox,
   FormGroup,
@@ -12,10 +14,19 @@ import * as React from 'react';
 import { IPackageManager } from '../npm';
 import { AppState } from '../state';
 
+export enum SettingItemType {
+  EnvVars = 'environmentVariables',
+  Flags = 'executionFlags',
+}
+
 interface ExecutionSettingsProps {
   appState: AppState;
 }
 
+interface ExecutionSettingsState {
+  [setting: string]: Record<string, string>;
+}
+
 /**
  * Settings content to manage execution-related preferences.
  *
@@ -23,18 +34,56 @@ interface ExecutionSettingsProps {
  * @extends {React.Component<ExecutionSettingsProps, {}>}
  */
 @observer
-export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
+export class ExecutionSettings extends React.Component<
+  ExecutionSettingsProps,
+  ExecutionSettingsState
+> {
   constructor(props: ExecutionSettingsProps) {
     super(props);
 
+    this.state = {
+      executionFlags: Object.assign(
+        {},
+        ...props.appState.executionFlags.map((flag, idx) => {
+          return { [idx]: flag };
+        }),
+      ),
+      environmentVariables: Object.assign(
+        {},
+        ...props.appState.environmentVariables.map((envVar, idx) => {
+          return { [idx]: envVar };
+        }),
+      ),
+    };
+
     this.handleDeleteDataChange = this.handleDeleteDataChange.bind(this);
     this.handleElectronLoggingChange = this.handleElectronLoggingChange.bind(
       this,
     );
-    this.handleExecutionFlagChange = this.handleExecutionFlagChange.bind(this);
-    this.handleEnvironmentVariableChange = this.handleEnvironmentVariableChange.bind(
-      this,
-    );
+
+    this.handleSettingsItemChange = this.handleSettingsItemChange.bind(this);
+    this.addNewSettingsItem = this.addNewSettingsItem.bind(this);
+  }
+
+  public componentDidMount() {
+    const { environmentVariables, executionFlags } = this.state;
+
+    if (Object.keys(executionFlags).length === 0) {
+      this.addNewSettingsItem(SettingItemType.Flags);
+    }
+
+    if (Object.keys(environmentVariables).length === 0) {
+      this.addNewSettingsItem(SettingItemType.EnvVars);
+    }
+  }
+
+  public componentDidUpdate() {
+    const { appState } = this.props;
+
+    for (const type of Object.values(SettingItemType)) {
+      const values = Object.values(this.state[type]);
+      appState[type] = values.filter((v) => v !== '');
+    }
   }
 
   /**
@@ -59,48 +108,156 @@ export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
   }
 
   /**
-   * Handles a change in the execution flags run with the Electron executable.
+   * Handles a change in the execution flags or environment variables
+   * run with the Electron executable.
    *
    * @param {React.ChangeEvent<HTMLInputElement>} event
+   * @param {SettingItemType} type
    */
-  public handleExecutionFlagChange(event: React.FormEvent<HTMLInputElement>) {
-    const { value } = event.currentTarget;
-    const flags = value.split('|');
-    this.props.appState.executionFlags = flags;
+  public handleSettingsItemChange(
+    event: React.ChangeEvent<HTMLInputElement>,
+    type: SettingItemType,
+  ) {
+    const { name, value } = event.currentTarget;
+
+    this.setState((prevState) => ({
+      [type]: {
+        ...prevState[type],
+        [name]: value,
+      },
+    }));
   }
 
   /**
-   * Handles a change in the environment variables run with the Electron executable.
+   * Adds a new settings item input field.
    *
-   * @param {React.ChangeEvent<HTMLInputElement>} event
+   * @param {SettingItemType} type
    */
-  public handleEnvironmentVariableChange(
-    event: React.FormEvent<HTMLInputElement>,
-  ) {
+  private addNewSettingsItem(type: SettingItemType) {
+    const array = Object.entries(this.state[type]);
+
+    this.setState((prevState) => ({
+      [type as any]: {
+        ...prevState[type],
+        [array.length]: '',
+      },
+    }));
+  }
+
+  /**
+   * Handle a change to the package manager used to install modules when running
+   * Fiddles;
+   *
+   * @param {React.FormEvent<HTMLInputElement>} event
+   */
+  private handlePMChange = (event: React.FormEvent<HTMLInputElement>) => {
+    const { appState } = this.props;
     const { value } = event.currentTarget;
 
-    const variables = value.split('|');
-    this.props.appState.environmentVariables = variables;
+    appState.packageManager = value as IPackageManager;
+  };
+
+  public renderDeleteItem(idx: string, type: SettingItemType): JSX.Element {
+    const updated = this.state[type];
+
+    const removeFn = () => {
+      if (Object.keys(updated).length === 1) {
+        updated[idx] = '';
+      } else {
+        delete updated[idx];
+      }
+
+      this.setState({ [type]: updated });
+    };
+
+    return (
+      <Button
+        icon="cross"
+        disabled={Object.keys(updated).length === 1}
+        onClick={removeFn}
+      />
+    );
+  }
+
+  private renderEnvironmentVariables() {
+    const { environmentVariables } = this.state;
+
+    const varsArray = Object.entries(environmentVariables);
+    const type = SettingItemType.EnvVars;
+
+    return (
+      <FormGroup>
+        <p>
+          Electron allows starting the executable with{' '}
+          <a href="https://www.electronjs.org/docs/api/environment-variables">
+            user-provided environment variables
+          </a>
+          , such as{' '}
+          <code>
+            NODE_OPTIONS=&quot;--no-warnings --max-old-space-size=2048&quot;
+          </code>
+          . Those can be added here to run when you start your Fiddles.
+        </p>
+        <br />
+        {varsArray.map(([idx, envVar]) => {
+          return (
+            <InputGroup
+              placeholder='NODE_OPTIONS="--no-warnings --max-old-space-size=2048"'
+              value={envVar}
+              name={idx}
+              key={idx}
+              onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
+                this.handleSettingsItemChange(e, type)
+              }
+              rightElement={this.renderDeleteItem(idx, type)}
+            />
+          );
+        })}
+      </FormGroup>
+    );
+  }
+
+  private renderExecutionFlags() {
+    const { executionFlags } = this.state;
+
+    const flagsArray = Object.entries(executionFlags);
+    const type = SettingItemType.Flags;
+
+    return (
+      <FormGroup>
+        <p>
+          Electron allows starting the executable with{' '}
+          <a href="https://www.electronjs.org/docs/api/command-line-switches">
+            user-provided flags
+          </a>
+          , such as <code>--js-flags=--expose-gc</code>. Those can be added to
+          run when you start your Fiddles.
+        </p>
+        <br />
+        {flagsArray.map(([idx, flag]) => {
+          return (
+            <InputGroup
+              placeholder="--js-flags=--expose-gc"
+              value={flag}
+              name={idx}
+              key={idx}
+              onChange={(e: React.ChangeEvent<HTMLInputElement>) =>
+                this.handleSettingsItemChange(e, type)
+              }
+              rightElement={this.renderDeleteItem(idx, type)}
+            />
+          );
+        })}
+      </FormGroup>
+    );
   }
 
   public render() {
     const {
       isKeepingUserDataDirs,
       isEnablingElectronLogging,
-      executionFlags,
-      environmentVariables,
     } = this.props.appState;
 
-    const deleteUserDirLabel = `
-      Whenever Electron runs, it creates a user data directory for cookies, the cache,
-      and various other things that it needs to keep around. Since fiddles are usually
-      just run once, we delete this directory after your fiddle exits. Enable this
-      setting to keep the user data directories around.
-    `.trim();
-    const electronLoggingLabel = `
-      There are some flags that Electron uses to log extra information both internally
-      and through Chromium. Enable this option to make Fiddle produce those logs.`.trim();
-
     return (
       <div>
         <h2>Execution</h2>
@@ -110,7 +267,14 @@ export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
         </Callout>
         <br />
         <Callout>
-          <FormGroup label={deleteUserDirLabel}>
+          <FormGroup>
+            <p>
+              Whenever Electron runs, it creates a user data directory for
+              cookies, the cache, and various other things that it needs to keep
+              around. Since fiddles are usually just run once, we delete this
+              directory after your fiddle exits. Enable this setting to keep the
+              user data directories around.
+            </p>
             <Checkbox
               checked={isKeepingUserDataDirs}
               label="Do not delete user data directories."
@@ -120,10 +284,12 @@ export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
         </Callout>
         <br />
         <Callout>
-          <FormGroup label={electronLoggingLabel}>
+          <FormGroup>
             <p>
-              Enabling advanced Electron logging will set the{' '}
-              <code>ELECTRON_ENABLE_LOGGING</code>,
+              There are some flags that Electron uses to log extra information
+              both internally and through Chromium. Enable this option to make
+              Fiddle produce those logs. Enabling advanced Electron logging will
+              set the <code>ELECTRON_ENABLE_LOGGING</code>,{' '}
               <code>ELECTRON_DEBUG_NOTIFICATION</code>, and{' '}
               <code>ELECTRON_ENABLE_STACK_DUMPING</code> environment variables
               to true. See{' '}
@@ -141,46 +307,25 @@ export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
         </Callout>
         <br />
         <Callout>
-          <FormGroup>
-            <p>
-              Electron allows starting the executable with{' '}
-              <a href="https://www.electronjs.org/docs/api/command-line-switches">
-                user-provided flags
-              </a>
-              , such as <code>--js-flags=--expose-gc</code>. Those can be added
-              here as bar-separated (|) flags to run when you start your
-              Fiddles.
-            </p>
-            <br />
-            <InputGroup
-              placeholder="--js-flags=--expose-gc|--lang=es"
-              value={executionFlags.join('|')}
-              onChange={this.handleExecutionFlagChange}
-            />
-          </FormGroup>
+          {this.renderExecutionFlags()}
+          <ButtonGroup>
+            <Button
+              onClick={() => this.addNewSettingsItem(SettingItemType.Flags)}
+            >
+              Add New Flag
+            </Button>
+          </ButtonGroup>
         </Callout>
         <br />
         <Callout>
-          <FormGroup>
-            <p>
-              Electron allows starting the executable with{' '}
-              <a href="https://www.electronjs.org/docs/api/environment-variables">
-                user-provided environment variables
-              </a>
-              , such as{' '}
-              <code>
-                NODE_OPTIONS=&quot;--no-warnings --max-old-space-size=2048&quot;
-              </code>
-              . Those can be added here as bar-separated (|) variables to run
-              when you start your Fiddles.
-            </p>
-            <br />
-            <InputGroup
-              placeholder='NODE_OPTIONS="--no-warnings --max-old-space-size=2048"'
-              value={environmentVariables.join('|')}
-              onChange={this.handleEnvironmentVariableChange}
-            />
-          </FormGroup>
+          {this.renderEnvironmentVariables()}
+          <ButtonGroup>
+            <Button
+              onClick={() => this.addNewSettingsItem(SettingItemType.EnvVars)}
+            >
+              Add New Variable
+            </Button>
+          </ButtonGroup>
         </Callout>
         <br />
         <Callout>
@@ -214,9 +359,4 @@ export class ExecutionSettings extends React.Component<ExecutionSettingsProps> {
       </div>
     );
   }
-
-  private handlePMChange = (event: React.FormEvent<HTMLInputElement>) => {
-    this.props.appState.packageManager = event.currentTarget
-      .value as IPackageManager;
-  };
 }


Test patch:
diff --git a/tests/renderer/components/__snapshots__/settings-execution-spec.tsx.snap b/tests/renderer/components/__snapshots__/settings-execution-spec.tsx.snap
index 73d9b636..3de9e94d 100644
--- a/tests/renderer/components/__snapshots__/settings-execution-spec.tsx.snap
+++ b/tests/renderer/components/__snapshots__/settings-execution-spec.tsx.snap
@@ -10,12 +10,10 @@ exports[`ExecutionSettings component renders 1`] = `
   </Blueprint3.Callout>
   <br />
   <Blueprint3.Callout>
-    <Blueprint3.FormGroup
-      label="Whenever Electron runs, it creates a user data directory for cookies, the cache,
-      and various other things that it needs to keep around. Since fiddles are usually
-      just run once, we delete this directory after your fiddle exits. Enable this
-      setting to keep the user data directories around."
-    >
+    <Blueprint3.FormGroup>
+      <p>
+        Whenever Electron runs, it creates a user data directory for cookies, the cache, and various other things that it needs to keep around. Since fiddles are usually just run once, we delete this directory after your fiddle exits. Enable this setting to keep the user data directories around.
+      </p>
       <Blueprint3.Checkbox
         label="Do not delete user data directories."
         onChange={[Function]}
@@ -24,17 +22,14 @@ exports[`ExecutionSettings component renders 1`] = `
   </Blueprint3.Callout>
   <br />
   <Blueprint3.Callout>
-    <Blueprint3.FormGroup
-      label="There are some flags that Electron uses to log extra information both internally
-      and through Chromium. Enable this option to make Fiddle produce those logs."
-    >
+    <Blueprint3.FormGroup>
       <p>
-        Enabling advanced Electron logging will set the
-         
+        There are some flags that Electron uses to log extra information both internally and through Chromium. Enable this option to make Fiddle produce those logs. Enabling advanced Electron logging will set the 
         <code>
           ELECTRON_ENABLE_LOGGING
         </code>
         ,
+         
         <code>
           ELECTRON_DEBUG_NOTIFICATION
         </code>
@@ -74,15 +69,31 @@ exports[`ExecutionSettings component renders 1`] = `
         <code>
           --js-flags=--expose-gc
         </code>
-        . Those can be added here as bar-separated (|) flags to run when you start your Fiddles.
+        . Those can be added to run when you start your Fiddles.
       </p>
       <br />
       <Blueprint3.InputGroup
+        key="0"
+        name="0"
         onChange={[Function]}
-        placeholder="--js-flags=--expose-gc|--lang=es"
+        placeholder="--js-flags=--expose-gc"
+        rightElement={
+          <Blueprint3.Button
+            disabled={true}
+            icon="cross"
+            onClick={[Function]}
+          />
+        }
         value=""
       />
     </Blueprint3.FormGroup>
+    <Blueprint3.ButtonGroup>
+      <Blueprint3.Button
+        onClick={[Function]}
+      >
+        Add New Flag
+      </Blueprint3.Button>
+    </Blueprint3.ButtonGroup>
   </Blueprint3.Callout>
   <br />
   <Blueprint3.Callout>
@@ -100,15 +111,31 @@ exports[`ExecutionSettings component renders 1`] = `
         <code>
           NODE_OPTIONS="--no-warnings --max-old-space-size=2048"
         </code>
-        . Those can be added here as bar-separated (|) variables to run when you start your Fiddles.
+        . Those can be added here to run when you start your Fiddles.
       </p>
       <br />
       <Blueprint3.InputGroup
+        key="0"
+        name="0"
         onChange={[Function]}
         placeholder="NODE_OPTIONS=\\"--no-warnings --max-old-space-size=2048\\""
+        rightElement={
+          <Blueprint3.Button
+            disabled={true}
+            icon="cross"
+            onClick={[Function]}
+          />
+        }
         value=""
       />
     </Blueprint3.FormGroup>
+    <Blueprint3.ButtonGroup>
+      <Blueprint3.Button
+        onClick={[Function]}
+      >
+        Add New Variable
+      </Blueprint3.Button>
+    </Blueprint3.ButtonGroup>
   </Blueprint3.Callout>
   <br />
   <Blueprint3.Callout>
diff --git a/tests/renderer/components/settings-execution-spec.tsx b/tests/renderer/components/settings-execution-spec.tsx
index ede9414e..cf0ec800 100644
--- a/tests/renderer/components/settings-execution-spec.tsx
+++ b/tests/renderer/components/settings-execution-spec.tsx
@@ -1,7 +1,10 @@
 import { shallow } from 'enzyme';
 import * as React from 'react';
 
-import { ExecutionSettings } from '../../../src/renderer/components/settings-execution';
+import {
+  ExecutionSettings,
+  SettingItemType,
+} from '../../../src/renderer/components/settings-execution';
 
 describe('ExecutionSettings component', () => {
   let store: any;
@@ -54,47 +57,74 @@ describe('ExecutionSettings component', () => {
     });
   });
 
-  describe('handleExecutionFlagChange()', () => {
-    it('handles new flags', async () => {
-      const wrapper = shallow(<ExecutionSettings appState={store} />);
-      const instance = wrapper.instance() as any;
-      await instance.handleExecutionFlagChange({
-        currentTarget: { value: '--lang=es' },
+  describe('handleItemSettingsChange()', () => {
+    describe('with executionFlags', () => {
+      it('updates when new flags are added', async () => {
+        const wrapper = shallow(<ExecutionSettings appState={store} />);
+        const instance = wrapper.instance() as any;
+
+        const lang = '--lang=es';
+        const flags = '--js-flags=--expose-gc';
+
+        await instance.handleSettingsItemChange(
+          {
+            currentTarget: { name: '0', value: lang },
+          },
+          SettingItemType.Flags,
+        );
+
+        expect(instance.state.executionFlags).toEqual({ '0': lang });
+        expect(store.executionFlags).toEqual([lang]);
+
+        await instance.handleSettingsItemChange(
+          {
+            currentTarget: { name: '1', value: flags },
+          },
+          SettingItemType.Flags,
+        );
+
+        expect(instance.state.executionFlags).toEqual({
+          '0': lang,
+          '1': flags,
+        });
+        expect(store.executionFlags).toEqual([lang, flags]);
       });
-
-      expect(store.executionFlags).toEqual(['--lang=es']);
-
-      await instance.handleExecutionFlagChange({
-        currentTarget: { value: '--lang=es|--js-flags=--expose-gc' },
-      });
-
-      expect(store.executionFlags).toEqual([
-        '--lang=es',
-        '--js-flags=--expose-gc',
-      ]);
     });
-  });
 
-  describe('handleEnvironmentVariableChange()', () => {
-    it('handles new environment variables', async () => {
-      const wrapper = shallow(<ExecutionSettings appState={store} />);
-      const instance = wrapper.instance() as any;
-
-      const dragRegions = 'ELECTRON_DEBUG_DRAG_REGIONS=1';
-      const trash = 'ELECTRON_TRASH=trash-cli';
-      await instance.handleEnvironmentVariableChange({
-        currentTarget: { value: dragRegions },
+    describe('with environmentVariables', () => {
+      it('updates when new flags are added', async () => {
+        const wrapper = shallow(<ExecutionSettings appState={store} />);
+        const instance = wrapper.instance() as any;
+
+        const debug = 'ELECTRON_DEBUG_DRAG_REGIONS=1';
+        const trash = 'ELECTRON_TRASH=trash-cli';
+
+        await instance.handleSettingsItemChange(
+          {
+            currentTarget: {
+              name: '0',
+              value: debug,
+            },
+          },
+          SettingItemType.EnvVars,
+        );
+
+        expect(instance.state.environmentVariables).toEqual({ '0': debug });
+        expect(store.environmentVariables).toEqual([debug]);
+
+        await instance.handleSettingsItemChange(
+          {
+            currentTarget: { name: '1', value: trash },
+          },
+          SettingItemType.EnvVars,
+        );
+
+        expect(instance.state.environmentVariables).toEqual({
+          '0': debug,
+          '1': trash,
+        });
+        expect(store.environmentVariables).toEqual([debug, trash]);
       });
-
-      expect(store.environmentVariables).toEqual([dragRegions]);
-
-      await instance.handleEnvironmentVariableChange({
-        currentTarget: {
-          value: `${dragRegions}|${trash}`,
-        },
-      });
-
-      expect(store.environmentVariables).toEqual([dragRegions, trash]);
     });
   });
 });