From 5a67cccd2c41b342038eed1f082227d023679fae Mon Sep 17 00:00:00 2001
From: John Cowen <johncowen@users.noreply.github.com>
Date: Tue, 16 Nov 2021 15:36:54 +0000
Subject: [PATCH] ui: Upstream CSS Tweaks (#11554)

Various CSS tweaks/HTML cleanup around upstreams (but impacts other 'rows')

- Prefer {{tooltip}} to <Tooltip>
- Removed some now unnecessary spans
- Stop using an empty class="" for styling purposes.
- Renamed any classes used to identify response properties to follow the exact property name but kebab-cased.
- Fixed up the alignment of things in the rows when used with a 'tiny copy button' (see screengrab) which was minus positioning and knocking things out (pending a proper refactor of our copy button CSS which is from the very very start of things)
---
 .../consul/upstream-instance/list/index.hbs   | 89 +++++++++----------
 .../consul/upstream-instance/list/index.scss  | 13 ++-
 .../components/consul/upstream/list/index.hbs | 57 ++++++------
 .../app/components/copy-button/index.scss     | 14 +++
 .../components/definition-table/layout.scss   |  1 +
 .../app/components/form-elements/index.scss   |  3 +-
 .../components/horizontal-kv-list/index.scss  |  4 -
 .../components/horizontal-kv-list/layout.scss |  6 +-
 .../components/horizontal-kv-list/skin.scss   |  5 +-
 .../consul-ui/mock-api/v1/catalog/connect/_   |  2 +-
 .../consul-ui/mock-api/v1/health/service/_    |  6 +-
 11 files changed, 109 insertions(+), 91 deletions(-)

diff --git a/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.hbs b/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.hbs
index 0f9737699..3b525336d 100644
--- a/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.hbs
+++ b/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.hbs
@@ -3,7 +3,7 @@
   ...attributes
 >
   <ul>
-  {{#each @items as |item|}}
+{{#each @items as |item|}}
     <li>
       <div class="header">
         <p>
@@ -11,59 +11,59 @@
         </p>
       </div>
       <div class="detail">
-    {{#if (env 'CONSUL_NSPACES_ENABLED')}}
-      {{#if (not-eq item.DestinationType 'prepared_query')}}
+{{#if (env 'CONSUL_NSPACES_ENABLED')}}
+  {{#if (not-eq item.DestinationType 'prepared_query')}}
         <dl class="nspace">
-          <dt>
-            <Tooltip>
-              Namespace
-            </Tooltip>
+          <dt
+            {{tooltip}}
+          >
+            Namespace
           </dt>
           <dd>
             {{or item.DestinationNamespace 'default'}}
           </dd>
         </dl>
-      {{/if}}
-    {{/if}}
-      {{#if (and (not-eq item.Datacenter @dc) (not-eq item.Datacenter ""))}}
+  {{/if}}
+{{/if}}
+{{#if (and (not-eq item.Datacenter @dc) (not-eq item.Datacenter ""))}}
         <dl class="datacenter">
-          <dt>
-            <Tooltip>
-              Datacenter
-            </Tooltip>
+          <dt
+            {{tooltip}}
+          >
+            Datacenter
           </dt>
           <dd>
             {{item.Datacenter}}
           </dd>
         </dl>
-      {{/if}}
-      {{#if item.LocalBindSocketPath}}
-          <dl>
-            <dt>
-                Local bind socket path
-            </dt>
-            <dd>
-              <CopyButton
-                @value={{item.LocalBindSocketPath}}
-                @name="Local bind socket path"
-              />
-              {{item.LocalBindSocketPath}}
-            </dd>
-          </dl>
-          <dl class="local-bind-mode">
-            <dt class>mode</dt>
-            <dd>
-              {{or item.LocalBindSocketMode '-'}}
-            </dd>
-          </dl>
-      {{else}}
-        {{#if (gt item.LocalBindPort 0)}}
-  {{#let (concat (or item.LocalBindAddress '127.0.0.1') ':' item.LocalBindPort) as |combinedAddress|}}
+{{/if}}
+{{#if item.LocalBindSocketPath}}
+        <dl class="local-bind-socket-path">
+          <dt>
+              Local bind socket path
+          </dt>
+          <dd>
+            <CopyButton
+              @value={{item.LocalBindSocketPath}}
+              @name="Local bind socket path"
+            />
+            {{item.LocalBindSocketPath}}
+          </dd>
+        </dl>
+        <dl class="local-bind-socket-mode">
+          <dt>
+            Mode
+          </dt>
+          <dd>
+            {{or item.LocalBindSocketMode '-'}}
+          </dd>
+        </dl>
+{{else}}
+  {{#if (gt item.LocalBindPort 0)}}
+    {{#let (concat (or item.LocalBindAddress '127.0.0.1') ':' item.LocalBindPort) as |combinedAddress|}}
           <dl class="local-bind-address">
             <dt>
-              <span>
-                Address
-              </span>
+              Address
             </dt>
             <dd>
               <CopyButton
@@ -73,12 +73,11 @@
                 {{combinedAddress}}
             </dd>
           </dl>
-  {{/let}}
-        {{/if}}
-
-      {{/if}}
+    {{/let}}
+  {{/if}}
+{{/if}}
       </div>
     </li>
-  {{/each}}
+{{/each}}
   </ul>
 </div>
diff --git a/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.scss b/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.scss
index 9a46faaad..5ad7c5c25 100644
--- a/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.scss
+++ b/ui/packages/consul-ui/app/components/consul/upstream-instance/list/index.scss
@@ -1,4 +1,4 @@
-.consul-upstream-instance-list {
+%consul-upstream-instance-list {
   li {
     @extend %composite-row;
   }
@@ -11,7 +11,16 @@
   dl.nspace dt::before {
     @extend %with-folder-outline-mask, %as-pseudo;
   }
-  dl.local-bind-mode dt {
+  dl.local-bind-socket-path dt,
+  dl.local-bind-address dt {
+    @extend %horizontal-kv-list-hidden-title;
+  }
+  dl.local-bind-socket-mode dt {
+    @extend %horizontal-kv-list-visible-title;
+    text-transform: lowercase;
     font-weight: var(--typo-weight-semibold);
   }
 }
+.consul-upstream-instance-list {
+  @extend %consul-upstream-instance-list;
+}
diff --git a/ui/packages/consul-ui/app/components/consul/upstream/list/index.hbs b/ui/packages/consul-ui/app/components/consul/upstream/list/index.hbs
index 14ba0027f..29d3720f6 100644
--- a/ui/packages/consul-ui/app/components/consul/upstream/list/index.hbs
+++ b/ui/packages/consul-ui/app/components/consul/upstream/list/index.hbs
@@ -5,13 +5,14 @@
   @linkable="linkable upstream"
 as |item index|>
   <BlockSlot @name="header">
-  {{#if (gt item.InstanceCount 0)}}
+{{#if (gt item.InstanceCount 0)}}
     <dl class={{item.MeshStatus}}>
       <dt>
         Health
       </dt>
-      <dd>
-        <Tooltip @position="top-start">
+      <dd
+        {{tooltip}}
+      >
           {{#if (eq 'critical' item.MeshStatus)}}
             At least one health check on one instance is failing.
           {{else if (eq 'warning' item.MeshStatus)}}
@@ -21,48 +22,44 @@ as |item index|>
           {{else}}
             There are no health checks.
           {{/if}}
-        </Tooltip>
       </dd>
     </dl>
-{{#if (and (can 'use nspaces') (not-eq item.Namespace @nspace))}}
-      <a
-        data-test-service-name
-        href={{href-to 'dc.services.show' @dc item.Name
-                params=(hash
-                  nspace=item.Namespace
-                )
-              }}
-      >
-        {{item.Name}}
-      </a>
+    <a
+      data-test-service-name
+      href={{if (and (can 'use nspaces') (not-eq item.Namespace @nspace))
+        (href-to 'dc.services.show' @dc item.Name
+          params=(hash
+            nspace=item.Namespace
+          )
+        )
+        (href-to 'dc.services.show' item.Name)
+      }}
+    >
+      {{item.Name}}
+    </a>
 {{else}}
-      <a data-test-service-name href={{href-to 'dc.services.show' item.Name}}>
-        {{item.Name}}
-      </a>
-{{/if}}
-  {{else}}
     <p data-test-service-name>
       {{item.Name}}
     </p>
-  {{/if}}
+{{/if}}
   </BlockSlot>
   <BlockSlot @name="details">
-  {{#if (and (env 'CONSUL_NSPACES_ENABLED') (not-eq item.Namespace @nspace))}}
+{{#if (and (can 'use nspaces') (not-eq item.Namespace @nspace))}}
     <dl class="nspace">
-      <dt>
-        <Tooltip>
-          Namespace
-        </Tooltip>
+      <dt
+        {{tooltip}}
+      >
+        Namespace
       </dt>
       <dd>
         {{item.Namespace}}
       </dd>
     </dl>
-  {{/if}}
-  {{#each item.GatewayConfig.Addresses as |address|}}
+{{/if}}
+{{#each item.GatewayConfig.Addresses as |address|}}
     <dl>
       <dt>
-        <span>Address</span>
+        Address
       </dt>
       <dd>
         <CopyButton
@@ -72,6 +69,6 @@ as |item index|>
         {{address}}
       </dd>
     </dl>
-  {{/each}}
+{{/each}}
   </BlockSlot>
 </ListCollection>
diff --git a/ui/packages/consul-ui/app/components/copy-button/index.scss b/ui/packages/consul-ui/app/components/copy-button/index.scss
index 12a2b759c..1971e4ee3 100644
--- a/ui/packages/consul-ui/app/components/copy-button/index.scss
+++ b/ui/packages/consul-ui/app/components/copy-button/index.scss
@@ -3,6 +3,20 @@
 %copy-button {
   @extend %button;
 }
+/* Temporary tweak for copy-buttons in kv-lists */
+/* once someone gets round to re-doing copy buttons */
+/* hopefully this little tweak can go */
+%horizontal-kv-list .copy-button {
+  margin-top: 0 !important;
+}
+%horizontal-kv-list .copy-btn {
+  top: 0 !important;
+}
+%horizontal-kv-list .copy-btn:empty::before {
+  left: 0 !important;
+}
+/**/
+
 .copy-button button {
   @extend %copy-button;
 }
diff --git a/ui/packages/consul-ui/app/components/definition-table/layout.scss b/ui/packages/consul-ui/app/components/definition-table/layout.scss
index feb5e1b27..b4ffaebc5 100644
--- a/ui/packages/consul-ui/app/components/definition-table/layout.scss
+++ b/ui/packages/consul-ui/app/components/definition-table/layout.scss
@@ -2,6 +2,7 @@
   display: grid;
   grid-template-columns: 140px auto;
   grid-gap: 0.4em 20px;
+  margin-bottom: 1.4em;
 }
 %definition-table dd > * {
   display: inline-block;
diff --git a/ui/packages/consul-ui/app/components/form-elements/index.scss b/ui/packages/consul-ui/app/components/form-elements/index.scss
index 2a95e8b6c..3ace8435f 100644
--- a/ui/packages/consul-ui/app/components/form-elements/index.scss
+++ b/ui/packages/consul-ui/app/components/form-elements/index.scss
@@ -60,8 +60,7 @@ span.label {
 }
 %form table,
 %radio-group,
-%checkbox-group,
-%main-content form dl {
+%checkbox-group {
   @extend %form-row;
 }
 %radio-group label,
diff --git a/ui/packages/consul-ui/app/components/horizontal-kv-list/index.scss b/ui/packages/consul-ui/app/components/horizontal-kv-list/index.scss
index 338ff82c5..bcdcb8e17 100644
--- a/ui/packages/consul-ui/app/components/horizontal-kv-list/index.scss
+++ b/ui/packages/consul-ui/app/components/horizontal-kv-list/index.scss
@@ -10,10 +10,6 @@
 %horizontal-kv-list:not([class]) dt:not([class]) {
   @extend %horizontal-kv-list-hidden-title;
 }
-%horizontal-kv-list[class] dt,
-%horizontal-kv-list dt[class] {
-  @extend %horizontal-kv-list-visible-title;
-}
 %horizontal-kv-list-hidden-title {
   @extend %visually-hidden;
 }
diff --git a/ui/packages/consul-ui/app/components/horizontal-kv-list/layout.scss b/ui/packages/consul-ui/app/components/horizontal-kv-list/layout.scss
index fab1160e9..672c4e19f 100644
--- a/ui/packages/consul-ui/app/components/horizontal-kv-list/layout.scss
+++ b/ui/packages/consul-ui/app/components/horizontal-kv-list/layout.scss
@@ -19,6 +19,7 @@
   flex-wrap: wrap;
 }
 %horizontal-kv-list-visible-title {
+  display: inline-flex;
   min-width: 18px;
 }
 %horizontal-kv-list-reversed:not(:first-of-type),
@@ -35,9 +36,12 @@
 }
 /**/
 %horizontal-kv-list-reversed dt,
-%horizontal-kv-list-visible-title + dd {
+%horizontal-kv-list dt + dd {
   margin-left: 4px;
 }
+%horizontal-kv-list-hidden-title + dd {
+  margin-left: 0 !important;
+}
 
 %horizontal-kv-list-reversed dt + dd {
   margin-left: 0;
diff --git a/ui/packages/consul-ui/app/components/horizontal-kv-list/skin.scss b/ui/packages/consul-ui/app/components/horizontal-kv-list/skin.scss
index 9cbf85d80..5c337ae19 100644
--- a/ui/packages/consul-ui/app/components/horizontal-kv-list/skin.scss
+++ b/ui/packages/consul-ui/app/components/horizontal-kv-list/skin.scss
@@ -1,8 +1,7 @@
-%horizontal-kv-list dt[class=""]::after,
-%horizontal-kv-list dt[class=""]::after {
+%horizontal-kv-list-visible-title::after {
   display: inline;
 }
-%horizontal-kv-list dt[class=""]::after {
+%horizontal-kv-list-visible-title::after {
   content: var(--horizontal-kv-list-key-separator);
 }
 %horizontal-kv-list-reversed dt[class=""]::after {
diff --git a/ui/packages/consul-ui/mock-api/v1/catalog/connect/_ b/ui/packages/consul-ui/mock-api/v1/catalog/connect/_
index 983d8ad97..4957c6d83 100644
--- a/ui/packages/consul-ui/mock-api/v1/catalog/connect/_
+++ b/ui/packages/consul-ui/mock-api/v1/catalog/connect/_
@@ -58,7 +58,7 @@ ${range(env('CONSUL_EXPOSED_COUNT', 3)).map((i) => `
           ${ location.pathname.slice(4) === "service-0" ? `
           ,
           "DestinationServiceID": "${location.pathname.slice(4)}-with-id",
-${ fake.random.number({min: 1, max: 10}) > 2 ? `
+${ fake.random.number({min: 1, max: 10}) > 5 ? `
           "LocalServiceAddress": "${fake.internet.ip()}",
           "LocalServicePort": ${fake.random.number({min: 0, max: 65535})}
 ` : `
diff --git a/ui/packages/consul-ui/mock-api/v1/health/service/_ b/ui/packages/consul-ui/mock-api/v1/health/service/_
index 3427304ae..555d46f5e 100644
--- a/ui/packages/consul-ui/mock-api/v1/health/service/_
+++ b/ui/packages/consul-ui/mock-api/v1/health/service/_
@@ -127,13 +127,13 @@ ${range(env('CONSUL_EXPOSED_COUNT', 3)).map((i) => `
                 ]
               },
               "Upstreams": [
-  ${range(env('CONSUL_UPSTREAM_COUNT', 10)).map((item, j) => `
+${range(env('CONSUL_UPSTREAM_COUNT', 10)).map((item, j) => `
                   {
                     "Datacenter": "${fake.address.countryCode().toLowerCase()} ${ i % 2 ? "west" : "east"}-${j}",
                     "DestinationName": "${fake.hacker.noun()}",
                     "DestinationNamespace": "${fake.hacker.noun()}",
                     "DestinationType": "${fake.helpers.randomize(['service', 'prepared_query'])}",
-${ false ? `
+${fake.random.number({min: 1, max: 10}) > 5 ? `
                     "LocalBindAddress": "${fake.internet.ip()}",
                     "LocalBindPort": ${fake.random.number({min: 0, max: 65535})}
 ` : `
@@ -141,7 +141,7 @@ ${ false ? `
                     "LocalBindSocketPath": "/${fake.lorem.words(fake.random.number({min: 1, max: 5})).split(' ').join('/')}${fake.random.boolean() ? fake.system.fileName() : ''}"
 ` }
                   }
-  `)}
+`)}
               ]
 ` : ``}
             },