ui: L7 intentions improvements (#8851)

* Disable source as well as destination on editing

* Various visual/textual amends

* Make errors only appear once you've interacted with a field

* Move tests that involve selecting menus to a create form

* Revert fieldsets and checkboxes

ui-v2/app/components/consul-intention-form/fieldsets/index.hbs

@@ -9,6 +9,7 @@
         <label data-test-source-element class="type-select{{if item.error.SourceName ' has-error'}}">
           <span>Source Service</span>
           <PowerSelectWithCreate
+              @disabled={{not create}}
               @options={{services}}
               @searchField="Name"
               @selected={{SourceName}}
@@ -23,14 +24,16 @@
                     {{service.Name}}
                   {{/if}}
           </PowerSelectWithCreate>
+        {{#if create}}
           <em>Search for an existing service, or enter any Service name.</em>
+        {{/if}}
         </label>
   {{#if (env 'CONSUL_NSPACES_ENABLED')}}
         <label data-test-source-nspace class="type-select{{if item.error.SourceNS ' has-error'}}">
           <span>Source Namespace</span>
           <PowerSelectWithCreate
+              @disabled={{not create}}
               @options={{nspaces}}
-              @searchField="Name"
               @selected={{SourceNS}}
               @searchPlaceholder="Type namespace name"
               @buildSuggestion={{action "createNewLabel" "Use a Consul Namespace called '{{term}}'"}}
@@ -43,9 +46,9 @@
                   {{nspace.Name}}
                 {{/if}}
           </PowerSelectWithCreate>
-          {{#if create}}
+        {{#if create}}
-            <em>Search for an existing namespace, or enter any Namespace name.</em>
+          <em>Search for an existing namespace, or enter any Namespace name.</em>
-          {{/if}}
+        {{/if}}
         </label>
 {{/if}}
       </fieldset>
@@ -69,9 +72,9 @@
                     {{service.Name}}
                   {{/if}}
           </PowerSelectWithCreate>
-          {{#if create}}
+        {{#if create}}
-            <em>Search for an existing service, or enter any Service name.</em>
+          <em>Search for an existing service, or enter any Service name.</em>
-          {{/if}}
+        {{/if}}
         </label>
   {{#if (env 'CONSUL_NSPACES_ENABLED')}}
         <label data-test-destination-nspace class="type-select{{if item.error.DestinationNS ' has-error'}}">

ui-v2/app/components/consul-intention-form/fieldsets/skin.scss

@@ -1,2 +1,11 @@
-%consul-intention-fieldsets {
+.consul-intention-fieldsets {
+  .value-allow > :last-child::before {
+    @extend %with-arrow-right-color-icon, %as-pseudo;
+  }
+  .value-deny > :last-child::before {
+    @extend %with-deny-color-icon, %as-pseudo;
+  }
+  .value- > :last-child::before {
+    @extend %with-layers-mask, %as-pseudo;
+  }
 }

ui-v2/app/components/consul-intention-permission-form/index.hbs

@@ -2,6 +2,10 @@
   ...attributes
   class="consul-intention-permission-form"
 >
+<FormGroup
+  @name={{name}}
+as |group|>
+
   {{yield (hash
     submit=(action 'submit' changeset)
     reset=(action 'reset' changeset)
@@ -35,35 +39,45 @@
       <h2>Path</h2>
     </header>
     <div>
-      <label class="type-select">
+        <group.Element
-        <span>Path Type</span>
+          @name="PathType"
-        <PowerSelect
+          @type="select"
-          @options={{pathTypes}}
+        as |el|>
-          @selected={{pathType}}
+          <el.Label>
-          @onChange={{action 'change' 'HTTP.PathType' changeset}} as |Type|>
+            Path type
-            {{get pathLabels Type}}
+          </el.Label>
-        </PowerSelect>
+          <PowerSelect
-      </label>
+            @options={{pathTypes}}
+            @selected={{pathType}}
+            @onChange={{action 'change' 'HTTP.PathType' changeset}} as |Type|>
+              {{get pathLabels Type}}
+          </PowerSelect>
+        </group.Element>
+
 {{#if shouldShowPathField}}
-      <label class="type-text{{if changeset.error.HTTP.Path ' has-error'}}">
+        <group.Element
-        <span>{{get pathLabels pathType}}</span>
+          @name="Path"
-        <input
+          @error={{changeset-get changeset 'error.HTTP.Path'}}
-          type="text"
+        as |el|>
-          name="Path"
+          <el.Label>
-          value={{changeset-get changeset 'HTTP.Path'}}
+            {{get pathLabels pathType}}
-          oninput={{action 'change' 'HTTP.Path' changeset}}
+          </el.Label>
-        />
+          <el.Text
-        {{#if changeset.error.HTTP.Path}}
+            @value={{changeset-get changeset 'HTTP.Path'}}
-          <strong>
+            oninput={{action 'change' 'HTTP.Path' changeset}}
-            {{#if (eq (changeset-get changeset 'HTTP.PathType') 'PathRegex')}}
+          />
-              Path Regex should not be blank
+          <State @state={{el.state}} @matches="error">
-            {{else}}
+            <el.Error>
-              Path should begin with a '/'
+              {{#if (eq (changeset-get changeset 'HTTP.Path') 'Regex')}}
-            {{/if}}
+                Path Regex should not be blank
-          </strong>
+              {{else}}
-        {{/if}}
+                Path should begin with a '/'
-      </label>
+              {{/if}}
+            </el.Error>
+          </State>
+        </group.Element>
 {{/if}}
+
     </div>
   </fieldset>
 
@@ -71,15 +85,17 @@
     <h2>Methods</h2>
     <div class="type-toggle">
       <span>All methods are applied by default unless specified</span>
-      <label class="type-checkbox">
+      <group.Element
-        <input
+        @name="allMethods"
-          type="checkbox"
+      as |el|>
-          name="{{name}}[allMethods]"
+        <el.Checkbox
           checked={{if allMethods 'checked'}}
           onchange={{action 'change' 'allMethods' changeset}}
         />
-        <span>All methods</span>
+        <el.Label>
-      </label>
+          All Methods
+        </el.Label>
+      </group.Element>
     </div>
 
 {{#if shouldShowMethods}}
@@ -102,6 +118,7 @@
 
   <fieldset>
     <h2>Headers</h2>
+
     <ConsulIntentionPermissionHeaderList
       @items={{changeset-get changeset 'HTTP.Header'}}
       @ondelete={{action 'delete' 'HTTP.Header' changeset}}
@@ -121,7 +138,7 @@
       disabled={{if (not this.headerForm.isDirty) 'disabled'}}
       onclick={{action this.headerForm.submit}}
     >
-      Add another header
+      Add{{#if (gt (get (changeset-get changeset 'HTTP.Header') 'length') 0)}} another{{/if}} header
     </button>
     <button
       type="button"
@@ -132,4 +149,5 @@
     </button>
 
   </fieldset>
+  </FormGroup>
 </div>

ui-v2/app/components/consul-intention-permission-form/skin.scss

@@ -2,5 +2,15 @@
   h2 {
     border-top: 1px solid $blue-500;
   }
+  button.type-submit {
+    @extend %frame-blue-300;
+  }
+  button.type-submit:hover:not(:disabled),
+  button.type-submit:focus:not(:disabled) {
+    @extend %frame-blue-500;
+  }
+  button.type-submit:disabled {
+    @extend %frame-blue-200;
+  }
 }
 

ui-v2/app/components/consul-intention-permission-header-form/index.hbs

@@ -2,56 +2,69 @@
   ...attributes
   class="consul-intention-permission-header-form"
 >
-  {{yield (hash
+  <FormGroup
-    submit=(action 'submit' changeset)
+    @name={{name}}
-    reset=(action 'reset' changeset)
+  as |group|>
 
-    isDirty=(and changeset.isValid changeset.isDirty)
+    {{yield (hash
-    changeset=changeset
+      submit=(action 'submit' changeset)
-  )}}
+      reset=(action 'reset' changeset)
 
-  <fieldset>
+      isDirty=(and changeset.isValid changeset.isDirty)
-    <div>
+      changeset=changeset
+    )}}
 
-      <label class="type-select">
+    <fieldset>
-        <span>Header Type</span>
+      <div>
-        <div>
+        <group.Element
+          @name="HeaderType"
+          @type="select"
+        as |el|>
+          <el.Label>Header type</el.Label>
           <PowerSelect
             @options={{headerTypes}}
             @selected={{headerType}}
             @onChange={{action 'change' 'HeaderType' changeset}} as |Type|>
               {{get headerLabels Type}}
           </PowerSelect>
-        </div>
+        </group.Element>
-      </label>
+
-      <label class="type-text{{if changeset.error.Name ' has-error'}}">
+
-        <span>Header name</span>
+        <group.Element
-        <input
+          @name="Name"
-          type="text"
+          @error={{changeset-get changeset 'error.Name'}}
-          name={{concat name '[Name]'}}
+        as |el|>
-          value={{changeset-get changeset 'Name'}}
+          <el.Label>Header name</el.Label>
-          oninput={{action 'change' 'Name' changeset}}
+          <el.Text
-        />
+            @value={{changeset-get changeset 'Name'}}
-        {{#if changeset.error.Name}}
+            oninput={{action 'change' 'Name' changeset}}
-          <strong>{{changeset.error.Name.validation}}</strong>
+          />
-        {{/if}}
+          <State @state={{el.state}} @matches="error">
-      </label>
+            <el.Error>
+              {{changeset-get changeset 'error.Name.validation'}}
+            </el.Error>
+          </State>
+        </group.Element>
 
   {{#if shouldShowValueField}}
-      <label class="type-text{{if changeset.error.Value ' has-error'}}">
+        <group.Element
-        <span>Header {{lowercase (get headerLabels headerType)}}</span>
+          @name="Value"
-        <input
+          @error={{changeset-get changeset 'error.Value'}}
-          type="text"
+        as |el|>
-          name="Value"
+          <el.Label>Header {{lowercase (get headerLabels headerType)}}</el.Label>
-          value={{changeset-get changeset 'Value'}}
+          <el.Text
-          oninput={{action 'change' 'Value' changeset}}
+            @value={{changeset-get changeset 'Value'}}
-        />
+            oninput={{action 'change' 'Value' changeset}}
-        {{#if changeset.error.Value}}
+          />
-          <strong>{{changeset.error.Value.validation}}</strong>
+          <State @state={{el.state}} @matches="error">
-        {{/if}}
+            <el.Error>
-      </label>
+              {{changeset-get changeset 'error.Value.validation'}}
+            </el.Error>
+          </State>
+        </group.Element>
   {{/if}}
 
-    </div>
+      </div>
-  </fieldset>
+    </fieldset>
+  </FormGroup>
 </div>

ui-v2/app/components/consul-intention-permission-list/index.hbs

@@ -3,7 +3,6 @@
   class="consul-intention-permission-list{{if (not onclick) ' readonly'}}"
   @scroll="native"
   @items={{items}}
-  @cellHeight={{42}}
 as |item|>
   <BlockSlot @name="details">
     <div onclick={{action (optional onclick) item}}>

ui-v2/app/components/consul-intention-permission-list/index.scss

@@ -1,24 +1,5 @@
 @import './skin';
 @import './layout';
-%list-row-200 {
-  @extend %list-row;
-  padding-top: 0 !important;
-  padding-bottom: 0 !important;
-}
-%list-row-200 .detail {
-  grid-row-start: header !important;
-  grid-row-end: detail !important;
-  align-self: center !important;
-}
-%list-row-200 .popover-menu > [type="checkbox"] + label {
-  padding: 0;
-}
-%list-row-200 .popover-menu > [type="checkbox"] + label + div:not(.above) {
-  top: 30px;
-}
-%list-row-200 dd {
-  @extend %p2;
-}
 .consul-intention-permission-list > ul > li {
   @extend %list-row-200;
 }

ui-v2/app/components/form-group/element/checkbox/index.hbs

@@ -0,0 +1,8 @@
+<input
+  {{did-insert (optional @didinsert)}}
+  {{on 'change' (optional @onchange)}}
+  type="checkbox"
+  name={{@name}}
+  value={{@value}}
+   ...attributes
+/>

ui-v2/app/components/form-group/element/error/index.hbs

@@ -0,0 +1,6 @@
+<strong
+  role="alert"
+  ...attributes
+>
+  {{yield}}
+</strong>

ui-v2/app/components/form-group/element/index.hbs

@@ -0,0 +1,33 @@
+{{#let (hash
+
+  Element=(component 'form-group/element' group=@group name=@name)
+
+  Text=(component 'form-group/element/text' didinsert=(action this.connect) name=this.name oninput=(action (mut this.touched) true))
+  Checkbox=(component 'form-group/element/checkbox' didinsert=(action this.connect) name=this.name onchange=(action (mut this.touched) true))
+  Radio=(component 'form-group/element/radio' didinsert=(action this.connect) name=this.name onchange=(action (mut this.touched) true))
+
+
+  Label=(component 'form-group/element/label')
+  Error=(component 'form-group/element/error')
+
+  state=state
+)
+as |el|}}
+  {{#if (contains this.type (array 'radiogroup' 'checkbox-group' 'checkboxgroup'))}}
+    <div
+      data-property={{this.prop}}
+      class="type-{{this.type}}{{if (state-matches state 'error') ' has-error'}}"
+      ...attributes
+    >
+      {{yield el}}
+    </div>
+  {{else}}
+    <label
+      data-property={{this.prop}}
+      class="type-{{this.type}}{{if (state-matches state 'error') ' has-error'}}"
+      ...attributes
+    >
+      {{yield el}}
+    </label>
+  {{/if}}
+{{/let}}

ui-v2/app/components/form-group/element/index.js

@@ -0,0 +1,37 @@
+import Component from '@glimmer/component';
+import { tracked } from '@glimmer/tracking';
+import { action } from '@ember/object';
+
+export default class Element extends Component {
+  @tracked el;
+
+  @tracked touched = false;
+
+  get type() {
+    if (typeof this.el !== 'undefined') {
+      return this.el.dataset.type || this.el.getAttribute('type') || this.el.getAttribute('role');
+    }
+    return this.args.type;
+  }
+  get name() {
+    if (typeof this.args.group !== 'undefined') {
+      return `${this.args.group.name}[${this.args.name}]`;
+    } else {
+      return this.args.name;
+    }
+  }
+  get prop() {
+    return `${this.args.name.toLowerCase().replaceAll('.', '-')}`;
+  }
+  get state() {
+    const error = this.touched && this.args.error;
+    return {
+      matches: name => name === 'error' && error,
+    };
+  }
+
+  @action
+  connect($el) {
+    this.el = $el;
+  }
+}

ui-v2/app/components/form-group/element/label/index.hbs

@@ -0,0 +1,6 @@
+<span
+  class="form-elements-label label"
+  ...attributes
+>
+  {{yield}}
+</span>

ui-v2/app/components/form-group/element/radio/index.hbs

@@ -0,0 +1,8 @@
+<input
+  {{did-insert (optional @didinsert)}}
+  {{on 'change' (optional @onchange)}}
+  type="radio"
+  name={{@name}}
+  value={{@value}}
+   ...attributes
+/>

ui-v2/app/components/form-group/element/text/index.hbs

@@ -0,0 +1,8 @@
+<input
+  {{did-insert (optional @didinsert)}}
+  {{on 'input' (optional @oninput)}}
+  type="text"
+  name={{@name}}
+  value={{@value}}
+   ...attributes
+/>

ui-v2/app/components/form-group/index.hbs

@@ -0,0 +1,3 @@
+{{yield (hash
+  Element=(component 'form-group/element' group=this)
+)}}

ui-v2/app/components/form-group/index.js

@@ -0,0 +1,7 @@
+import Component from '@glimmer/component';
+
+export default class FormGroup extends Component {
+  get name() {
+    return this.args.name;
+  }
+}

ui-v2/app/styles/base/color/frame-placeholders.scss

@@ -107,6 +107,18 @@
   border-color: $green-800;
   color: $white;
 }
+%frame-blue-200 {
+  @extend %frame-border-000;
+  background-color: $white;
+  border-color: $blue-300;
+  color: $blue-300;
+}
+%frame-blue-300 {
+  @extend %frame-border-000;
+  background-color: $white;
+  border-color: $blue-500;
+  color: $blue-500;
+}
 %frame-blue-500 {
   @extend %frame-border-000;
   background-color: $blue-050;

ui-v2/app/styles/components/list-row/layout.scss

@@ -30,3 +30,23 @@
 %list-row-detail > span {
   margin-right: 18px;
 }
+%list-row-200 {
+  @extend %list-row;
+  padding-top: 0 !important;
+  padding-bottom: 0 !important;
+}
+%list-row-200 .detail {
+  grid-row-start: header !important;
+  grid-row-end: detail !important;
+  align-self: center !important;
+  padding: 5px 0;
+}
+%list-row-200 .popover-menu > [type='checkbox'] + label {
+  padding: 0;
+}
+%list-row-200 .popover-menu > [type='checkbox'] + label + div:not(.above) {
+  top: 30px;
+}
+%list-row-200 dd {
+  @extend %p2;
+}

ui-v2/app/styles/routes.scss

@@ -3,3 +3,4 @@
 @import 'routes/dc/nodes/index';
 @import 'routes/dc/kv/index';
 @import 'routes/dc/acls/index';
+@import 'routes/dc/intentions/index';

ui-v2/app/styles/routes/dc/intentions/index.scss

@@ -0,0 +1,3 @@
+html[data-route^='dc.intentions.edit'] .definition-table {
+  margin-bottom: 1em;
+}

ui-v2/tests/acceptance/dc/intentions/filtered-select.feature

@@ -16,17 +16,11 @@ Feature: dc / intentions / filtered-select: Intention Service Select Dropdowns
     - Name: service-3
       Kind: connect-proxy
     ---
-    And 1 intention model from yaml
-    ---
-    SourceName: 'service-0'
-    DestinationName: 'service-1'
-    ---
     When I visit the intention page for yaml
     ---
       dc: datacenter
-      intention: intention
     ---
-    Then the url should be /datacenter/intentions/intention
+    Then the url should be /datacenter/intentions/create
     And I click "[data-test-[Name]-element] .ember-power-select-trigger"
     Then I see the text "* (All Services)" in ".ember-power-select-option:nth-last-child(3)"
     Then I see the text "service-0" in ".ember-power-select-option:nth-last-child(2)"
@@ -35,7 +29,7 @@ Feature: dc / intentions / filtered-select: Intention Service Select Dropdowns
       ---------------
       | Name        |
       | source      |
-      #| destination |
+      | destination |
       ---------------
   Scenario: Opening the [Name] dropdown with 2 services with the same name from different nspaces
     Given 1 datacenter model with the value "datacenter"
@@ -47,17 +41,11 @@ Feature: dc / intentions / filtered-select: Intention Service Select Dropdowns
       Namespace: nspace
       Kind: ~
     ---
-    And 1 intention model from yaml
-    ---
-    SourceName: 'service-0'
-    DestinationName: 'service-0'
-    ---
     When I visit the intention page for yaml
     ---
       dc: datacenter
-      intention: intention
     ---
-    Then the url should be /datacenter/intentions/intention
+    Then the url should be /datacenter/intentions/create
     And I click "[data-test-[Name]-element] .ember-power-select-trigger"
     Then I see the text "* (All Services)" in ".ember-power-select-option:nth-last-child(2)"
     Then I see the text "service-0" in ".ember-power-select-option:last-child"
@@ -65,5 +53,5 @@ Feature: dc / intentions / filtered-select: Intention Service Select Dropdowns
       ---------------
       | Name        |
       | source      |
-      #| destination |
+      | destination |
       ---------------

ui-v2/tests/acceptance/dc/intentions/form-select.feature

@@ -8,9 +8,8 @@ Feature: dc / intentions / form-select: Intention Service Select Dropdowns
     When I visit the intention page for yaml
     ---
       dc: datacenter
-      intention: intention
     ---
-    Then the url should be /datacenter/intentions/intention
+    Then the url should be /datacenter/intentions/create
     And I click "[data-test-[Name]-element] .ember-power-select-trigger"
     And I type "something" into ".ember-power-select-search-input"
     And I click ".ember-power-select-option:first-child"
@@ -19,5 +18,5 @@ Feature: dc / intentions / form-select: Intention Service Select Dropdowns
       ---------------
       | Name        |
       | source      |
-      # | destination |
+      | destination |
       ---------------