From f38dd58078bc29c06e578a4e243fa0249effb209 Mon Sep 17 00:00:00 2001
From: Jai <41024828+ChaiWithJai@users.noreply.github.com>
Date: Mon, 12 Dec 2022 10:56:31 -0500
Subject: [PATCH] ui:  move "add more" button in variable create and edit forms
 (#15434)

* refact: move conditional logic

* chore: prettify template

* test: update test for markup change

* Revert "chore: prettify template"

This reverts commit ff1e0f02eb7ea30ede2cf93e9646a339601acdbe.

* styles: add space to save button for consistency

* style:  add save selector for Submit button

* refact: conditionally render add button based on view

* Revert "test: update test for markup change"

This reverts commit 59318cde68b50aaf915be7cb9f7e332b7f0204c7.

* style: add more button wonkiness

* test: use data-test attr

* test: handle non-table view on create

* ui:  add reactive getter property to use in template

* style use grid instead of margin

Co-authored-by: Preston Bourne <preston.bourne@icloud.com>
---
 ui/app/components/variable-form.hbs           | 24 +++++++++----------
 ui/app/components/variable-form.js            |  5 ++++
 ui/app/styles/components/variables.scss       | 16 +++++++++----
 .../components/variable-form-test.js          | 22 ++++++++---------
 4 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/ui/app/components/variable-form.hbs b/ui/app/components/variable-form.hbs
index 852696195..2e8ad2aa7 100644
--- a/ui/app/components/variable-form.hbs
+++ b/ui/app/components/variable-form.hbs
@@ -110,16 +110,6 @@
           />
         </label>
         <VariableForm::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"
@@ -127,7 +117,6 @@
           >
             Delete
           </button>
-        {{/if}}
         {{#each-in entry.warnings as |k v|}}
           <span class="key-value-error help is-danger">
             {{v}}
@@ -148,9 +137,20 @@
   {{/if}}
 
   <footer>
+    {{#unless this.isJSONView}}
+    <button
+      class="add-more button is-info is-inverted"
+      type="button"
+      disabled={{not (and this.keyValues.lastObject.key this.keyValues.lastObject.value)}}
+      {{on "click" this.appendRow}}
+      data-test-add-kv
+    >
+      Add More
+    </button>
+    {{/unless}}
     <button
       disabled={{this.shouldDisableSave}}
-      class="button is-primary"
+      class="button is-primary save"
       type="submit"
       data-test-submit-var
     >
diff --git a/ui/app/components/variable-form.js b/ui/app/components/variable-form.js
index fe47f2b03..67dd7e1db 100644
--- a/ui/app/components/variable-form.js
+++ b/ui/app/components/variable-form.js
@@ -240,6 +240,11 @@ export default class VariableFormComponent extends Component {
   //#region JSON Editing
 
   view = this.args.view;
+
+  get isJSONView() {
+    return this.args.view === 'json';
+  }
+
   // Prevent duplicate onUpdate events when @view is set to its already-existing value,
   // which happens because parent's queryParams and toggle button both resolve independently.
   @action onViewChange([view]) {
diff --git a/ui/app/styles/components/variables.scss b/ui/app/styles/components/variables.scss
index 024ddfca7..b531e7baa 100644
--- a/ui/app/styles/components/variables.scss
+++ b/ui/app/styles/components/variables.scss
@@ -75,10 +75,6 @@
       border-color: $grey-blue;
     }
 
-    button.add-more[disabled] {
-      border-color: #dbdbdb;
-    }
-
     .key-value-error {
       grid-column: -1 / 1;
       position: relative;
@@ -184,3 +180,15 @@ table.variable-items {
     opacity: 1;
   }
 }
+
+footer {
+  display: grid;
+  grid-auto-columns: max-content;
+  grid-auto-flow: column;
+  gap: 1rem;
+
+  .button.is-info.is-inverted.add-more[disabled] {
+    border-color: #dbdbdb;
+    box-shadow: 0 2px 0 0 rgb(122 122 122 / 20%);
+  }
+}
diff --git a/ui/tests/integration/components/variable-form-test.js b/ui/tests/integration/components/variable-form-test.js
index d70aa74a2..627b2cf1a 100644
--- a/ui/tests/integration/components/variable-form-test.js
+++ b/ui/tests/integration/components/variable-form-test.js
@@ -47,7 +47,7 @@ module('Integration | Component | variable-form', function (hooks) {
     );
 
     assert
-      .dom('.key-value button.add-more')
+      .dom('[data-test-add-kv]')
       .isDisabled(
         'The "Add More" button is disabled until key and value are filled'
       );
@@ -55,7 +55,7 @@ module('Integration | Component | variable-form', function (hooks) {
     await typeIn('.key-value label:nth-child(1) input', 'foo');
 
     assert
-      .dom('.key-value button.add-more')
+      .dom('[data-test-add-kv]')
       .isDisabled(
         'The "Add More" button is still disabled with only key filled'
       );
@@ -63,12 +63,12 @@ module('Integration | Component | variable-form', function (hooks) {
     await typeIn('.key-value label:nth-child(2) input', 'bar');
 
     assert
-      .dom('.key-value button.add-more')
+      .dom('[data-test-add-kv]')
       .isNotDisabled(
         'The "Add More" button is no longer disabled after key and value are filled'
       );
 
-    await click('.key-value button.add-more');
+    await click('[data-test-add-kv]');
 
     assert.equal(
       findAll('div.key-value').length,
@@ -79,7 +79,7 @@ module('Integration | Component | variable-form', function (hooks) {
     await typeIn('.key-value:last-of-type label:nth-child(1) input', 'foo');
     await typeIn('.key-value:last-of-type label:nth-child(2) input', 'bar');
 
-    await click('.key-value button.add-more');
+    await click('[data-test-add-kv]');
 
     assert.equal(
       findAll('div.key-value').length,
@@ -109,7 +109,7 @@ module('Integration | Component | variable-form', function (hooks) {
       assert.expect(6);
 
       await render(hbs`<VariableForm @model={{this.mockedModel}} />`);
-      await click('.key-value button.add-more'); // add a second variable
+      await click('[data-test-add-kv]'); // add a second variable
 
       findAll('input.value-input').forEach((input, iter) => {
         assert.equal(
@@ -173,11 +173,11 @@ module('Integration | Component | variable-form', function (hooks) {
     );
     assert.equal(
       findAll('button.delete-row').length,
-      4,
-      'Shows "delete" for the first four rows'
+      5,
+      'Shows "delete" for all five rows'
     );
     assert.equal(
-      findAll('button.add-more').length,
+      findAll('[data-test-add-kv]').length,
       1,
       'Shows "add more" only on the last row'
     );
@@ -301,7 +301,7 @@ module('Integration | Component | variable-form', function (hooks) {
 
       await render(hbs`<VariableForm @model={{this.mockedModel}} />`);
 
-      await click('.key-value button.add-more');
+      await click('[data-test-add-kv]');
 
       const secondKey = document.querySelectorAll('[data-test-var-key]')[1];
       await typeIn(secondKey, 'myWonderfulKey');
@@ -380,7 +380,7 @@ module('Integration | Component | variable-form', function (hooks) {
 
       this.set('view', 'table');
 
-      await click('.key-value button.add-more');
+      await click('[data-test-add-kv]');
 
       await typeIn('.key-value:last-of-type label:nth-child(1) input', 'howdy');
       await typeIn(