diff --git a/ui/app/components/variable-form.hbs b/ui/app/components/variable-form.hbs index 2c4a98a61..cab126f62 100644 --- a/ui/app/components/variable-form.hbs +++ b/ui/app/components/variable-form.hbs @@ -50,22 +50,21 @@ {{/if}}
- + + + - + + Key + - + {{on "click" (action this.deleteRow entry)}} + /> {{#each-in entry.warnings as |k v|}} {{v}} @@ -166,25 +154,23 @@
{{#unless this.isJSONView}} {{#unless this.isJobTemplateVariable}} - + /> {{/unless}} {{/unless}} - + />
\ No newline at end of file diff --git a/ui/app/components/variable-form.js b/ui/app/components/variable-form.js index b64614d18..0fd3e84f1 100644 --- a/ui/app/components/variable-form.js +++ b/ui/app/components/variable-form.js @@ -180,6 +180,7 @@ export default class VariableFormComponent extends Component { delete entry.warnings.duplicateKeyError; entry.warnings.notifyPropertyChange('duplicateKeyError'); } + set(entry, 'key', value); } @action appendRow() { @@ -207,6 +208,7 @@ export default class VariableFormComponent extends Component { * @param {KeyboardEvent} e */ @action setModelPath(e) { + set(this, 'path', e.target.value); set(this.args.model, 'path', e.target.value); } diff --git a/ui/app/components/variable-form/input-group.hbs b/ui/app/components/variable-form/input-group.hbs index 67460ad9b..66a1f4e13 100644 --- a/ui/app/components/variable-form/input-group.hbs +++ b/ui/app/components/variable-form/input-group.hbs @@ -4,26 +4,15 @@ ~}} \ No newline at end of file diff --git a/ui/app/components/variable-form/namespace-filter.hbs b/ui/app/components/variable-form/namespace-filter.hbs index 53031aec8..15d4e2e35 100644 --- a/ui/app/components/variable-form/namespace-filter.hbs +++ b/ui/app/components/variable-form/namespace-filter.hbs @@ -10,19 +10,18 @@ {{#if trigger.data.isSuccess}} {{#if trigger.data.result}} {{#if @data.namespaceOptions}} - + + + {{#each @data.namespaceOptions as |option|}} + + {{option.label}} + + {{/each}} + {{/if}} {{/if}} {{/if}} diff --git a/ui/app/components/variable-form/related-entities.hbs b/ui/app/components/variable-form/related-entities.hbs index 0885641da..9c33a18d3 100644 --- a/ui/app/components/variable-form/related-entities.hbs +++ b/ui/app/components/variable-form/related-entities.hbs @@ -3,18 +3,18 @@ SPDX-License-Identifier: BUSL-1.1 ~}} -

- - + + Automatically-accessible variable + This variable {{#if @new}}will be{{else}}is{{/if}} accessible by {{#if @task}} - task {{@task}} in group {{@group}} + task {{@task}} in group {{@group}} {{else if @group}} - group {{@group}} + group {{@group}} {{else if @job}} - job {{@job}} + job {{@job}} {{else}} - all nomad jobs in this namespace - {{/if}} - -

+ all nomad jobs in this namespace + {{/if}} + + diff --git a/ui/app/components/variable-paths.hbs b/ui/app/components/variable-paths.hbs index 3016a2a88..4b00fc46f 100644 --- a/ui/app/components/variable-paths.hbs +++ b/ui/app/components/variable-paths.hbs @@ -3,22 +3,24 @@ SPDX-License-Identifier: BUSL-1.1 ~}} - - - - Path - - - Namespace - - - Last Modified - - - + + <:head as |H|> + + + Path + + + Namespace + + + Last Modified + + + + <:body as |B|> {{#each this.folders as |folder|}} - - + - - + + + {{/each}} {{#each this.files as |file|}} - - + {{#if (can "read variable" path=file.absoluteFilePath namespace=file.variable.namespace)}} {{file.name}} {{/if}} - - + + {{file.variable.namespace}} - - + + {{moment-from-now file.variable.modifyTime}} - - - {{/each}} - - - + + + {{/each}} + + diff --git a/ui/app/controllers/variables/variable/index.js b/ui/app/controllers/variables/variable/index.js index b9a89f382..b2b609092 100644 --- a/ui/app/controllers/variables/variable/index.js +++ b/ui/app/controllers/variables/variable/index.js @@ -37,6 +37,10 @@ export default class VariablesVariableIndexController extends Controller { this.isDeleting = false; } + @action copyVariable() { + navigator.clipboard.writeText(JSON.stringify(this.model.items, null, 2)); + } + @task(function* () { try { yield this.model.deleteRecord(); diff --git a/ui/app/styles/components/variables.scss b/ui/app/styles/components/variables.scss index b0c5c9810..2e3419871 100644 --- a/ui/app/styles/components/variables.scss +++ b/ui/app/styles/components/variables.scss @@ -5,28 +5,18 @@ .section.single-variable { margin-top: 1.5rem; - - .back-link { - text-decoration: none; - color: #363636; - position: relative; - top: 4px; - } } +$hdsLabelTopOffset: 26px; +$hdsInputHeight: 35px; + .variable-title { - .toggle { - font-size: 0.8rem; - margin-left: 1rem; - position: relative; - top: -0.25rem; - .toggler { - margin-right: 0.25rem; - } + margin-bottom: 2rem; + .hds-page-header__main { + flex-direction: unset; } - .copy-button { - position: relative; - top: 3px; + .copy-variable span { + color: var(--token-color-foreground-primary); } } @@ -35,18 +25,6 @@ margin-bottom: 1rem; } - .path-input { - height: 2.25em; - - &:disabled { - background-color: #f5f5f5; - } - &.error { - color: $red; - border-color: $red; - } - } - .duplicate-path-error { position: relative; animation: slide-in 0.3s ease-out; @@ -56,13 +34,21 @@ display: grid; grid-template-columns: 6fr 1fr; gap: 0 1rem; + align-items: start; + .namespace-dropdown { + white-space: nowrap; + width: auto; + position: relative; + top: $hdsLabelTopOffset; + height: $hdsInputHeight; + } } .key-value { display: grid; grid-template-columns: 1fr 4fr 130px; gap: 0 1rem; - align-items: end; + align-items: start; input.error { color: $red; @@ -77,6 +63,12 @@ } } + .delete-entry-button { + position: relative; + top: $hdsLabelTopOffset; + height: $hdsInputHeight; + } + button.show-hide-values { height: 100%; box-shadow: none; @@ -131,11 +123,6 @@ 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%); - } } } @@ -152,20 +139,8 @@ table.path-tree { } } -.section .notification.related-entities { - --blue: #1563ff; - display: flex; - align-items: center; - gap: 0.5rem; - &.notification { - align-items: center; - } - a { - color: $blue; - display: inline-flex; - align-items: center; - gap: 0.25rem; - } +.related-entities { + margin-bottom: 2rem; } .related-entities-hint { @@ -178,25 +153,6 @@ table.path-tree { } } -.job-template-hint { - margin-top: 0.5rem; - code { - background-color: #eee; - padding: 0.25rem; - } - .copy-button { - display: inline-block; - padding-left: 0; - position: relative; - top: -5px; - button, - .button { - background-color: transparent; - padding-right: 0.25rem; - } - } -} - table.variable-items { // table-layout: fixed; td.value-cell { diff --git a/ui/app/templates/variables/index.hbs b/ui/app/templates/variables/index.hbs index 1df15c4f4..4cebdb013 100644 --- a/ui/app/templates/variables/index.hbs +++ b/ui/app/templates/variables/index.hbs @@ -5,46 +5,50 @@ {{page-title "Variables"}}
-
-
+ + + {{#if this.namespaceOptions}} - + + {{#each this.namespaceOptions as |option|}} + + {{option.label}} + + {{/each}} + + {{/if}} + + {{#if (can "write variable" path="*" namespace=this.namespaceSelection)}} +
+ +
+ {{else}} + {{/if}} -
- {{#if (can "write variable" path="*" namespace=this.namespaceSelection)}} - - Create Variable - - {{else}} - - {{/if}} + + -
-
-
{{#if this.isForbidden}} {{else}} diff --git a/ui/app/templates/variables/new.hbs b/ui/app/templates/variables/new.hbs index 6b60fbd11..1891c2388 100644 --- a/ui/app/templates/variables/new.hbs +++ b/ui/app/templates/variables/new.hbs @@ -7,15 +7,24 @@
-

- Create a Variable - JSON -

+ + Create a Variable + + + JSON + + + {{/each}}
-
-
- {{#if this.namespaceOptions}} - - {{/if}} -
+ + /{{this.absolutePath}} + + {{#if this.namespaceOptions}} + + + {{#each this.namespaceOptions as |option|}} + + {{option.label}} + + {{/each}} + + {{/if}} + {{#if (can "write variable" path=(concat this.absolutePath "/") namespace=this.namespaceSelection)}} - - Create Variable - + +
{{else}} - + /> {{/if}} -
-
-
+ + {{#if this.isForbidden}} {{else}} diff --git a/ui/app/templates/variables/variable/edit.hbs b/ui/app/templates/variables/variable/edit.hbs index 0249a1cdb..a4deca70b 100644 --- a/ui/app/templates/variables/variable/edit.hbs +++ b/ui/app/templates/variables/variable/edit.hbs @@ -4,25 +4,30 @@ ~}} {{page-title "Edit Variable"}} - -

- - - - Edit - {{this.model.path}} - JSON - -

+ + Editing {{this.model.path}} + + + + JSON + + + + + + + + -
- - {{this.model.path}} - + + {{this.model.path}} + + + {{#unless this.isDeleting}} - JSON -
-
- {{#unless this.isDeleting}} + checked={{eq this.view "json"}} + data-test-json-toggle + {{on "change" (action this.toggleView)}} + as |F|> + JSON + + +
+ +
+ {{#if (can "write variable" path=this.model.path namespace=this.model.namespace)}} -
- - Edit - -
+ data-test-edit-button + {{autofocus}} + /> {{/if}} {{/unless}} + {{#if (can "destroy variable" path=this.model.path namespace=this.model.namespace)}} {{/if}} -
- + + {{#if this.shouldShowLinkedEntities}}
Key/Value Data -
{{else}} - - - Key - Value - - - - - {{row.model.key}} - - + + <:body as |B|> + + {{B.data.key}} +
- {{#if row.model.isVisible}} - {{row.model.value}} + {{#if B.data.isVisible}} + {{B.data.value}} {{else}} ******** {{/if}}
- - - - + + + + + {{/if}} diff --git a/ui/tests/acceptance/variables-test.js b/ui/tests/acceptance/variables-test.js index 7b33235b8..9b8634730 100644 --- a/ui/tests/acceptance/variables-test.js +++ b/ui/tests/acceptance/variables-test.js @@ -13,10 +13,7 @@ import { visit, } from '@ember/test-helpers'; import { setupMirage } from 'ember-cli-mirage/test-support'; -import { - selectChoose, - clickTrigger, -} from 'ember-power-select/test-support/helpers'; +import { clickToggle, clickOption } from 'nomad-ui/tests/helpers/helios'; import { setupApplicationTest } from 'ember-qunit'; import { module, test } from 'qunit'; import a11yAudit from 'nomad-ui/tests/helpers/a11y-audit'; @@ -365,14 +362,13 @@ module('Acceptance | variables', function (hooks) { window.localStorage.nomadTokenSecret = server.db.tokens[0].secretId; await Variables.visitNew(); assert.equal(currentURL(), '/variables/new'); - await typeIn('.path-input', 'foo/bar'); + await typeIn('[data-test-path-input]', 'foo/bar'); await click('button[type="submit"]'); assert.dom('.flash-message.alert-critical').exists(); await click('.flash-message.alert-critical .hds-dismiss-button'); assert.dom('.flash-message.alert-critical').doesNotExist(); - - await typeIn('.key-value label:nth-child(1) input', 'myKey'); - await typeIn('.key-value label:nth-child(2) input', 'superSecret'); + await typeIn('[data-test-var-key]', 'myKey'); + await typeIn('[data-test-var-value]', 'superSecret'); await percySnapshot(assert); @@ -412,9 +408,12 @@ module('Acceptance | variables', function (hooks) { assert.equal(currentRouteName(), 'variables.new'); await typeIn('[data-test-path-input]', 'foo/bar'); - await clickTrigger('[data-test-variable-namespace-filter]'); - - assert.dom('.dropdown-options').exists('Namespace can be edited.'); + await clickToggle('[data-test-variable-namespace-filter]'); + assert + .dom( + '[data-test-variable-namespace-filter] .hds-menu-primitive__content' + ) + .exists('Namespace can be edited.'); assert .dom('[data-test-variable-namespace-filter]') .containsText( @@ -422,10 +421,7 @@ module('Acceptance | variables', function (hooks) { 'The first alphabetically sorted namespace should be selected as the default option.' ); - await selectChoose( - '[data-test-variable-namespace-filter]', - 'namespace-1' - ); + await clickOption('[data-test-variable-namespace-filter]', 'namespace-1'); await typeIn('[data-test-var-key]', 'kiki'); await typeIn('[data-test-var-value]', 'do you love me'); await click('[data-test-submit-var]'); @@ -555,7 +551,7 @@ module('Acceptance | variables', function (hooks) { .dom('.related-entities-hint') .doesNotExist('Hides the hint when editing a job template variable'); assert - .dom('.job-template-hint') + .dom('[data-test-job-template-hint]') .exists('Shows a hint about job templates'); assert .dom('.CodeMirror') @@ -574,7 +570,7 @@ module('Acceptance | variables', function (hooks) { module('edit flow', function () { test('allows a user with correct permissions to edit a variable', async function (assert) { - assert.expect(8); + assert.expect(7); // Arrange Test Set-up allScenarios.variableTestCluster(server); server.createList('variable', 3); @@ -603,10 +599,6 @@ module('Acceptance | variables', function (hooks) { await percySnapshot(assert); assert.dom('[data-test-path-input]').isDisabled('Path cannot be edited'); - await clickTrigger('[data-test-variable-namespace-filter]'); - assert - .dom('.dropdown-options') - .doesNotExist('Namespace cannot be edited.'); document.querySelector('[data-test-var-key]').value = ''; // clear current input await typeIn('[data-test-var-key]', 'kiki'); @@ -884,8 +876,8 @@ module('Acceptance | variables', function (hooks) { }); // Act - await clickTrigger('[data-test-variable-namespace-filter]'); - await selectChoose('[data-test-variable-namespace-filter]', 'default'); + await clickToggle('[data-test-variable-namespace-filter]'); + await clickOption('[data-test-variable-namespace-filter]', 'default'); assert .dom('[data-test-no-matching-variables-list-headline]') @@ -946,8 +938,8 @@ module('Acceptance | variables', function (hooks) { }); // Act - await clickTrigger('[data-test-variable-namespace-filter]'); - await selectChoose('[data-test-variable-namespace-filter]', 'default'); + await clickToggle('[data-test-variable-namespace-filter]'); + await clickOption('[data-test-variable-namespace-filter]', 'default'); assert .dom('[data-test-no-matching-variables-list-headline]') diff --git a/ui/tests/helpers/helios.js b/ui/tests/helpers/helios.js new file mode 100644 index 000000000..06898bbf3 --- /dev/null +++ b/ui/tests/helpers/helios.js @@ -0,0 +1,38 @@ +/** + * Copyright (c) HashiCorp, Inc. + * SPDX-License-Identifier: BUSL-1.1 + */ + +// @ts-check + +import { + click, + // fillIn, + // triggerKeyEvent, + // triggerEvent, +} from '@ember/test-helpers'; + +/** + * @param {string} scope + * @param {*} options + */ +export async function clickToggle(scope, options) { + let selector = '.hds-dropdown-toggle-button'; + if (scope) { + selector = `${scope} ${selector}`; + } + return click(selector, options); +} + +/** + * @param {string} scope + * @param {string} option the name of the option to click + * @param {*} options + */ +export async function clickOption(scope, option, options) { + let selector = `.hds-dropdown-list-item label input[name="${option}"]`; + if (scope) { + selector = `${scope} ${selector}`; + } + return click(selector, options); +} diff --git a/ui/tests/integration/components/variable-form-test.js b/ui/tests/integration/components/variable-form-test.js index 1708c7aff..b14cdeb18 100644 --- a/ui/tests/integration/components/variable-form-test.js +++ b/ui/tests/integration/components/variable-form-test.js @@ -12,10 +12,8 @@ import { setupMirage } from 'ember-cli-mirage/test-support'; import setupCodeMirror from 'nomad-ui/tests/helpers/codemirror'; import { codeFillable, code } from 'nomad-ui/tests/pages/helpers/codemirror'; import percySnapshot from '@percy/ember'; -import { - selectChoose, - clickTrigger, -} from 'ember-power-select/test-support/helpers'; +import { clickToggle, clickOption } from 'nomad-ui/tests/helpers/helios'; + import faker from 'nomad-ui/mirage/faker'; module('Integration | Component | variable-form', function (hooks) { @@ -57,7 +55,7 @@ module('Integration | Component | variable-form', function (hooks) { 'The "Add More" button is disabled until key and value are filled' ); - await typeIn('.key-value label:nth-child(1) input', 'foo'); + await typeIn('[data-test-var-key]', 'foo'); assert .dom('[data-test-add-kv]') @@ -65,7 +63,7 @@ module('Integration | Component | variable-form', function (hooks) { 'The "Add More" button is still disabled with only key filled' ); - await typeIn('.key-value label:nth-child(2) input', 'bar'); + await typeIn('[data-test-var-value]', 'bar'); assert .dom('[data-test-add-kv]') @@ -81,9 +79,8 @@ module('Integration | Component | variable-form', function (hooks) { 'A second KV row exists after adding a new one' ); - 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 typeIn('.key-value:last-of-type [data-test-var-key]', 'foo'); + await typeIn('.key-value:last-of-type [data-test-var-value]', 'bar'); await click('[data-test-add-kv]'); assert.equal( @@ -92,7 +89,7 @@ module('Integration | Component | variable-form', function (hooks) { 'A third KV row exists after adding a new one' ); - await click('.key-value button.delete-row'); + await click('.delete-entry-button'); assert.equal( findAll('div.key-value').length, @@ -116,37 +113,33 @@ module('Integration | Component | variable-form', function (hooks) { await render(hbs``); await click('[data-test-add-kv]'); // add a second variable - findAll('input.value-input').forEach((input, iter) => { - assert.equal( - input.getAttribute('type'), - 'password', + findAll('.value-label').forEach((label, iter) => { + const maskedInput = label.querySelector('.hds-form-masked-input'); + assert.ok( + maskedInput.classList.contains('hds-form-masked-input--is-masked'), `Value ${iter + 1} is hidden by default` ); }); - await click('.key-value button.show-hide-values'); - const [firstRow, secondRow] = findAll('input.value-input'); + await click('.hds-form-visibility-toggle'); + const [firstRow, secondRow] = findAll('.hds-form-masked-input'); - assert.equal( - firstRow.getAttribute('type'), - 'text', + assert.ok( + firstRow.classList.contains('hds-form-masked-input--is-not-masked'), 'Only the row that is clicked on toggles visibility' ); - assert.equal( - secondRow.getAttribute('type'), - 'password', + assert.ok( + secondRow.classList.contains('hds-form-masked-input--is-masked'), 'Rows that are not clicked remain obscured' ); - await click('.key-value button.show-hide-values'); - assert.equal( - firstRow.getAttribute('type'), - 'password', + await click('.hds-form-visibility-toggle'); + assert.ok( + firstRow.classList.contains('hds-form-masked-input--is-masked'), 'Only the row that is clicked on toggles visibility' ); - assert.equal( - secondRow.getAttribute('type'), - 'password', + assert.ok( + secondRow.classList.contains('hds-form-masked-input--is-masked'), 'Rows that are not clicked remain obscured' ); await percySnapshot(assert); @@ -177,7 +170,7 @@ module('Integration | Component | variable-form', function (hooks) { 'Shows 5 existing key values' ); assert.equal( - findAll('button.delete-row').length, + findAll('.delete-entry-button').length, 5, 'Shows "delete" for all five rows' ); @@ -189,13 +182,13 @@ module('Integration | Component | variable-form', function (hooks) { findAll('div.key-value').forEach((row, idx) => { assert.equal( - row.querySelector(`label:nth-child(1) input`).value, + row.querySelector(`[data-test-var-key]`).value, keyValues[idx].key, `Key ${idx + 1} is correct` ); assert.equal( - row.querySelector(`label:nth-child(2) input`).value, + row.querySelector(`[data-test-var-value]`).value, keyValues[idx].value, keyValues[idx].value ); @@ -214,9 +207,9 @@ module('Integration | Component | variable-form', function (hooks) { variable.isNew = false; this.set('variable', variable); await render(hbs``); - assert.dom('input.path-input').hasValue('/baz/bat', 'Path is set'); + assert.dom('[data-test-path-input]').hasValue('/baz/bat', 'Path is set'); assert - .dom('input.path-input') + .dom('[data-test-path-input]') .isDisabled('Existing variable is in disabled state'); variable.isNew = true; @@ -224,7 +217,7 @@ module('Integration | Component | variable-form', function (hooks) { this.set('variable', variable); await render(hbs``); assert - .dom('input.path-input') + .dom('[data-test-path-input]') .isNotDisabled('New variable is not in disabled state'); }); @@ -254,27 +247,36 @@ module('Integration | Component | variable-form', function (hooks) { hbs`` ); - await typeIn('.path-input', 'foo/bar'); - assert.dom('.duplicate-path-error').doesNotExist(); - assert.dom('.path-input').doesNotHaveClass('error'); + await typeIn('[data-test-path-input]', 'foo/bar'); + assert.dom('[data-test-duplicate-variable-error]').doesNotExist(); + assert + .dom('[data-test-path-input]') + .doesNotHaveClass('hds-form-text-input--is-invalid'); - document.querySelector('.path-input').value = ''; // clear current input - await typeIn('.path-input', 'baz/bat'); - assert.dom('.duplicate-path-error').exists(); - assert.dom('.path-input').hasClass('error'); + document.querySelector('[data-test-path-input]').value = ''; // clear current input + await typeIn('[data-test-path-input]', 'baz/bat'); - await clickTrigger('[data-test-variable-namespace-filter]'); - await selectChoose( + assert.dom('[data-test-duplicate-variable-error]').exists(); + assert + .dom('[data-test-path-input]') + .hasClass('hds-form-text-input--is-invalid'); + + await clickToggle('[data-test-variable-namespace-filter]'); + await clickOption( '[data-test-variable-namespace-filter]', server.db.namespaces[2].id ); - assert.dom('.duplicate-path-error').doesNotExist(); - assert.dom('.path-input').doesNotHaveClass('error'); + assert.dom('[data-test-duplicate-variable-error]').doesNotExist(); + assert + .dom('[data-test-path-input]') + .doesNotHaveClass('hds-form-text-input--is-invalid'); - document.querySelector('.path-input').value = ''; // clear current input - await typeIn('.path-input', 'baz/bat/qux'); - assert.dom('.duplicate-path-error').exists(); - assert.dom('.path-input').hasClass('error'); + document.querySelector('[data-test-path-input]').value = ''; // clear current input + await typeIn('[data-test-path-input]', 'baz/bat/qux'); + assert.dom('[data-test-duplicate-variable-error]').exists(); + assert + .dom('[data-test-path-input]') + .hasClass('hds-form-text-input--is-invalid'); }); test('warns you when you set a key with . in it', async function (assert) { @@ -424,11 +426,8 @@ module('Integration | Component | variable-form', function (hooks) { await click('[data-test-add-kv]'); - await typeIn('.key-value:last-of-type label:nth-child(1) input', 'howdy'); - await typeIn( - '.key-value:last-of-type label:nth-child(2) input', - 'partner' - ); + await typeIn('.key-value:last-of-type [data-test-var-key]', 'howdy'); + await typeIn('.key-value:last-of-type [data-test-var-value]', 'partner'); this.set('view', 'json'); @@ -459,13 +458,13 @@ module('Integration | Component | variable-form', function (hooks) { ); this.set('view', 'table'); assert.equal( - find(`.key-value:last-of-type label:nth-child(1) input`).value, + find(`.key-value:last-of-type [data-test-var-key]`).value, 'golden', 'Key persists from JSON to Table' ); assert.equal( - find(`.key-value:last-of-type label:nth-child(2) input`).value, + find(`.key-value:last-of-type [data-test-var-value]`).value, 'gate', 'Value persists from JSON to Table' );