ui: make eye toggles only toggle on/off for their respective row (#13406)

* chore:  update tests to handle invidual toggle responsibility

* chore:  prettify secure-variable-form template

* ui:  extract input group markup into seperate component

ui/app/components/secure-variable-form.hbs

-<form
-  class="new-secure-variables"
-  {{on "submit" this.save}}
-  autocomplete="off"
->
+<form class="new-secure-variables" autocomplete="off" {{on "submit" this.save}}>
   {{!-- TODO: {{if this.parseError 'is-danger'}} on inputs --}}
   <div>
     <label>
-      <span>Path</span>
+      <span>
+        Path
+      </span>
       <Input
         @type="text"
         @value={{@model.path}}
@@ -15,74 +13,73 @@
         disabled={{not @model.isNew}}
         {{on "input" this.validatePath}}
         {{autofocus}}
-        />
+      />
     </label>
     {{#if this.duplicatePathWarning}}
       <p class="duplicate-path-error help is-danger">
-        There is already a Secure Variable located at {{@model.path}}.
+        There is already a Secure Variable located at
+        {{@model.path}}
+        .
         <br />
-        Please choose a different path, or <LinkTo @route="variables.variable.edit" @model={{this.duplicatePathWarning.path}}>edit the existing Secure Variable</LinkTo>.
+        Please choose a different path, or
+        <LinkTo
+          @route="variables.variable.edit"
+          @model={{this.duplicatePathWarning.path}}
+        >
+          edit the existing Secure Variable
+        </LinkTo>
+        .
       </p>
     {{/if}}
   </div>
   {{#each this.keyValues as |entry iter|}}
-  <div class="key-value">
-
-    <label>
-      <span>Key</span>
-      <Input
-        @type="text"
-        @value={{entry.key}}
-        class="input"
-        {{autofocus ignore=(eq iter 0)}}
-        {{on "input" (fn this.validateKey entry)}}
+    <div class="key-value">
+      <label>
+        <span>
+          Key
+        </span>
+        <Input
+          @type="text"
+          @value={{entry.key}}
+          class="input"
+          {{autofocus ignore=(eq iter 0)}}
+          {{on "input" (fn this.validateKey entry)}}
         />
-    </label>
-
-    <label class="value-label">
-      <span>Value
-      </span>
-      <Input
-        @type={{this.valueFieldType}}
-        @value={{entry.value}}
-        class="input value-input"
-        autocomplete="new-password" {{!-- prevent auto-fill --}}
-        />
-      <button
-        class="show-hide-values button is-light"
-        type="button"
-        tabindex="-1"
-        {{on "click" this.toggleShowHide}}
-      >
-        <FlightIcon
-          @name={{if this.shouldHideValues "eye-off" "eye"}}
-          @title={{if this.shouldHideValues "Show Values" "Hide Values"}}
-        />
-      </button>
-    </label>
-
-    {{#if (eq entry this.keyValues.lastObject)}}
-      <button
-        {{on "click" this.appendRow}}
-        class="add-more button is-info is-inverted"
-        type="button"
-        disabled={{not (and entry.key entry.value)}}
-      >Add More</button>
-    {{else}}
-      <button
-        {{on "click" (action this.deleteRow entry)}}
-        class="delete-row button is-danger is-inverted" type="button">Delete</button>
-    {{/if}}
-
-    {{#each-in entry.warnings as |k v|}}
-      <span class="key-value-error help is-danger">{{v}}</span>
-    {{/each-in}}
-  </div>
+      </label>
+      <SecureVariableForm::InputGroup @entry={{entry}} />
+      {{#if (eq entry this.keyValues.lastObject)}}
+        <button
+          class="add-more button is-info is-inverted"
+          type="button"
+          disabled={{not (and entry.key entry.value)}}
+          {{on "click" this.appendRow}}
+        >
+          Add More
+        </button>
+      {{else}}
+        <button
+          class="delete-row button is-danger is-inverted"
+          type="button"
+          {{on "click" (action this.deleteRow entry)}}
+        >
+          Delete
+        </button>
+      {{/if}}
+      {{#each-in entry.warnings as |k v|}}
+        <span class="key-value-error help is-danger">
+          {{v}}
+        </span>
+      {{/each-in}}
+    </div>
   {{/each}}
-
   <footer>
     <button
       disabled={{this.shouldDisableSave}}
-      class="button is-primary" type="submit">Save {{pluralize 'Variable' @this.keyValues.length}}</button>
+      class="button is-primary"
+      type="submit"
+    >
+      Save
+      {{pluralize "Variable" @this.keyValues.length}}
+    </button>
   </footer>
-</form>
+</form>
\ No newline at end of file

ui/app/components/secure-variable-form.js

@@ -12,9 +12,6 @@ export default class SecureVariableFormComponent extends Component {
   @service router;
   @service flashMessages;
 
-  @tracked
-  shouldHideValues = true;
-
   /**
    * @typedef {Object} DuplicatePathWarning
    * @property {string} path
@@ -25,10 +22,6 @@ export default class SecureVariableFormComponent extends Component {
    */
   @tracked duplicatePathWarning = null;
 
-  get valueFieldType() {
-    return this.shouldHideValues ? 'password' : 'text';
-  }
-
   get shouldDisableSave() {
     return !this.args.model?.path;
   }
@@ -68,11 +61,6 @@ export default class SecureVariableFormComponent extends Component {
     }
   }
 
-  @action
-  toggleShowHide() {
-    this.shouldHideValues = !this.shouldHideValues;
-  }
-
   @action appendRow() {
     this.keyValues.pushObject({
       key: '',

ui/app/components/secure-variable-form/input-group.hbs

+<label class="value-label">
+  <span>
+    Value
+  </span>
+  <Input
+    @type={{this.inputType}}
+    @value={{@entry.value}}
+    class="input value-input"
+    autocomplete="new-password"
+    {{! prevent auto-fill }}
+  />
+  <button
+    class="show-hide-values button is-light"
+    type="button"
+    tabindex="-1"
+    {{on "click" this.toggleInputType}}
+  >
+    <FlightIcon
+      @name={{if this.isObscured "eye-off" "eye"}}
+      @title={{if this.isObscured "Show Values" "Hide Values"}}
+    />
+  </button>
+</label>
\ No newline at end of file

ui/app/components/secure-variable-form/input-group.js

+// @ts-check
+
+import { action } from '@ember/object';
+import Component from '@glimmer/component';
+import { tracked } from '@glimmer/tracking';
+
+export default class InputGroup extends Component {
+  @tracked isObscured = true;
+
+  get inputType() {
+    return this.isObscured ? 'password' : 'text';
+  }
+
+  @action
+  toggleInputType() {
+    this.isObscured = !this.isObscured;
+  }
+}

ui/tests/integration/components/secure-variable-form-test.js

@@ -81,42 +81,52 @@ module('Integration | Component | secure-variable-form', function (hooks) {
     );
   });
 
-  test('Values can be toggled to show/hide', async function (assert) {
-    this.set(
-      'mockedModel',
-      server.create('variable', {
-        keyValues: [{ key: 'foo', value: 'bar' }],
-      })
-    );
+  module('editing and creating new key/value pairs', function () {
+    test('it should allow each key/value row to toggle password visibility', async function (assert) {
+      this.set(
+        'mockedModel',
+        server.create('variable', {
+          keyValues: [{ key: 'foo', value: 'bar' }],
+        })
+      );
 
-    assert.expect(6);
+      assert.expect(6);
 
-    await render(hbs`<SecureVariableForm @model={{this.mockedModel}} />`);
-    await click('.key-value button.add-more'); // add a second variable
+      await render(hbs`<SecureVariableForm @model={{this.mockedModel}} />`);
+      await click('.key-value button.add-more'); // add a second variable
+
+      findAll('input.value-input').forEach((input, iter) => {
+        assert.equal(
+          input.getAttribute('type'),
+          'password',
+          `Value ${iter + 1} is hidden by default`
+        );
+      });
+
+      await click('.key-value button.show-hide-values');
+      const [firstRow, secondRow] = findAll('input.value-input');
 
-    findAll('input.value-input').forEach((input, iter) => {
       assert.equal(
-        input.getAttribute('type'),
+        firstRow.getAttribute('type'),
+        'text',
+        'Only the row that is clicked on toggles visibility'
+      );
+      assert.equal(
+        secondRow.getAttribute('type'),
         'password',
-        `Value ${iter + 1} is hidden by default`
+        'Rows that are not clicked remain obscured'
       );
-    });
 
-    await click('.key-value button.show-hide-values');
-    findAll('input.value-input').forEach((input, iter) => {
+      await click('.key-value button.show-hide-values');
       assert.equal(
-        input.getAttribute('type'),
-        'text',
-        `Value ${iter + 1} is shown when toggled`
+        firstRow.getAttribute('type'),
+        'password',
+        'Only the row that is clicked on toggles visibility'
       );
-    });
-
-    await click('.key-value button.show-hide-values');
-    findAll('input.value-input').forEach((input, iter) => {
       assert.equal(
-        input.getAttribute('type'),
+        secondRow.getAttribute('type'),
         'password',
-        `Value ${iter + 1} is hidden when toggled again`
+        'Rows that are not clicked remain obscured'
       );
     });
   });