[ui] UI actions implementation on task index #19036

philrenaud · 2023-11-08T19:57:35Z

Resolves #19035

Adds an Actions dropdown on the task index page

Note: This formerly resolved #19018 as well, but we decided to work toward a design (persistent action service sidebar) that makes this undesirable.

philrenaud · 2023-11-08T19:58:55Z

ui/app/controllers/clients/client/index.js

@@ -338,13 +338,4 @@ export default class ClientController extends Controller.extend(
    }
  }
  // #endregion metadata
-


As a side-effect here, I've decided to always show the "Actions" column on clients/:clientid pages (under alloc table).

Before, this ended up looking a bit jarring and now it'll take up a small amount of space but with the benefit of a consistent look/feel. See also the {{true}} pass in the client index handlebars file.

+1 on avoid conditional rendering as it can make the UI surprising (why do I only see this thing sometimes?)

More over though, I think I would not have an Actions column at all. AllocationRow is already quite crowded and adding more rows just makes everything more "squishy" and knowing the number of actions in an alloc doesn't seem very relevant.

Side note: not sure if this will be fixed in a follow-up PR, but the column rendering seems wrong?

nomad/ui/app/templates/components/allocation-row.hbs

Lines 140 to 142 in ff928a8

{{#if this.allocation.job.actions.length}}

<td />

{{/if}}

Thanks; ended up removing the condition on allocation-row.hbs and kept the otherwise empty td there permanently.

The row is getting too crowded for sure, and I'll address that in a future PR; for now, the allocation row's actions column serves only to let the task-sub-row actions dropdown have space held for it:

If anything, maybe I should set the condition here for "show tasks" being turned on

Ahh OK, I saw actions.length somewhere and thought this would be a number 😅

Not sure why I didn't see the ... button 🤔

philrenaud · 2023-11-08T19:59:48Z

ui/app/templates/allocations/allocation/task/index.hbs

+  <Hds::PageHeader class="job-page-header" as |PH|>
+    <PH.Title data-test-title>


Further added the Helios PageHeader component here as the right-aligned buttons look nicer with it.

philrenaud · 2023-11-08T20:00:17Z

ui/app/styles/core/notifications.scss

@@ -7,7 +7,7 @@ section.notifications {
  position: fixed;
  bottom: 20px;
  right: 20px;
-  z-index: 100;
+  z-index: $z-notification;


A fun new z-index, since we want to launch actions from the sidebar, the notifications for those actions should also show up above the sidebar.

philrenaud · 2023-11-08T20:01:45Z

ui/app/services/nomad-actions.js

+  get hasActionPermissions() {
+    return this.can.can('exec allocation');
+  }
+}


Not much to this file yet, but expect big things soon as we use this service to persist running-in-the-background actions websockets among other things!

github-actions · 2023-11-08T20:17:42Z

Ember Test Audit comparison

	main	c8645da26adf40fd560f22a16393b3d20a6e4c97	change
passes	1538	1538	0
failures	0	0	0
flaky	0	0	0
duration	12m 39s 538ms	10m 51s 997ms	-1m 47s 541ms

lgfa29 · 2023-11-20T20:59:33Z

.github/workflows/test-ui.yml

@@ -2,14 +2,14 @@ name: test-ui
 on:
  pull_request:
    paths:
-      - 'ui'
+      - 'ui/**'


Not sure if this will cause a merge conflict, but may be better to undo it in this PR.

lgfa29 · 2023-11-20T21:26:20Z

ui/app/controllers/clients/client/index.js

@@ -338,13 +338,4 @@ export default class ClientController extends Controller.extend(
    }
  }
  // #endregion metadata
-


+1 on avoid conditional rendering as it can make the UI surprising (why do I only see this thing sometimes?)

More over though, I think I would not have an Actions column at all. AllocationRow is already quite crowded and adding more rows just makes everything more "squishy" and knowing the number of actions in an alloc doesn't seem very relevant.

Side note: not sure if this will be fixed in a follow-up PR, but the column rendering seems wrong?

nomad/ui/app/templates/components/allocation-row.hbs

Lines 140 to 142 in ff928a8

{{#if this.allocation.job.actions.length}}

<td />

{{/if}}

lgfa29 · 2023-11-20T21:31:06Z

ui/app/templates/allocations/allocation/task/index.hbs

+        {{#if this.shouldShowActions}}
+          <Hds::Dropdown class="actions-dropdown" as |dd|>
+            <dd.ToggleButton @color="secondary" @text="Actions" @size="medium" />
+            {{#each this.model.task.actions as |action|}}
+              <dd.Interactive {{on "click" (perform this.runAction action (get (object-at 0 action.allocations) "id"))}} @text="{{action.name}}" />
+            {{/each}}
+          </Hds::Dropdown>
+        {{/if}}


The spacing between the buttons at different levels is a bit jarring.

I think we're also missing the Actions button at the alloc and group level?

The spacing on the Task and Job page will be the norm for all pages; I'm rolling it the new PageHeader component on a page-by-page basis.

I hadn't considered the spacing issue too off-putting, but this comment makes me think should do a wholesale switch-over soon.

Actions on Group and Alloc level, I was planning to leave until the Actions service from #19084 is wrapped up; I'll handle that in a future PR.

github-actions · 2025-02-03T02:15:22Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

philrenaud requested review from lgfa29, mikenomitch and juliezzhou November 8, 2023 19:57

philrenaud self-assigned this Nov 8, 2023

philrenaud linked an issue Nov 8, 2023 that may be closed by this pull request

[ui] Actions implementation in Task Fly-out #19018

Closed

vercel bot deployed to Preview – nomad-storybook-and-ui November 8, 2023 19:58 View deployment

philrenaud commented Nov 8, 2023

View reviewed changes

philrenaud force-pushed the 19018-ui-actions-implementation-in-task-fly-out branch from ef2ef13 to 69f457a Compare November 8, 2023 20:00

philrenaud commented Nov 8, 2023

View reviewed changes

vercel bot deployed to Preview – nomad-storybook-and-ui November 8, 2023 20:04 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui November 9, 2023 14:49 View deployment

philrenaud force-pushed the 19018-ui-actions-implementation-in-task-fly-out branch 2 times, most recently from 43b0fe7 to 265df7c Compare November 10, 2023 16:27

philrenaud changed the title ~~[ui] UI actions implementation in task fly out and task index~~ [ui] UI actions implementation on task index Nov 10, 2023

vercel bot deployed to Preview – nomad-storybook-and-ui November 10, 2023 16:30 View deployment

vercel bot deployed to Preview – nomad-storybook-and-ui November 14, 2023 14:29 View deployment

lgfa29 approved these changes Nov 20, 2023

View reviewed changes

philrenaud added 4 commits November 21, 2023 09:45

Sidequest: always show actions column on client view

041e20a

Task sidebar and task index actions menus

e7ae692

Fixing test-fixture selector to no longer use old title class

6906ccf

Remove actions dropdown from fly-out

ce32d81

philrenaud force-pushed the 19018-ui-actions-implementation-in-task-fly-out branch from c8645da to 3c9c975 Compare November 21, 2023 14:46

PR feedback addressed

eb68dab

philrenaud force-pushed the 19018-ui-actions-implementation-in-task-fly-out branch from 3c9c975 to eb68dab Compare November 21, 2023 14:47

vercel bot deployed to Preview – nomad-storybook-and-ui November 21, 2023 14:50 View deployment

philrenaud merged commit e03e674 into main Nov 22, 2023

philrenaud deleted the 19018-ui-actions-implementation-in-task-fly-out branch November 22, 2023 06:18

github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ui] UI actions implementation on task index #19036

[ui] UI actions implementation on task index #19036

philrenaud commented Nov 8, 2023 •

edited

Loading

philrenaud Nov 8, 2023

lgfa29 Nov 20, 2023

philrenaud Nov 21, 2023

lgfa29 Nov 21, 2023

philrenaud Nov 8, 2023

philrenaud Nov 8, 2023

philrenaud Nov 8, 2023

github-actions bot commented Nov 8, 2023 •

edited

Loading

lgfa29 Nov 20, 2023

lgfa29 Nov 20, 2023

lgfa29 Nov 20, 2023

philrenaud Nov 21, 2023

philrenaud Nov 21, 2023

github-actions bot commented Feb 3, 2025

		<Hds::PageHeader class="job-page-header" as \|PH\|>
		<PH.Title data-test-title>

[ui] UI actions implementation on task index #19036

[ui] UI actions implementation on task index #19036

Conversation

philrenaud commented Nov 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Nov 8, 2023 • edited Loading

Ember Test Audit comparison

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 3, 2025

philrenaud commented Nov 8, 2023 •

edited

Loading

github-actions bot commented Nov 8, 2023 •

edited

Loading