Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add links to legend items in allocation-summary #11820

Merged
merged 8 commits into from
Jan 14, 2022
3 changes: 3 additions & 0 deletions .changelog/11820.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: add links to legend items in allocation-summary
```
45 changes: 40 additions & 5 deletions ui/app/components/allocation-status-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,24 @@ export default class AllocationStatusBar extends DistributionBar {
layoutName = 'components/distribution-bar';

allocationContainer = null;
job = null;

'data-test-allocation-status-bar' = true;

generateLegendLink(job, status) {
if (!job || status === 'queued') return null;

return {
queryParams: {
status: JSON.stringify([status]),
namespace: job.belongsTo('namespace').id(),
},
};
}

@computed(
'allocationContainer.{queuedAllocs,completeAllocs,failedAllocs,runningAllocs,startingAllocs}'
'allocationContainer.{queuedAllocs,completeAllocs,failedAllocs,runningAllocs,startingAllocs}',
'job.namespace'
)
get data() {
if (!this.allocationContainer) {
Expand All @@ -25,21 +38,43 @@ export default class AllocationStatusBar extends DistributionBar {
'lostAllocs'
);
return [
{ label: 'Queued', value: allocs.queuedAllocs, className: 'queued' },
{
label: 'Queued',
value: allocs.queuedAllocs,
className: 'queued',
legendLink: this.generateLegendLink(this.job, 'queued'),
},
{
label: 'Starting',
value: allocs.startingAllocs,
className: 'starting',
layers: 2,
legendLink: this.generateLegendLink(this.job, 'starting'),
},
{
label: 'Running',
value: allocs.runningAllocs,
className: 'running',
legendLink: this.generateLegendLink(this.job, 'running'),
},
{ label: 'Running', value: allocs.runningAllocs, className: 'running' },
{
label: 'Complete',
value: allocs.completeAllocs,
className: 'complete',
legendLink: this.generateLegendLink(this.job, 'complete'),
},
{
label: 'Failed',
value: allocs.failedAllocs,
className: 'failed',
legendLink: this.generateLegendLink(this.job, 'failed'),
},
{
label: 'Lost',
value: allocs.lostAllocs,
className: 'lost',
legendLink: this.generateLegendLink(this.job, 'lost'),
},
{ label: 'Failed', value: allocs.failedAllocs, className: 'failed' },
{ label: 'Lost', value: allocs.lostAllocs, className: 'lost' },
];
}
}
4 changes: 0 additions & 4 deletions ui/app/styles/charts/distribution-bar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
display: block;
background-color: transparent;
transition: background-color 0.1s ease-in-out;
border: 1px solid $grey-blue;
padding: 0.25em 0.75em;
margin: 0.25em;
border-radius: $radius;
Expand Down Expand Up @@ -98,8 +97,6 @@
&.is-clickable {
a {
display: block;
text-decoration: none;
color: inherit;
}

&:not(.is-empty) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can remove this rule now? The background colour change on hover may not be necessary anymore.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought:

Let's wait on styling and UX changes until the Nomad Design Refresh slated for 2.0. Otherwise, we'll go back and forth on changes for a long time when I can prioritize other higher value activities in the meantime. Also... we don't have visual regression testing in place, I'd rather focus on getting that up prior to making changes so we don't get blindsided by changes.

Expand All @@ -111,7 +108,6 @@

&.is-empty {
color: darken($grey-blue, 20%);
border: none;

.label {
color: darken($grey-blue, 20%);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
{{this.jobClientStatus.totalNodes}}
</span>
{{/if}}
<span class="tooltip multiline" aria-label="Aggreate status of job's allocations in each client.">
<span
class="tooltip multiline"
aria-label="Aggreate status of job's allocations in each client."
>
{{x-icon "info-circle-outline" class="is-faded"}}
</span>
</div>
Expand Down Expand Up @@ -46,9 +49,20 @@
>
<ol data-test-legend class="legend">
{{#each chart.data as |datum index|}}
<li data-test-legent-label="{{datum.className}}" class="{{datum.className}} {{if (eq datum.label chart.activeDatum.label) "is-active"}} {{if (eq datum.value 0) "is-empty" "is-clickable"}}">
<li
data-test-legend-label="{{datum.className}}"
class="{{datum.className}}

{{if (eq datum.label chart.activeDatum.label) "is-active"}}

{{if (eq datum.value 0) "is-empty" "is-clickable"}}"
>
{{#if (gt datum.value 0)}}
<LinkTo @route="jobs.job.clients" @model={{this.job}} @query={{datum.legendLink.queryParams}}>
<LinkTo
@route="jobs.job.clients"
@model={{this.job}}
@query={{datum.legendLink.queryParams}}
>
<JobPage::Parts::SummaryLegendItem @datum={{datum}} @index={{index}} />
</LinkTo>
{{else}}
Expand All @@ -60,4 +74,4 @@
</JobClientStatusBar>
{{/if}}
</a.body>
</ListAccordion>
</ListAccordion>
14 changes: 10 additions & 4 deletions ui/app/templates/components/job-page/parts/summary-legend-item.hbs
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
<div class="legend-item">
<span class="color-swatch {{if @datum.className @datum.className (concat "swatch-" @index)}}" />
<span
class="color-swatch {{if @datum.className @datum.className (concat "swatch-" @index)}}"
></span>
<span class="text">
<span class="value" data-test-legend-value="{{@datum.className}}">{{@datum.value}}</span>
<span class="label">{{@datum.label}}</span>
<span class="value" data-test-legend-value="{{@datum.className}}">
{{@datum.value}}
</span>
<span>
{{@datum.label}}
</span>
Comment on lines -2 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-formater strikes again 😄

I think this file didn't actually change?

</span>
{{#if @datum.help}}
<span class="tooltip multiline" role="tooltip" aria-label="{{@datum.help}}">
{{x-icon "info-circle-outline" class="is-faded"}}
</span>
{{/if}}
</div>
</div>
70 changes: 51 additions & 19 deletions ui/app/templates/components/job-page/parts/summary.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,58 @@
</div>
</a.head>
<a.body>
{{#component
(if a.item.hasChildren "children-status-bar" "allocation-status-bar")
allocationContainer=a.item.summary
job=a.item.summary
onSliceClick=this.onSliceClick
class="split-view" as |chart|
}}
<ol data-test-legend class="legend">
{{#each chart.data as |datum index|}}
<li
class="{{datum.className}}
{{#if a.item.hasChildren}}
<ChildrenStatusBar
@allocationContainer={{a.item.summary}}
@job={{a.item.summary}}
@class="split-view" as |chart|
>
<ol data-test-legend class="legend">
{{#each chart.data as |datum index|}}
<li
class="{{datum.className}}

{{if (eq datum.label chart.activeDatum.label) "is-active"}}
{{if (eq datum.label chart.activeDatum.label) "is-active"}}

{{if (eq datum.value 0) "is-empty"}}"
>
<JobPage::Parts::SummaryLegendItem @datum={{datum}} @index={{index}} />
</li>
{{/each}}
</ol>
{{/component}}
{{if (eq datum.value 0) "is-empty"}}"
>
<JobPage::Parts::SummaryLegendItem @datum={{datum}} @index={{index}} />
</li>
{{/each}}
</ol>
</ChildrenStatusBar>
{{else}}
<AllocationStatusBar
@allocationContainer={{a.item.summary}}
@job={{this.job}}
@onSliceClick={{this.onSliceClick}}
@class="split-view" as |chart|
>
<ol data-test-legend class="legend">
{{#each chart.data as |datum index|}}
<li
data-test-legend-label="{{datum.className}}"
class="{{datum.className}}

{{if (eq datum.label chart.activeDatum.label) "is-active"}}

{{if (eq datum.value 0) "is-empty" "is-clickable"}}"
>
{{#if (and (gt datum.value 0) datum.legendLink)}}
<LinkTo
@route="jobs.job.allocations"
@model={{this.job}}
@query={{datum.legendLink.queryParams}}
>
<JobPage::Parts::SummaryLegendItem @datum={{datum}} @index={{index}} />
</LinkTo>
{{else}}
<JobPage::Parts::SummaryLegendItem @datum={{datum}} @index={{index}} />
{{/if}}
</li>
{{/each}}
</ol>
</AllocationStatusBar>
{{/if}}
</a.body>
</ListAccordion>
50 changes: 45 additions & 5 deletions ui/tests/helpers/module-for-job.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,14 @@ export default function moduleForJob(title, context, jobFactory, additionalTests

if (context === 'allocations') {
test('allocations for the job are shown in the overview', async function(assert) {
assert.ok(JobDetail.allocationsSummary, 'Allocations are shown in the summary section');
assert.notOk(JobDetail.childrenSummary, 'Children are not shown in the summary section');
assert.ok(
JobDetail.allocationsSummary.isPresent,
'Allocations are shown in the summary section'
);
assert.ok(
JobDetail.childrenSummary.isHidden,
'Children are not shown in the summary section'
);
});

test('clicking in an allocation row navigates to that allocation', async function(assert) {
Expand All @@ -109,13 +115,47 @@ export default function moduleForJob(title, context, jobFactory, additionalTests
'Allocation row links to allocation detail'
);
});

test('clicking legend item navigates to a pre-filtered allocations table', async function(assert) {
const legendItem = JobDetail.allocationsSummary.legend.clickableItems[1];
const status = legendItem.label;
await legendItem.click();

const encodedStatus = encodeURIComponent(JSON.stringify([status]));
const expectedURL = new URL(
urlWithNamespace(`/jobs/${job.name}/clients?status=${encodedStatus}`, job.namespace),
window.location
);
const gotURL = new URL(currentURL(), window.location);
assert.deepEqual(gotURL.path, expectedURL.path);
assert.deepEqual(gotURL.searchParams, expectedURL.searchParams);
});

test('clicking in a slice takes you to a pre-filtered allocations table', async function(assert) {
const slice = JobDetail.allocationsSummary.slices[1];
const status = slice.label;
await slice.click();

const encodedStatus = encodeURIComponent(JSON.stringify([status]));
const expectedURL = new URL(
urlWithNamespace(`/jobs/${job.name}/allocations?status=${encodedStatus}`, job.namespace),
window.location
);
const gotURL = new URL(currentURL(), window.location);
assert.deepEqual(gotURL.pathname, expectedURL.pathname);

// Sort and compare URL query params.
gotURL.searchParams.sort();
expectedURL.searchParams.sort();
assert.equal(gotURL.searchParams.toString(), expectedURL.searchParams.toString());
});
}

if (context === 'children') {
test('children for the job are shown in the overview', async function(assert) {
assert.ok(JobDetail.childrenSummary, 'Children are shown in the summary section');
assert.notOk(
JobDetail.allocationsSummary,
assert.ok(JobDetail.childrenSummary.isPresent, 'Children are shown in the summary section');
assert.ok(
JobDetail.allocationsSummary.isHidden,
'Allocations are not shown in the summary section'
);
});
Expand Down
4 changes: 2 additions & 2 deletions ui/tests/pages/components/job-client-status-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ export default scope => ({
scope: '.legend',

items: collection('li', {
label: attribute('data-test-legent-label'),
label: attribute('data-test-legend-label'),
}),

clickableItems: collection('li.is-clickable', {
label: attribute('data-test-legent-label'),
label: attribute('data-test-legend-label'),
click: clickable('a'),
}),
},
Expand Down
8 changes: 4 additions & 4 deletions ui/tests/pages/jobs/detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ export default create({
tooltip: attribute('aria-label'),
},
},

childrenSummary: isPresent('[data-test-job-summary] [data-test-children-status-bar]'),
allocationsSummary: isPresent('[data-test-job-summary] [data-test-allocation-status-bar]'),

childrenSummary: jobClientStatusBar('[data-test-job-summary] [data-test-children-status-bar]'),
allocationsSummary: jobClientStatusBar(
'[data-test-job-summary] [data-test-allocation-status-bar]'
),
...allocations(),

viewAllAllocations: text('[data-test-view-all-allocations]'),
Expand Down