From e9d44c2d27ac7f03c094e44ecbd526062ead345f Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 09:38:13 -0500 Subject: [PATCH 1/6] feat: add title to default breadcrumb component --- ui/app/components/breadcrumbs/default.hbs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ui/app/components/breadcrumbs/default.hbs b/ui/app/components/breadcrumbs/default.hbs index 161adee03c2e..baf06cc7801b 100644 --- a/ui/app/components/breadcrumbs/default.hbs +++ b/ui/app/components/breadcrumbs/default.hbs @@ -1,5 +1,16 @@
  • - {{@crumb.label}} + {{#if @crumb.title}} +
    +
    + {{@crumb.title}} +
    +
    + {{@crumb.label}} +
    +
    + {{else}} + {{@crumb.label}} + {{/if}}
  • \ No newline at end of file From c6aae0ca4909f56bea7c6ca3330769c456c98834 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 09:38:41 -0500 Subject: [PATCH 2/6] style: add styling for title on breadcrumbs --- ui/app/styles/core/breadcrumb.scss | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ui/app/styles/core/breadcrumb.scss b/ui/app/styles/core/breadcrumb.scss index 092bb022149a..c1b7357ca740 100644 --- a/ui/app/styles/core/breadcrumb.scss +++ b/ui/app/styles/core/breadcrumb.scss @@ -11,7 +11,23 @@ } } + ul { + align-items: center; + } + + li::before { + font-size: x-large; + } + li.is-active a { opacity: 1; } + + dl dd { + font-size: medium; + } + + dl dt { + font-size: small; + } } From aed6fd36d9035935047394ecd8e4e85f0f754e91 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 09:40:43 -0500 Subject: [PATCH 3/6] refact: add title to breadcrumb generator All breadcrumbs do not need a title property because some views drill down by using a tab-based UI (e.g. CSI volumes and the Job Overview) The goal is to help us identify breadcrumbs that are non-descriptive (i.e. breadcrumbs that display as an ID). --- ui/app/controllers/allocations/allocation.js | 2 ++ ui/app/controllers/allocations/allocation/task.js | 1 + ui/app/controllers/clients/client.js | 1 + ui/app/controllers/jobs/job/task-group.js | 1 + ui/app/controllers/servers/server.js | 1 + 5 files changed, 6 insertions(+) diff --git a/ui/app/controllers/allocations/allocation.js b/ui/app/controllers/allocations/allocation.js index d2c2aee40345..b29d7b6cb02a 100644 --- a/ui/app/controllers/allocations/allocation.js +++ b/ui/app/controllers/allocations/allocation.js @@ -33,10 +33,12 @@ export default class AllocationsAllocationController extends Controller { { label: 'Jobs', args: ['jobs.index', jobQueryParams] }, { type: 'job', job: job }, { + title: 'Task Group', label: allocation.taskGroupName, args: ['jobs.job.task-group', job.plainId, allocation.taskGroupName, jobQueryParams], }, { + title: 'Allocation', label: allocation.shortId, args: ['allocations.allocation', allocation], }, diff --git a/ui/app/controllers/allocations/allocation/task.js b/ui/app/controllers/allocations/allocation/task.js index 0d19fd42f238..1fefdd66c5a6 100644 --- a/ui/app/controllers/allocations/allocation/task.js +++ b/ui/app/controllers/allocations/allocation/task.js @@ -6,6 +6,7 @@ export default class AllocationsAllocationTaskController extends Controller { if (!model) return []; return [ { + title: 'Task', label: model.get('name'), args: ['allocations.allocation.task', model.get('allocation'), model], }, diff --git a/ui/app/controllers/clients/client.js b/ui/app/controllers/clients/client.js index aa558c80b0bb..e13e1e9c19b1 100644 --- a/ui/app/controllers/clients/client.js +++ b/ui/app/controllers/clients/client.js @@ -6,6 +6,7 @@ export default class ClientsClientController extends Controller { if (!model) return []; return [ { + title: 'Client', label: model.get('shortId'), args: ['clients.client', model.get('id')], }, diff --git a/ui/app/controllers/jobs/job/task-group.js b/ui/app/controllers/jobs/job/task-group.js index 6d8b745fffd4..4afc4cd7471e 100644 --- a/ui/app/controllers/jobs/job/task-group.js +++ b/ui/app/controllers/jobs/job/task-group.js @@ -91,6 +91,7 @@ export default class TaskGroupController extends Controller.extend( if (!model) return []; return [ { + title: 'Task Group', label: model.get('name'), args: [ 'jobs.job.task-group', diff --git a/ui/app/controllers/servers/server.js b/ui/app/controllers/servers/server.js index 0f99b8985c17..e01310c1a54f 100644 --- a/ui/app/controllers/servers/server.js +++ b/ui/app/controllers/servers/server.js @@ -7,6 +7,7 @@ export default class ServersServerController extends Controller { get breadcrumb() { return { + title: 'Server', label: this.server.name, args: ['servers.server', this.server.id], }; From 03a0f41983d29ec22700aa89c09e5a9fec0cfee5 Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 09:54:46 -0500 Subject: [PATCH 4/6] feat: handle title behavior for job breadcrumb --- ui/app/components/breadcrumbs/job.hbs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/ui/app/components/breadcrumbs/job.hbs b/ui/app/components/breadcrumbs/job.hbs index d6578baf6970..87e5e2a7b81c 100644 --- a/ui/app/components/breadcrumbs/job.hbs +++ b/ui/app/components/breadcrumbs/job.hbs @@ -16,7 +16,14 @@ @query={{hash namespace=(or trigger.data.result.namespace.name "default")}} data-test-breadcrumb={{"jobs.job.index"}} > - {{trigger.data.result.trimmedName}} +
    +
    + Parent Job +
    +
    + {{trigger.data.result.trimmedName}} +
    +
    {{/if}} @@ -28,7 +35,14 @@ data-test-breadcrumb={{"jobs.job.index"}} data-test-job-breadcrumb > - {{this.job.trimmedName}} +
    +
    + {{if this.job.hasChildren "Parent Job" "Job"}} +
    +
    + {{this.job.trimmedName}} +
    +
    {{/if}} From b58df56c750781e767c2a5663c67d00a2ecd6b2b Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 10:12:29 -0500 Subject: [PATCH 5/6] style: centering and spacing for titled breadcrumbs --- ui/app/styles/core/breadcrumb.scss | 1 + ui/app/styles/core/navbar.scss | 1 + 2 files changed, 2 insertions(+) diff --git a/ui/app/styles/core/breadcrumb.scss b/ui/app/styles/core/breadcrumb.scss index c1b7357ca740..f2b2f1413f3f 100644 --- a/ui/app/styles/core/breadcrumb.scss +++ b/ui/app/styles/core/breadcrumb.scss @@ -24,6 +24,7 @@ } dl dd { + margin: -4px 0px; font-size: medium; } diff --git a/ui/app/styles/core/navbar.scss b/ui/app/styles/core/navbar.scss index 840d8d1e9bee..dffa0ce019a1 100644 --- a/ui/app/styles/core/navbar.scss +++ b/ui/app/styles/core/navbar.scss @@ -1,5 +1,6 @@ .navbar { display: flex; + align-items: center; &.is-primary { background: linear-gradient(to right, $nomad-green-darker, $nomad-green-dark); From c628d7f37ea83f185b29912fb4785e5ffcf283de Mon Sep 17 00:00:00 2001 From: Jai Bhagat Date: Thu, 16 Dec 2021 10:28:46 -0500 Subject: [PATCH 6/6] fix: test specs should expect to receive breadcrumb titles --- ui/tests/acceptance/client-detail-test.js | 4 ++-- ui/tests/acceptance/client-monitor-test.js | 2 +- ui/tests/acceptance/server-monitor-test.js | 2 +- ui/tests/acceptance/task-detail-test.js | 8 ++++---- ui/tests/acceptance/task-group-detail-test.js | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/ui/tests/acceptance/client-detail-test.js b/ui/tests/acceptance/client-detail-test.js index 58ccb6ed2efa..c2ceb0a5f66b 100644 --- a/ui/tests/acceptance/client-detail-test.js +++ b/ui/tests/acceptance/client-detail-test.js @@ -68,8 +68,8 @@ module('Acceptance | client detail', function(hooks) { ); assert.equal( Layout.breadcrumbFor('clients.client').text, - node.id.split('-')[0], - 'Second breadcrumb says the node short id' + `Client ${node.id.split('-')[0]}`, + 'Second breadcrumb is a titled breadcrumb saying the node short id' ); await Layout.breadcrumbFor('clients.index').visit(); assert.equal(currentURL(), '/clients', 'First breadcrumb links back to clients'); diff --git a/ui/tests/acceptance/client-monitor-test.js b/ui/tests/acceptance/client-monitor-test.js index 49523009ce8b..c795345e600d 100644 --- a/ui/tests/acceptance/client-monitor-test.js +++ b/ui/tests/acceptance/client-monitor-test.js @@ -36,7 +36,7 @@ module('Acceptance | client monitor', function(hooks) { await ClientMonitor.visit({ id: node.id }); assert.equal(Layout.breadcrumbFor('clients.index').text, 'Clients'); - assert.equal(Layout.breadcrumbFor('clients.client').text, node.id.split('-')[0]); + assert.equal(Layout.breadcrumbFor('clients.client').text, `Client ${node.id.split('-')[0]}`); await Layout.breadcrumbFor('clients.index').visit(); assert.equal(currentURL(), '/clients'); diff --git a/ui/tests/acceptance/server-monitor-test.js b/ui/tests/acceptance/server-monitor-test.js index 00bcc6b09536..0f6aeed3e02d 100644 --- a/ui/tests/acceptance/server-monitor-test.js +++ b/ui/tests/acceptance/server-monitor-test.js @@ -38,7 +38,7 @@ module('Acceptance | server monitor', function(hooks) { 'Servers', 'The page should read the breadcrumb Servers' ); - assert.equal(Layout.breadcrumbFor('servers.server').text, agent.name); + assert.equal(Layout.breadcrumbFor('servers.server').text, `Server ${agent.name}`); await Layout.breadcrumbFor('servers.index').visit(); assert.equal(currentURL(), '/servers'); diff --git a/ui/tests/acceptance/task-detail-test.js b/ui/tests/acceptance/task-detail-test.js index fa74af8030e1..becc9c212f1f 100644 --- a/ui/tests/acceptance/task-detail-test.js +++ b/ui/tests/acceptance/task-detail-test.js @@ -62,22 +62,22 @@ module('Acceptance | task detail', function(hooks) { await waitFor('[data-test-job-breadcrumb]'); assert.equal( Layout.breadcrumbFor('jobs.job.index').text, - job.name, + `Job ${job.name}`, 'Job is the second breadcrumb' ); assert.equal( Layout.breadcrumbFor('jobs.job.task-group').text, - taskGroup, + `Task Group ${taskGroup}`, 'Task Group is the third breadcrumb' ); assert.equal( Layout.breadcrumbFor('allocations.allocation').text, - shortId, + `Allocation ${shortId}`, 'Allocation short id is the fourth breadcrumb' ); assert.equal( Layout.breadcrumbFor('allocations.allocation.task').text, - task.name, + `Task ${task.name}`, 'Task name is the fifth breadcrumb' ); diff --git a/ui/tests/acceptance/task-group-detail-test.js b/ui/tests/acceptance/task-group-detail-test.js index a4533f532b73..4b3866984e72 100644 --- a/ui/tests/acceptance/task-group-detail-test.js +++ b/ui/tests/acceptance/task-group-detail-test.js @@ -122,12 +122,12 @@ module('Acceptance | task group detail', function(hooks) { assert.equal(Layout.breadcrumbFor('jobs.index').text, 'Jobs', 'First breadcrumb says jobs'); assert.equal( Layout.breadcrumbFor('jobs.job.index').text, - job.name, + `Job ${job.name}`, 'Second breadcrumb says the job name' ); assert.equal( Layout.breadcrumbFor('jobs.job.task-group').text, - taskGroup.name, + `Task Group ${taskGroup.name}`, 'Third breadcrumb says the job name' ); });