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
This commit is contained in:
John Cowen 2022-02-18 17:16:03 +00:00 committed by GitHub
parent 999992eb3c
commit 6e0eddd841
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 187 additions and 244 deletions

3
.changelog/12305.txt Normal file
View File

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

View File

@ -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)
---

View File

@ -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}}

View File

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

View File

@ -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;
}
}

View File

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

View File

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

View File

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

View File

@ -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>

View File

@ -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 { .topology-container {
color: rgb(var(--tone-gray-700)); color: rgb(var(--tone-gray-700));
} }

View File

@ -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);

View File

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

View File

@ -27,66 +27,107 @@ as |route|>
loader.data loader.data
as |nspace dc items topology|}} as |nspace dc items topology|}}
<div class="tab-section"> <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 <DataSource
@src={{uri '/${partition}/${nspace}/${dc}/ui-config' @src={{uri '/${partition}/${nspace}/${dc}/ui-config'
(hash (hash
@ -97,6 +138,7 @@ as |nspace dc items topology|}}
}} }}
as |config|> as |config|>
{{#if config.data}} {{#if config.data}}
<TopologyMetrics <TopologyMetrics
@nspace={{nspace}} @nspace={{nspace}}
@dc={{dc}} @dc={{dc}}
@ -117,6 +159,7 @@ as |nspace dc items topology|}}
@hasMetricsProvider={{gt config.data.metrics_provider.length 0}} @hasMetricsProvider={{gt config.data.metrics_provider.length 0}}
@oncreate={{route-action 'createIntention'}} @oncreate={{route-action 'createIntention'}}
/> />
{{/if}} {{/if}}
</DataSource> </DataSource>
</div> </div>

View File

@ -123,34 +123,6 @@ topology-metrics:
routing-config: routing-config:
tooltip: This is not a registered Consul service. Its a routing configuration that routes traffic to real services in Consul. tooltip: This is not a registered Consul service. Its a routing configuration that routes traffic to real services in Consul.
text: Routing configuration text: Routing configuration
notice:
limited-access:
header: Limited Access
body: This service may have dependencies you wont see because you dont 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: popover:
l7: l7:
header: Layer 7 permissions header: Layer 7 permissions

View File

@ -17,6 +17,49 @@ dc:
</p> </p>
services: services:
show: show:
topology:
notices:
open: Expand Banners
close: Collapse Banners
notice:
filtered-by-acls:
header: Limited Access
body: This service may have dependencies you wont see because you dont 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: upstreams:
intro: | intro: |
<p> <p>