From 6e8a8a2bc0fc4d1aab29a1719b6839eab7b69fba Mon Sep 17 00:00:00 2001
From: John Cowen <johncowen@users.noreply.github.com>
Date: Wed, 18 Dec 2019 18:27:54 +0000
Subject: [PATCH] ui: Various amends for 1.7beta (#6965)

* Remove empty init

* Actually make the disco chain endpoint send the nspace, note:

The backend doesn't support this as yet.

* Tweak the font size of flash-messages ever so slightly

* Make sure the nspace menu is kept up to date when creating a new one

* Move comment to the correct place

* Only refresh the namespace menu if you specifically created a nspace

* Change FIXMEs to TODOs as we are happy for these to wait until later
---
 ui-v2/app/adapters/discovery-chain.js         |  8 +++++--
 ui-v2/app/components/aria-menu.js             | 13 ++++++-----
 ui-v2/app/components/discovery-chain.js       |  8 +++----
 ui-v2/app/helpers/href-mut.js                 |  4 ++--
 ui-v2/app/instance-initializers/nspace.js     |  2 +-
 ui-v2/app/routes/application.js               |  7 +++---
 ui-v2/app/routes/dc.js                        | 18 +++++++++++++++
 ui-v2/app/routes/dc/acls.js                   |  5 ++++-
 ui-v2/app/routes/dc/services/show.js          |  5 +++--
 .../components/discovery-chain/layout.scss    |  2 +-
 .../components/healthchecked-resource.scss    | 22 ++-----------------
 ui-v2/app/styles/components/index.scss        |  2 +-
 .../main-nav-horizontal/layout.scss           | 15 +++++++++++--
 ui-v2/app/styles/components/tooltip.scss      | 19 ++++++++++++++++
 ui-v2/app/styles/core/typography.scss         |  1 +
 15 files changed, 86 insertions(+), 45 deletions(-)

diff --git a/ui-v2/app/adapters/discovery-chain.js b/ui-v2/app/adapters/discovery-chain.js
index 81f6f6f05..1f1efa0a9 100644
--- a/ui-v2/app/adapters/discovery-chain.js
+++ b/ui-v2/app/adapters/discovery-chain.js
@@ -1,14 +1,18 @@
 import Adapter from './application';
 
+// TODO: Update to use this.formatDatacenter()
 export default Adapter.extend({
-  requestForQueryRecord: function(request, { dc, index, id }) {
+  requestForQueryRecord: function(request, { dc, ns, index, id }) {
     if (typeof id === 'undefined') {
       throw new Error('You must specify an id');
     }
     return request`
       GET /v1/discovery-chain/${id}?${{ dc }}
 
-      ${{ index }}
+      ${{
+        ...this.formatNspace(ns),
+        index,
+      }}
     `;
   },
 });
diff --git a/ui-v2/app/components/aria-menu.js b/ui-v2/app/components/aria-menu.js
index 51ee98691..2e44215e2 100644
--- a/ui-v2/app/components/aria-menu.js
+++ b/ui-v2/app/components/aria-menu.js
@@ -34,12 +34,14 @@ const keys = {
   horizontal: {},
 };
 const COMPONENT_ID = 'component-aria-menu-';
+// ^menuitem supports menuitemradio and menuitemcheckbox
+const MENU_ITEMS = '[role^="menuitem"]';
 export default Component.extend({
   tagName: '',
   dom: service('dom'),
   guid: '',
   expanded: false,
-  direction: 'vertical',
+  orientation: 'vertical',
   init: function() {
     this._super(...arguments);
     set(this, 'guid', this.dom.guid(this));
@@ -67,9 +69,8 @@ export default Component.extend({
       // Also we may do this but not need it if we return early below
       // although once we add support for [A-Za-z] it unlikely we won't use
       // the keypress
-      // ^menuitem supports menuitemradio and menuitemcheckbox
       // TODO: We need to use > somehow here so we don't select submenus
-      const $items = [...this.dom.elements('[role^="menuitem"]', this.$menu)];
+      const $items = [...this.dom.elements(MENU_ITEMS, this.$menu)];
       if (!this.expanded) {
         this.$trigger.dispatchEvent(new MouseEvent('click'));
         if (e.keyCode === ENTER || e.keyCode === SPACE) {
@@ -79,17 +80,17 @@ export default Component.extend({
       }
       // this will prevent anything happening if you haven't pushed a
       // configurable key
-      if (typeof keys[this.direction][e.keyCode] === 'undefined') {
+      if (typeof keys[this.orientation][e.keyCode] === 'undefined') {
         return;
       }
-      const $focused = this.dom.element('[role="menuitem"]:focus', this.$menu);
+      const $focused = this.dom.element(`${MENU_ITEMS}:focus`, this.$menu);
       let i;
       if ($focused) {
         i = $items.findIndex(function($item) {
           return $item === $focused;
         });
       }
-      const $next = $items[keys[this.direction][e.keyCode]($items, i)];
+      const $next = $items[keys[this.orientation][e.keyCode]($items, i)];
       $next.focus();
     },
     // TODO: The argument here needs to change to an event
diff --git a/ui-v2/app/components/discovery-chain.js b/ui-v2/app/components/discovery-chain.js
index 47e6d0061..45380c2a2 100644
--- a/ui-v2/app/components/discovery-chain.js
+++ b/ui-v2/app/components/discovery-chain.js
@@ -10,7 +10,7 @@ const getNodesByType = function(nodes = {}, type) {
 const targetsToFailover = function(targets, a) {
   let type;
   const Targets = targets.map(function(b) {
-    // FIXME: this isn't going to work past namespace for services
+    // TODO: this isn't going to work past namespace for services
     // with dots in the name
     const [aRev, bRev] = [a, b].map(item => item.split('.').reverse());
     const types = ['Datacenter', 'Namespace', 'Service', 'Subset'];
@@ -57,7 +57,7 @@ const getTargetResolvers = function(dc, nspace = 'default', targets = [], nodes
       let failoverable = resolver;
       if (item.ServiceSubset) {
         failoverable = item;
-        // FIXME: Sometimes we have set the resolvers ID to the ID of the
+        // TODO: Sometimes we have set the resolvers ID to the ID of the
         // subset this just shifts the subset of the front of the URL for the moment
         const temp = item.ID.split('.');
         temp.shift();
@@ -65,7 +65,7 @@ const getTargetResolvers = function(dc, nspace = 'default', targets = [], nodes
         resolver.Children.push(item);
       }
       if (typeof node.Resolver.Failover !== 'undefined') {
-        // FIXME: Figure out if we can get rid of this
+        // TODO: Figure out if we can get rid of this
         /* eslint ember/no-side-effects: "warn" */
         set(failoverable, 'Failover', targetsToFailover(node.Resolver.Failover.Targets, item.ID));
       } else {
@@ -245,7 +245,7 @@ export default Component.extend({
   // the developer access to the mouse event therefore we just use JS to add our events
   // revisit this post Octane
   addPathListeners: function() {
-    // FIXME: Figure out if we can remove this next
+    // TODO: Figure out if we can remove this next
     next(() => {
       this._listeners.remove();
       [...this.dom.elements('path.split', this.element)].forEach(item => {
diff --git a/ui-v2/app/helpers/href-mut.js b/ui-v2/app/helpers/href-mut.js
index fc530bd1a..6ad337df1 100644
--- a/ui-v2/app/helpers/href-mut.js
+++ b/ui-v2/app/helpers/href-mut.js
@@ -22,14 +22,14 @@ export default Helper.extend({
       atts = atts.concat(getRouteParams(parent, params));
       current = parent;
     }
-    let route = this.router.currentRoute.name;
+    let route = this.router.currentRouteName;
     // TODO: this is specific to consul/nspaces
     // 'ideally' we could try and do this elsewhere
     // not super important though.
     // This will turn an URL that has no nspace (/ui/dc-1/nodes) into one
     // that does have a namespace (/ui/~nspace/dc-1/nodes) if you href-mut with
     // a nspace parameter
-    if (typeof params.nspace !== 'undefined' && !route.startsWith('nspace.')) {
+    if (typeof params.nspace !== 'undefined' && route.startsWith('dc.')) {
       route = `nspace.${route}`;
       atts.push(params.nspace);
     }
diff --git a/ui-v2/app/instance-initializers/nspace.js b/ui-v2/app/instance-initializers/nspace.js
index 0a77daa19..72511faea 100644
--- a/ui-v2/app/instance-initializers/nspace.js
+++ b/ui-v2/app/instance-initializers/nspace.js
@@ -11,7 +11,7 @@ export function initialize(container) {
       .root()
       .classList.add('has-nspaces');
   }
-  // FIXME: This needs to live in its own initializer, either:
+  // TODO: This needs to live in its own initializer, either:
   // 1. Make it be about adding classes to the root dom node
   // 2. Make it be about config and things to do on initialization re: config
   // If we go with 1 then we need to move both this and the above nspaces class
diff --git a/ui-v2/app/routes/application.js b/ui-v2/app/routes/application.js
index 6023d3cfe..d815090bb 100644
--- a/ui-v2/app/routes/application.js
+++ b/ui-v2/app/routes/application.js
@@ -11,9 +11,6 @@ const removeLoading = function($from) {
 };
 export default Route.extend(WithBlockingActions, {
   dom: service('dom'),
-  init: function() {
-    this._super(...arguments);
-  },
   nspacesRepo: service('repository/nspace/disabled'),
   repo: service('repository/dc'),
   settings: service('settings'),
@@ -79,6 +76,7 @@ export default Route.extend(WithBlockingActions, {
       const $root = this.dom.root();
       hash({
         error: error,
+        nspace: this.nspacesRepo.getActive(),
         dc:
           error.status.toString().indexOf('5') !== 0
             ? this.repo.getActive()
@@ -89,6 +87,9 @@ export default Route.extend(WithBlockingActions, {
       })
         .then(model => {
           removeLoading($root);
+          model.nspaces = [model.nspace];
+          // we can't use setupController as we received an error
+          // so we do it manually instead
           next(() => {
             this.controllerFor('error').setProperties(model);
           });
diff --git a/ui-v2/app/routes/dc.js b/ui-v2/app/routes/dc.js
index f7911b7f1..29a194ce9 100644
--- a/ui-v2/app/routes/dc.js
+++ b/ui-v2/app/routes/dc.js
@@ -69,4 +69,22 @@ export default Route.extend({
   setupController: function(controller, model) {
     controller.setProperties(model);
   },
+  actions: {
+    // TODO: This will eventually be deprecated please see
+    // https://deprecations.emberjs.com/v3.x/#toc_deprecate-router-events
+    willTransition: function(transition) {
+      this._super(...arguments);
+      if (typeof transition !== 'undefined' && transition.from.name.endsWith('nspaces.create')) {
+        // Only when we create, reload the nspaces in the main menu to update them
+        // as we don't block for those
+        this.nspacesRepo.findAll().then(items => {
+          if (typeof this.controller !== 'undefined') {
+            this.controller.setProperties({
+              nspaces: items,
+            });
+          }
+        });
+      }
+    },
+  },
 });
diff --git a/ui-v2/app/routes/dc/acls.js b/ui-v2/app/routes/dc/acls.js
index a96251773..f1e759ee4 100644
--- a/ui-v2/app/routes/dc/acls.js
+++ b/ui-v2/app/routes/dc/acls.js
@@ -1,5 +1,6 @@
 import Route from '@ember/routing/route';
 import { get } from '@ember/object';
+import { config } from 'consul-ui/env';
 import { inject as service } from '@ember/service';
 import WithBlockingActions from 'consul-ui/mixins/with-blocking-actions';
 export default Route.extend(WithBlockingActions, {
@@ -31,7 +32,9 @@ export default Route.extend(WithBlockingActions, {
                   return false;
                 });
               } else {
-                if (get(item, 'token.Namespace') !== nspace) {
+                // TODO: Ideally we wouldn't need to use config() at a route level
+                // transitionTo should probably remove it instead if NSPACES aren't enabled
+                if (config('CONSUL_NSPACES_ENABLED') && get(item, 'token.Namespace') !== nspace) {
                   let routeName = this.router.currentRouteName;
                   if (!routeName.startsWith('nspace')) {
                     routeName = `nspace.${routeName}`;
diff --git a/ui-v2/app/routes/dc/services/show.js b/ui-v2/app/routes/dc/services/show.js
index f732832d8..4df3f4bbd 100644
--- a/ui-v2/app/routes/dc/services/show.js
+++ b/ui-v2/app/routes/dc/services/show.js
@@ -14,9 +14,10 @@ export default Route.extend({
   },
   model: function(params) {
     const dc = this.modelFor('dc').dc.Name;
+    const nspace = this.modelFor('nspace').nspace.substr(1);
     return hash({
-      item: this.repo.findBySlug(params.name, dc, this.modelFor('nspace').nspace.substr(1)),
-      chain: this.chainRepo.findBySlug(params.name, dc),
+      item: this.repo.findBySlug(params.name, dc, nspace),
+      chain: this.chainRepo.findBySlug(params.name, dc, nspace),
       urls: this.settings.findBySlug('urls'),
       dc: dc,
     });
diff --git a/ui-v2/app/styles/components/discovery-chain/layout.scss b/ui-v2/app/styles/components/discovery-chain/layout.scss
index 36b9a6108..0beb5548b 100644
--- a/ui-v2/app/styles/components/discovery-chain/layout.scss
+++ b/ui-v2/app/styles/components/discovery-chain/layout.scss
@@ -33,7 +33,7 @@
   @extend %blink-in-fade-out-active;
 }
 %resolver-card dt {
-  @extend %with-pseudo-tooltip;
+  @extend %with-pseudo-tooltip, %tooltip-right;
 }
 
 %discovery-chain {
diff --git a/ui-v2/app/styles/components/healthchecked-resource.scss b/ui-v2/app/styles/components/healthchecked-resource.scss
index 80fa42d22..2017a081a 100644
--- a/ui-v2/app/styles/components/healthchecked-resource.scss
+++ b/ui-v2/app/styles/components/healthchecked-resource.scss
@@ -2,32 +2,14 @@
 .healthchecked-resource > div {
   @extend %stats-card;
 }
-%tooltip-below::after {
-  top: calc(100% - 8px);
-  bottom: auto;
-  border-top: none;
-  border-bottom: 18px solid $gray-500;
-}
-%tooltip-below::before {
-  top: calc(100% + 4px);
-  bottom: auto;
-  /*TODO: This should probably go into base*/
-  line-height: 1em;
-}
-%tooltip-left::before {
-  right: 0;
-}
-%tooltip-right::before {
-  left: -10px;
-}
 %stats-card-icon {
   @extend %tooltip-below;
 }
 %stats-card-icon:first-child::before {
-  right: 0;
+  @extend %tooltip-left;
 }
 %stats-card-icon:last-child::before {
-  left: -10px;
+  @extend %tooltip-right;
 }
 
 %stats-card-icon:last-child {
diff --git a/ui-v2/app/styles/components/index.scss b/ui-v2/app/styles/components/index.scss
index 6783a7db5..445d0d8f8 100644
--- a/ui-v2/app/styles/components/index.scss
+++ b/ui-v2/app/styles/components/index.scss
@@ -15,6 +15,7 @@
 @import './app-view';
 @import './product';
 
+@import './tooltip';
 @import './tag-list';
 @import './healthcheck-output';
 @import './healthcheck-info';
@@ -30,6 +31,5 @@
 @import './feedback-dialog';
 @import './modal-dialog';
 @import './notice';
-@import './tooltip';
 @import './sort-control';
 @import './discovery-chain';
diff --git a/ui-v2/app/styles/components/main-nav-horizontal/layout.scss b/ui-v2/app/styles/components/main-nav-horizontal/layout.scss
index 4ddde5f29..6bbdd50ed 100644
--- a/ui-v2/app/styles/components/main-nav-horizontal/layout.scss
+++ b/ui-v2/app/styles/components/main-nav-horizontal/layout.scss
@@ -13,6 +13,10 @@
   padding: 5px 12px;
   white-space: nowrap;
 }
+%main-nav-horizontal input + label > * {
+  /* less space as the chevron adds space */
+  padding-right: 4px !important;
+}
 %main-nav-horizontal-drop-nav {
   z-index: 400;
   /* TODO: We should probably make menu-panel default to left hand side*/
@@ -30,7 +34,6 @@
   padding-top: 0.35em;
 }
 %main-nav-horizontal-drop-nav-separator:not(:first-child) {
-  padding-top: 0.7em;
   margin-top: 0.35em;
 }
 %main-nav-horizontal-drop-nav-header {
@@ -151,6 +154,14 @@
     min-width: 266px;
   }
   %main-nav-horizontal input + label {
-    padding-right: 5px !important;
+    /* Usually there is no space between buttons which is */
+    /* fine as the button only highlights when its selected */
+    /* therefore no two siblings are highlighted at the same time */
+    /* which means you don't notice there is no space between the */
+    /* buttons. popover-menu triggers on the other hand can be */
+    /* at the same time as a sibling, therefore they need a little */
+    /* space between it and its sibling - this is a poroperty of */
+    /* the main nav not the popover-menu */
+    padding-right: 5px;
   }
 }
diff --git a/ui-v2/app/styles/components/tooltip.scss b/ui-v2/app/styles/components/tooltip.scss
index 22f5059fb..efbb67806 100644
--- a/ui-v2/app/styles/components/tooltip.scss
+++ b/ui-v2/app/styles/components/tooltip.scss
@@ -7,6 +7,25 @@
 /* override structure min-width for the moment */
 /* TODO: Clarify whether these should actually use */
 /* the min-width from structure */
+/* TODO: See if we can move all these to base */
 %tooltip-bubble {
   min-width: 0;
 }
+%tooltip-below::after {
+  top: calc(100% - 8px);
+  bottom: auto;
+  border-top: none;
+  border-bottom: 18px solid $gray-500;
+}
+%tooltip-below::before {
+  top: calc(100% + 4px);
+  bottom: auto;
+  /*TODO: This should probably go into base*/
+  line-height: 1em;
+}
+%tooltip-left::before {
+  right: 0;
+}
+%tooltip-right::before {
+  left: -7px;
+}
diff --git a/ui-v2/app/styles/core/typography.scss b/ui-v2/app/styles/core/typography.scss
index c82a8133c..d6c789cf9 100644
--- a/ui-v2/app/styles/core/typography.scss
+++ b/ui-v2/app/styles/core/typography.scss
@@ -38,6 +38,7 @@ fieldset > header,
 pre code,
 %notice,
 %notice p,
+%flash-message p,
 %filter-bar input,
 %phrase-editor input {
   @extend %p1;