Merge ~caleb-ellis/maas:group-checkboxes into maas:master

Proposed by Caleb Ellis
Status: Superseded
Proposed branch: ~caleb-ellis/maas:group-checkboxes
Merge into: maas:master
Diff against target: 265 lines (+130/-14)
5 files modified
src/maasserver/static/js/angular/directives/machines_table.js (+31/-9)
src/maasserver/static/js/angular/directives/tests/test_machines_table.js (+82/-0)
src/maasserver/static/partials/machines-table.html (+7/-2)
src/maasserver/static/scss/_base_forms.scss (+1/-1)
src/maasserver/static/scss/_tables.scss (+9/-2)
Reviewer Review Type Date Requested Status
MAAS UI team Pending
Review via email: mp+366432@code.launchpad.net

This proposal has been superseded by a proposal from 2019-04-24.

Commit message

Add group checkboxes to machine list

Description of the change

## Done
- Added checkboxes to group headings, which when clicked either selects all unselected machines in a group, or unselects all machines in a group.
- Note there is currently a bug when using the "update your selection" link when action can't be performed on all machines, which has been present since the first grouping MP. This will get fixed as part of https://github.com/canonical-web-and-design/maas-squad/issues/1045

## QA
- Check that selecting a group selects all machines in that group and makes the background of the group white
- Check that unselecting a single machine in a group will cause the group checkbox to become unchecked

## Screenshots
### Partially selected
https://user-images.githubusercontent.com/25733845/56644794-5502d800-6674-11e9-9c94-977eabd67d68.png

### Fully selected
https://user-images.githubusercontent.com/25733845/56644810-5af8b900-6674-11e9-80d5-3b0d005cd9b8.png

To post a comment you must log in.

Unmerged commits

edd483a... by Caleb Ellis

Add group checkboxes to machine list

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/static/js/angular/directives/machines_table.js b/src/maasserver/static/js/angular/directives/machines_table.js
2index 710d9ea..c90d46f 100644
3--- a/src/maasserver/static/js/angular/directives/machines_table.js
4+++ b/src/maasserver/static/js/angular/directives/machines_table.js
5@@ -9,7 +9,7 @@
6 /* @ngInject */
7 function maasMachinesTable(
8 MachinesManager, NotificationsManager, UsersManager,
9- GeneralManager, $document, $window, $log) {
10+ GeneralManager, $filter, $document, $window, $log) {
11 return {
12 restrict: "E",
13 scope: {
14@@ -89,6 +89,7 @@ function maasMachinesTable(
15 "lock",
16 "unlock"
17 ];
18+
19 $scope.openMenu = "";
20
21 $scope.groupBy = (list, keyGetter) => {
22@@ -199,6 +200,22 @@ function maasMachinesTable(
23 $scope.updateAllChecked();
24 };
25
26+ $scope.toggleCheckGroup = groupLabel => {
27+ const machineGroup = $scope.groupedMachines.find(group => {
28+ return group.label === groupLabel
29+ });
30+ if ($scope.getGroupSelectedState(groupLabel)) {
31+ machineGroup.machines.forEach(machine => {
32+ MachinesManager.unselectItem(machine.system_id);
33+ });
34+ } else {
35+ machineGroup.machines.forEach(machine => {
36+ MachinesManager.selectItem(machine.system_id);
37+ });
38+ }
39+ $scope.updateAllChecked();
40+ };
41+
42 // Sorts the table by predicate.
43 $scope.sortTable = function(predicate) {
44 $scope.table.predicate = predicate;
45@@ -307,6 +324,13 @@ function maasMachinesTable(
46 }
47 };
48
49+ $scope.getGroupSelectedState = groupLabel => {
50+ const machineGroup = $scope.groupedMachines.find(group => {
51+ return group.label === groupLabel
52+ });
53+ return !machineGroup.machines.some(machine => !machine.$selected);
54+ };
55+
56 $scope.updateGroupedMachines = function(field) {
57 if ($scope.table.filteredMachines.length === 0) { return; }
58
59@@ -392,7 +416,7 @@ function maasMachinesTable(
60 ...(machines.get('Reserved') || [])
61 ]
62 }
63- ]
64+ ];
65 return;
66 }
67
68@@ -423,7 +447,7 @@ function maasMachinesTable(
69 label: 'none',
70 machines: $scope.table.filteredMachines
71 }
72- ]
73+ ];
74 return;
75 }
76
77@@ -438,12 +462,10 @@ function maasMachinesTable(
78 $scope.updateGroupedMachines($scope.groupByLabel);
79 });
80
81- // When the list of machines changes update grouping.
82- $scope.$watch("table.machines", function() {
83- if ($scope.groupByLabel !== 'none') {
84- $scope.updateGroupedMachines($scope.groupByLabel);
85- }
86- }, true);
87+ $scope.$watch("search", function() {
88+ $scope.table.filteredMachines
89+ = $filter('nodesFilter')($scope.table.machines, $scope.search);
90+ });
91
92 // Watch a simplified list of machines for changes to power state,
93 // then set transitional state accordingly.
94diff --git a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
95index a7dd821..5d153be 100644
96--- a/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
97+++ b/src/maasserver/static/js/angular/directives/tests/test_machines_table.js
98@@ -156,6 +156,51 @@ describe("maasMachinesTable", function() {
99 });
100 });
101
102+ describe("toggleCheckGroup", () => {
103+ it("selects all unselected machines in a group", () => {
104+ const directive = compileDirective();
105+ const scope = directive.isolateScope();
106+
107+ const machines = [makeMachine(), makeMachine(), makeMachine()];
108+ machines[0].status = 'New';
109+ machines[1].status = 'Broken';
110+ machines[2].status = 'New';
111+ scope.table.filteredMachines = machines;
112+ scope.groupedMachines = [
113+ { label: 'New', machines: [machines[0], machines[2]] },
114+ { label: 'Broken', machines: [machines[1]]}
115+ ];
116+ scope.toggleCheckGroup('New');
117+
118+ expect(machines[0].$selected).toBe(true);
119+ expect(machines[1].$selected).toBe(false);
120+ expect(machines[2].$selected).toBe(true);
121+ });
122+
123+ it("unselects all machines in a group if all are selected", () => {
124+ const directive = compileDirective();
125+ const scope = directive.isolateScope();
126+
127+ const machines = [makeMachine(), makeMachine(), makeMachine()];
128+ machines[0].status = 'New';
129+ machines[0].$selected = true;
130+ machines[1].status = 'Broken';
131+ machines[1].$selected = true;
132+ machines[2].status = 'New';
133+ machines[2].$selected = true;
134+ scope.table.filteredMachines = machines;
135+ scope.groupedMachines = [
136+ { label: 'New', machines: [machines[0], machines[2]] },
137+ { label: 'Broken', machines: [machines[1]]}
138+ ];
139+ scope.toggleCheckGroup('New');
140+
141+ expect(machines[0].$selected).toBe(false);
142+ expect(machines[1].$selected).toBe(true);
143+ expect(machines[2].$selected).toBe(false);
144+ });
145+ });
146+
147 describe("toggleChecked", function() {
148
149 it("selects machine", function() {
150@@ -886,4 +931,41 @@ describe("maasMachinesTable", function() {
151 { label: 'user1', machines: [machines[1]]}]);
152 });
153 });
154+
155+ describe("getGroupSelectedState", () => {
156+ it("returns true if all machines in a group are selected", () => {
157+ const directive = compileDirective();
158+ const scope = directive.isolateScope();
159+
160+ const machines = [makeMachine(), makeMachine()];
161+ machines[0].status = 'New';
162+ machines[0].$selected = true;
163+ machines[1].status = 'New';
164+ machines[1].$selected = true;
165+ scope.table.filteredMachines = machines;
166+ scope.groupedMachines = [
167+ { label: 'New', machines: [machines[0], machines[1]] }
168+ ];
169+ scope.getGroupSelectedState('New');
170+
171+ expect(scope.getGroupSelectedState('New')).toBe(true);
172+ });
173+
174+ it("returns false if not all machines in a group are selected", () => {
175+ const directive = compileDirective();
176+ const scope = directive.isolateScope();
177+
178+ const machines = [makeMachine(), makeMachine()];
179+ machines[0].status = 'New';
180+ machines[0].$selected = true;
181+ machines[1].status = 'New';
182+ scope.table.filteredMachines = machines;
183+ scope.groupedMachines = [
184+ { label: 'New', machines: [machines[0], machines[1]] }
185+ ];
186+ scope.getGroupSelectedState('New');
187+
188+ expect(scope.getGroupSelectedState('New')).toBe(false);
189+ });
190+ });
191 });
192diff --git a/src/maasserver/static/partials/machines-table.html b/src/maasserver/static/partials/machines-table.html
193index 16a1748..a5ccc63 100644
194--- a/src/maasserver/static/partials/machines-table.html
195+++ b/src/maasserver/static/partials/machines-table.html
196@@ -50,9 +50,14 @@
197 <th class="p-table__col--disks u-align--right" role="columnheader" data-ng-click="sortTable('physical_disk_count')" data-ng-class="{'is-sorted': table.predicate === 'physical_disk_count', 'sort-asc': table.reverse === false, 'sort-desc': table.reverse === true}" title="Disks">Disks</th>
198 <th class="p-table__col--storage u-align--right" role="columnheader" data-ng-click="sortTable('storage')" data-ng-class="{'is-sorted': table.predicate === 'storage', 'sort-asc': table.reverse === false, 'sort-desc': table.reverse === true}" title="Storage">Storage</th>
199 </tr>
200- <tr class="p-table__group p-table__row" data-ng-if="group.label != 'none' && group.machines.length && !actionOption && search != 'in:(Selected)'">
201+ <tr
202+ class="p-table__group p-table__row"
203+ data-ng-if="group.label != 'none'"
204+ data-ng-class="{ 'is-active': getGroupSelectedState(group.label) }"
205+ >
206 <th class="p-table__group-label p-table__col--name p-double-row u-align--left">
207- {$ group.label $}
208+ <input type="checkbox" id="{$ group.label $}" data-ng-click="toggleCheckGroup(group.label)" data-ng-checked="getGroupSelectedState(group.label)" />
209+ <label class="p-checkbox--action u-no-margin--bottom" for="{$ group.label $}">{$ group.label $}</label>
210 </th>
211 <th class="p-table__col--power" role="columnheader"></th>
212 <th class="p-table__col--status p-double-row" title="Status"></th>
213diff --git a/src/maasserver/static/scss/_base_forms.scss b/src/maasserver/static/scss/_base_forms.scss
214index 79be0e7..f8c3c38 100644
215--- a/src/maasserver/static/scss/_base_forms.scss
216+++ b/src/maasserver/static/scss/_base_forms.scss
217@@ -57,7 +57,7 @@
218 }
219 }
220
221- th & + label {
222+ th:not(.p-table__group-label) & + label {
223 &::before {
224 top: .65em;
225 }
226diff --git a/src/maasserver/static/scss/_tables.scss b/src/maasserver/static/scss/_tables.scss
227index 007cc4d..3075f4c 100644
228--- a/src/maasserver/static/scss/_tables.scss
229+++ b/src/maasserver/static/scss/_tables.scss
230@@ -378,7 +378,7 @@
231 .p-table__group-label {
232 color: $color-dark;
233 font-size: 1rem;
234- padding: $spv-inter--regular 0 $spv-inter--regular $group-padding-left;
235+ padding: $spv-inter--regular 0 $spv-inter--regular $sph-intra--condensed;
236 text-transform: none;
237 }
238 }
239@@ -386,11 +386,14 @@
240 .p-table__row {
241 position: relative;
242
243+ &::after {
244+ content: '';
245+ }
246+
247 &.is-grouped {
248 border: 0;
249
250 &::after {
251- content: '';
252 position: absolute;
253 left: $group-padding-left;
254 right: 0;
255@@ -474,6 +477,10 @@
256 max-width: 500px;
257 white-space: inherit;
258 }
259+
260+ &:last-of-type::after {
261+ display: none;
262+ }
263 }
264
265 .p-table--controller-interfaces {

Subscribers

People subscribed via source and target branches