ui: Replace CollapsibleNotices with more a11y focussed Disclosure component (#12305)

* Delete collapsible notices component and related helper

* Add relative t action/helper to our Route component

* Replace single use CollapsibleNotices with multi-use Disclosure

.changelog/12305.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+ui: Improve usability of Topology warning/information panels
+```

ui/packages/consul-ui/app/components/collapsible-notices/README.mdx

@@ -1,66 +0,0 @@
-# CollapsibleNotices
-
-Used as a wrapper to collapse the details of `<Notices/>`.
-
-```hbs preview-template
-<CollapsibleNotices>
-  <Notice
-    @type="error"
-    role="alert"
-  as |notice|>
-    <notice.Header>
-      <h3>Header</h3>
-    </notice.Header>
-    <notice.Body>
-      <p>
-        Body
-      </p>
-    </notice.Body>
-  </Notice>
-  <Notice
-    @type="info"
-  as |notice|>
-    <notice.Header>
-      <h3>Header</h3>
-    </notice.Header>
-    <notice.Body>
-      <p>
-        Body
-      </p>
-    </notice.Body>
-    <notice.Footer>
-      <p>
-        Footer
-      </p>
-    </notice.Footer>
-  </Notice>
-  <Notice
-    @type="warning"
-  as |notice|>
-    <notice.Header>
-      <h3>Header</h3>
-    </notice.Header>
-    <notice.Body>
-      <p>
-        Body
-      </p>
-    </notice.Body>
-    <notice.Footer>
-      <p>
-        Footer
-      </p>
-    </notice.Footer>
-  </Notice>
-</CollapsibleNotices>
-
-```
-
-## Arguments
-
-No arguments required. Wrap this component around the needed notices.
-
-## See
-
-- [Template Source Code](./index.hbs)
-
----

ui/packages/consul-ui/app/components/collapsible-notices/index.hbs

@@ -1,14 +0,0 @@
-{{#if @collapsible}}
-  <div class="collapsible-notices {{if this.collapsed 'collapsed'}}">
-    <div class="notices">
-      {{yield}}
-    </div>
-    {{#if this.collapsed}}
-    <button type="button" class="expand" {{on 'click' (set this 'collapsed' false)}}>{{t "components.app.collapsible-notices.expand"}}</button>
-    {{else}}
-    <button type="button" class="collapse" {{on 'click' (set this 'collapsed' true)}}>{{t "components.app.collapsible-notices.collapse"}}</button>
-    {{/if}}
-  </div>
-{{else}}
-  {{yield}}
-{{/if}}

ui/packages/consul-ui/app/components/collapsible-notices/index.js

@@ -1,3 +0,0 @@
-import Component from '@glimmer/component';
-
-export default class CollapsibleNotices extends Component {}

ui/packages/consul-ui/app/components/collapsible-notices/index.scss

@@ -1,31 +0,0 @@
-.collapsible-notices {
-  display: grid;
-  grid-template-columns: auto 168px;
-  grid-template-rows: auto 55px;
-  grid-template-areas:
-    'notices notices'
-    '. toggle-button';
-  &.collapsed p {
-    display: none;
-  }
-  .notices {
-    grid-area: notices;
-    :last-child {
-      margin-bottom: 0;
-    }
-  }
-  button {
-    @extend %button;
-    color: rgb(var(--color-action));
-    float: right;
-    grid-area: toggle-button;
-    margin-top: 1em;
-    margin-bottom: 2em;
-  }
-  button.expand::before {
-    @extend %with-chevron-down-mask, %as-pseudo;
-  }
-  button.collapse::before {
-    @extend %with-chevron-up-mask, %as-pseudo;
-  }
-}

ui/packages/consul-ui/app/components/route/index.hbs

@@ -6,6 +6,7 @@
   currentName=this.router.currentRoute.name
 
   refresh=this.refresh
+  t=this.t
 
   Title=(component "route/title")
   Announcer=(component "route/announcer")

ui/packages/consul-ui/app/components/route/index.js

@@ -3,18 +3,26 @@ import { inject as service } from '@ember/service';
 import { action } from '@ember/object';
 import { tracked } from '@glimmer/tracking';
 
+const templateRe = /\${([A-Za-z.0-9_-]+)}/g;
 export default class RouteComponent extends Component {
   @service('routlet') routlet;
   @service('router') router;
+  @service('intl') intl;
+  @service('encoder') encoder;
 
   @tracked _model;
 
+  constructor() {
+    super(...arguments);
+    this.intlKey = this.encoder.createRegExpEncoder(templateRe, _ => _);
+  }
+
   get params() {
     return this.routlet.paramsFor(this.args.name);
   }
 
   get model() {
-    if(this._model) {
+    if (this._model) {
       return this._model;
     }
     if (this.args.name) {
@@ -23,6 +31,13 @@ export default class RouteComponent extends Component {
     }
     return undefined;
   }
+  @action
+  t(str, options) {
+    if (str.includes('${')) {
+      str = this.intlKey(str, options);
+    }
+    return this.intl.t(`routes.${this.args.name}.${str}`, options);
+  }
 
   @action
   connect() {

ui/packages/consul-ui/app/components/topology-metrics/layout.scss

@@ -1,3 +1,11 @@
+.topology-notices {
+  display: flow-root;
+  button {
+    float: right;
+    margin-top: 16px;
+    margin-bottom: 32px;
+  }
+}
 .topology-container {
   display: grid;
   height: 100%;

ui/packages/consul-ui/app/components/topology-metrics/notice/index.hbs

@@ -1,31 +0,0 @@
-<Notice
-  class="topology-metrics-notice"
-  ...attributes
-  @type={{@type}}
-as |notice|>
-    <notice.Header>
-    <h3>
-      {{t (concat "components.consul.topology-metrics.notice." @for ".header")}}
-    </h3>
-  </notice.Header>
-  <notice.Body>
-    <p>
-     {{t (concat "components.consul.topology-metrics.notice." @for ".body")}}
-    </p>
-  </notice.Body>
-{{#if @action}}
-  <notice.Footer>
-    <p>
-      {{#if @internal}}
-        <Action @href={{href-to (t (concat "components.consul.topology-metrics.notice." @for ".footer.URL"))}}>
-          {{t (concat "components.consul.topology-metrics.notice." @for ".footer.name")}}
-        </Action>
-      {{else}}
-        <Action @href={{@link}} @external={{true}}>
-          {{t (concat "components.consul.topology-metrics.notice." @for ".footer")}}
-        </Action>
-      {{/if}}
-    </p>
-  </notice.Footer>
-{{/if}}
-</Notice>

ui/packages/consul-ui/app/components/topology-metrics/skin.scss

@@ -1,3 +1,16 @@
+.topology-notices {
+  button {
+    @extend %button;
+    color: rgb(var(--tone-blue-500));
+  }
+  button::before {
+    @extend %as-pseudo;
+    @extend %with-chevron-down-mask;
+  }
+  button[aria-expanded='true']::before {
+    @extend %with-chevron-up-mask;
+  }
+}
 .topology-container {
   color: rgb(var(--tone-gray-700));
 }

ui/packages/consul-ui/app/helpers/collapsible-notices.js

@@ -1,9 +0,0 @@
-import { helper } from '@ember/component/helper';
-
-export function collapsibleNotices(params, hash) {
-  // This filter will only return truthy items
-  const noticesCount = params.filter(Boolean).length;
-  return noticesCount > 2;
-}
-
-export default helper(collapsibleNotices);

ui/packages/consul-ui/app/styles/components.scss

@@ -92,7 +92,6 @@
 @import 'consul-ui/components/consul/auth-method';
 
 @import 'consul-ui/components/role-selector';
-@import 'consul-ui/components/collapsible-notices';
 @import 'consul-ui/components/topology-metrics';
 @import 'consul-ui/components/topology-metrics/card';
 @import 'consul-ui/components/topology-metrics/source-type';

ui/packages/consul-ui/app/templates/dc/services/show/topology.hbs

@@ -27,66 +27,107 @@ as |route|>
   loader.data
 as |nspace dc items topology|}}
   <div class="tab-section">
-  {{#let (collapsible-notices topology.FilteredByACLs (eq dc.DefaultACLPolicy 'allow') topology.wildcardIntention topology.notDefinedIntention) as |collapsible| }}
+
-    <CollapsibleNotices @collapsible={{collapsible}}>
+    <div
-      {{#if topology.FilteredByACLs}}
+      class="topology-notices"
-        <TopologyMetrics::Notice
+    >
-          data-test-notice='filtered-by-acls'
+      <Disclosure
-          @type="info"
+        @expanded={{true}}
-          @for="limited-access"
+        as |disclosure|
-          @action={{false}}
+      >
-        />
+        {{! Make a map of key=enabled to tell us which notices are enabled }}
-      {{/if}}
+        {{! The `false` here is so we can easily open all the notices up by changing to `true` }}
-      {{#if (eq dc.DefaultACLPolicy 'allow')}}
+        {{#let
-        <TopologyMetrics::Notice
+          (from-entries (array
-          data-test-notice='default-allow'
+            (array 'filtered-by-acls' (or false topology.FilteredByACLs))
-          @type="warning"
+            (array 'default-allow' (or false (eq dc.DefaultACLPolicy 'allow')))
-          @for="default-allow"
+            (array 'wildcard-intention' (or false topology.wildcardIntention))
-          @internal={{true}}
+            (array 'not-defined-intention' (or false topology.notDefinedIntention))
-          @action={{true}}
+            (array 'no-dependencies' (or false (and topology.noDependencies (can 'use acls'))))
-        />
+            (array 'acls-disabled' (or false (and topology.noDependencies (not (can 'use acls')))))
-      {{/if}}
+          ))
-      {{#if topology.wildcardIntention}}
+        as |notices|}}
-        <TopologyMetrics::Notice
+
-          data-test-notice='wildcard-intention'
+          {{! Make an array only of truthy values to we know how many notices are enabled }}
-          @type="warning"
+          {{#let (without false
-          @for="wildcard-intention"
+            (values notices)
-          @internal={{true}}
+          ) as |noticesEnabled|}}
-          @action={{true}}
+
-        />
+            {{! Render any enabled notices }}
-      {{/if}}
+            {{#each-in notices as |prop enabled|}}
-      {{#if topology.notDefinedIntention}}
+              {{#if enabled}}
-        <TopologyMetrics::Notice
+                <disclosure.Details
-          data-test-notice='not-defined-intention'
+                  @auto={{false}}
-          @type="warning"
+                as |details|>
-          @for="not-defined-intention"
+                  <Notice
-          @link="{{env 'CONSUL_DOCS_URL'}}/connect/registration/service-registration#upstreams"
+                    class="topology-metrics-notice"
-          @internal={{false}}
+                    id={{details.id}}
-          @action={{true}}
+                    data-test-notice={{prop}}
-        />
+                    @type={{if (contains prop (array 'filtered-by-acls' 'no-dependencies'))
-      {{/if}}
+                      'info'
-      {{#if (and topology.noDependencies (can 'use acls'))}}
+                      'warning'
-        <TopologyMetrics::Notice
+                    }}
-          data-test-notice='no-dependencies'
+                  as |notice|>
-          @type="info"
+                    <notice.Header>
-          @for="no-dependencies"
+                      <h3>
-          @link="{{env 'CONSUL_DOCS_URL'}}/connect/registration/service-registration#upstream-configuration-reference"
+                        {{compute (fn route.t 'notice.${prop}.header'
-          @internal={{false}}
+                          (hash
-          @action={{true}}
+                            prop=prop
-        />
+                          )
-      {{/if}}
+                        )}}
-      {{#if (and topology.noDependencies (not (can 'use acls')))}}
+                      </h3>
-        <TopologyMetrics::Notice
+                    </notice.Header>
-          data-test-notice='acls-disabled'
+                    {{#if disclosure.expanded}}
-          @type="warning"
+                      <notice.Body>
-          @for="acls-disabled"
+                        <p>
-          @link="{{env 'CONSUL_DOCS_URL'}}/security/acl/acl-system#configuring-acls"
+                          {{compute (fn route.t 'notice.${prop}.body'
-          @internal={{false}}
+                            (hash
-          @action={{true}}
+                              prop=prop
-        />
+                            )
-      {{/if}}
+                          )}}
-    </CollapsibleNotices>
+                        </p>
-  {{/let}}
+                      </notice.Body>
+                    {{/if}}
+                    {{#let
+                      (compute (fn route.t 'notice.${prop}.footer'
+                        (hash
+                          route_intentions=(href-to 'dc.services.show.intentions')
+                          prop=prop
+                          htmlSafe=true
+                        )
+                      ))
+                    as |footer|}}
+                      {{#if (and disclosure.expanded (not-eq prop 'filtered-by-acls'))}}
+                        <notice.Footer>
+                          {{footer}}
+                        </notice.Footer>
+                      {{/if}}
+                    {{/let}}
+                  </Notice>
+                </disclosure.Details>
+              {{/if}}
+            {{/each-in}}
+
+            {{! If more than 2 notices are enabled let the user close them }}
+            {{#if (gt noticesEnabled.length 2)}}
+              <disclosure.Action
+                {{on 'click' disclosure.toggle}}
+              >
+              {{compute (fn route.t 'notices.${expanded}'
+                (hash
+                  expanded=(if disclosure.expanded 'close' 'open')
+                )
+              )}}
+              </disclosure.Action>
+            {{/if}}
+
+          {{/let}}
+        {{/let}}
+      </Disclosure>
+
+    </div>
+
+
     <DataSource
       @src={{uri '/${partition}/${nspace}/${dc}/ui-config'
         (hash
@@ -97,6 +138,7 @@ as |nspace dc items topology|}}
       }}
     as |config|>
     {{#if config.data}}
+
       <TopologyMetrics
         @nspace={{nspace}}
         @dc={{dc}}
@@ -117,6 +159,7 @@ as |nspace dc items topology|}}
         @hasMetricsProvider={{gt config.data.metrics_provider.length 0}}
         @oncreate={{route-action 'createIntention'}}
       />
+
     {{/if}}
     </DataSource>
   </div>

ui/packages/consul-ui/translations/components/consul/en-us.yaml

@@ -123,34 +123,6 @@ topology-metrics:
     routing-config:
       tooltip: This is not a registered Consul service. It’s a routing configuration that routes traffic to real services in Consul.
       text: Routing configuration
-  notice:
-    limited-access:
-      header: Limited Access
-      body: This service may have dependencies you won’t see because you don’t have access to them.
-    default-allow:
-      header: Intentions are set to default allow
-      body: Your Intention settings are currently set to default allow. This means that this view will show connections to every service in your cluster. We recommend changing your Intention settings to default deny and creating specific Intentions for upstream and downstream services for this view to be useful.
-      footer:
-        name: Edit intentions
-        URL: dc.services.show.intentions
-    not-defined-intention:
-      header: Connections are not explicitly defined
-      body: There appears to be an Intention allowing traffic, but the services are unable to communicate until that connection is enabled by defining an explicit upstream or proxies are set to 'transparent' mode.
-      footer: Read the documentation
-    wildcard-intention:
-      header: Permissive Intention
-      body: One or more of your Intentions are set to allow traffic to and/or from all other services in a namespace. This Topology view will show all of those connections if that remains unchanged. We recommend setting more specific Intentions for upstream and downstream services to make this vizualization more useful.
-      footer:
-        name: Edit intentions
-        URL: dc.services.show.intentions
-    no-dependencies:
-      header: No dependencies
-      body: The service you are viewing currently has no dependencies. You will only see metrics for the current service until dependencies are added.
-      footer: Read the documentation
-    acls-disabled:
-      header: Enable ACLs
-      body: This connect-native service may have dependencies, but Consul isn't aware of them when ACLs are disabled. Enable ACLs to make this view more useful.
-      footer: Read the documentation
   popover:
     l7:
       header: Layer 7 permissions

ui/packages/consul-ui/translations/routes/en-us.yaml

@@ -17,6 +17,49 @@ dc:
             </p>
   services:
     show:
+      topology:
+        notices:
+          open: Expand Banners
+          close: Collapse Banners
+        notice:
+          filtered-by-acls:
+            header: Limited Access
+            body: This service may have dependencies you won’t see because you don’t have access to them.
+          default-allow:
+            header: Intentions are set to default allow
+            body: Your Intention settings are currently set to default allow. This means that this view will show connections to every service in your cluster. We recommend changing your Intention settings to default deny and creating specific Intentions for upstream and downstream services for this view to be useful.
+            footer: |
+              <p>
+                <a href="{route_intentions}">Edit Intentions</a>
+              </p>
+          wildcard-intention:
+            header: Permissive Intention
+            body: One or more of your Intentions are set to allow traffic to and/or from all other services in a namespace. This Topology view will show all of those connections if that remains unchanged. We recommend setting more specific Intentions for upstream and downstream services to make this vizualization more useful.
+            footer: |
+              <p>
+                <a href="{route_intentions}">Edit Intentions</a>
+              </p>
+          not-defined-intention:
+            header: Connections are not explicitly defined
+            body: There appears to be an Intention allowing traffic, but the services are unable to communicate until that connection is enabled by defining an explicit upstream or proxies are set to 'transparent' mode.
+            footer: |
+              <p>
+                <a href="{CONSUL_DOCS_URL}/connect/registration/service-registration#upstreams" target="_blank" rel="noopener noreferrer">Read the documentation</a>
+              </p>
+          no-dependencies:
+            header: No dependencies
+            body: The service you are viewing currently has no dependencies. You will only see metrics for the current service until dependencies are added.
+            footer: |
+              <p>
+                <a href="{CONSUL_DOCS_URL}/connect/registration/service-registration#upstream-configuration-reference" target="_blank" rel="noopener noreferrer">Read the documentation</a>
+              </p>
+          acls-disabled:
+            header: Enable ACLs
+            body: This connect-native service may have dependencies, but Consul isn't aware of them when ACLs are disabled. Enable ACLs to make this view more useful.
+            footer: |
+              <p>
+                <a href="{CONSUL_DOCS_URL}/security/acl/acl-system#configuring-acls" target="_blank" rel="noopener noreferrer">Read the documentation</a>
+              </p>
       upstreams:
         intro: |
           <p>