Skip to content

Commit

Permalink
Fix heatmap node display issues (#2497)
Browse files Browse the repository at this point in the history 
* [WIP] Added new gradient colors to heatmap
TODO: update legend to collapse sub-states and pick better naming for the running task slot sub-states

* Categories can expand and contract now and are closed by default.
* colors of the categories will stay as subitem color
* [WIP] need to fix running state error (showing up as black instead of green)
* [WIP] need to add dropdown icon next to categories

* Fix selectState logic and add caret expansion and collapse identifier on categories

* Add logic for handling task slots for running state
* Change task slot colors to be more accessible

* Add 100% for heatmap legend and picked colors for heatmap
* clean up

* Fix typo

* Comment explaining state counter for running task usages

* Remove whitespace

* Fix unit tests
* [WIP] fix unit tests for running task overlay

* Fix async issue and fix state counter unit tests

* Add running task slot usage tests and fix percentages

* Add a few more tests for task slot usage overlay
  • Loading branch information
@cRui861
cRui861 committed May 12, 2022
1 parent 8cbb11e commit f7b5fcf
Show file tree
Hide file tree
Showing 16 changed files with 190 additions and 128 deletions.
2 changes: 1 addition & 1 deletion src/app/components/node/connect/property-display/node-property-display.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ describe("NodePropertyDisplay", () => {
click(checkboxEl.query(By.css("input")));

expect(checkboxEl.componentInstance.checked).toBe(false);
await new Promise(r => setTimeout(() => r(), 1000));
await new Promise<void>(r => setTimeout(() => r(), 1000));
expect(component.userConfig.isAdmin).toEqual(false);
});
});
Expand Down
8 changes: 0 additions & 8 deletions src/app/components/pool/graphs/heatmap/heatmap-color.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,6 @@ describe("Statecounter", () => {
expect(colors.get("running")).toEqual("#888888");
});

it("substate should have category color by default", () => {
expect(colors.get("starting")).toEqual("#777777");
expect(colors.get("rebooting")).toEqual("#777777");

expect(colors.get("startTaskFailed")).toEqual("#5555555");
expect(colors.get("unusable")).toEqual("#5555555");
});

describe("when highlighting a state", () => {
beforeEach(() => {
colors.updateColors("idle");
Expand Down
2 changes: 1 addition & 1 deletion src/app/components/pool/graphs/heatmap/heatmap-color.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export class HeatmapColor {
colors[item.state] = item.color;
} else {
for (const subitem of item.states) {
colors[subitem.state] = item.color;
colors[subitem.state] = subitem.color;
}
}
}
Expand Down
20 changes: 8 additions & 12 deletions src/app/components/pool/graphs/heatmap/legend/nodes-heatmap-legend.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,29 +54,25 @@ describe("NodesHeatmapLegendComponent", () => {
fixture.detectChanges();
});

it("should show all states", () => {
it("should show all states initially", () => {
const stateEls = fixture.debugElement.queryAll(By.css(".legend-item.state"));
expect(stateEls.length).toBe(2);
expect(stateEls[0].nativeElement.textContent).toContain("idle");
expect(stateEls[1].nativeElement.textContent).toContain("running");
});

it("should show all categories", () => {
it("should not show substates initially", () => {
const substates = fixture.debugElement.queryAll(By.css(".legend-subitem.state"));
expect(substates.length).toBe(0);
});

it("should show all categories initially", () => {
const categories = fixture.debugElement.queryAll(By.css(".legend-item.category"));
expect(categories.length).toBe(2);
expect(categories[0].nativeElement.textContent).toContain("Transition states");
expect(categories[1].nativeElement.textContent).toContain("Error states");
});

it("should show all substates", () => {
const categories = fixture.debugElement.queryAll(By.css(".legend-subitem.state"));
expect(categories.length).toBe(4);
expect(categories[0].nativeElement.textContent).toContain("rebooting");
expect(categories[1].nativeElement.textContent).toContain("starting");
expect(categories[2].nativeElement.textContent).toContain("starttaskfailed");
expect(categories[3].nativeElement.textContent).toContain("unusable");
});

it("should set the colors correctly", () => {
const stateEls = fixture.debugElement.queryAll(By.css(".legend-item.state .color"));
const bgColor1 = stateEls[0].nativeElement.style.backgroundColor;
Expand Down Expand Up @@ -104,7 +100,7 @@ describe("NodesHeatmapLegendComponent", () => {

click(stateEls[0]);
expect(selectedStateSpy).toHaveBeenCalledOnce();
expect(selectedStateSpy).toHaveBeenCalledWith(null);
expect(selectedStateSpy).toHaveBeenCalledWith('');
fixture.detectChanges();
expect(stateEls[0].classes["highlighted"]).toBe(false, "Should not be highlighted anymore");
});
Expand Down
21 changes: 16 additions & 5 deletions src/app/components/pool/graphs/heatmap/legend/nodes-heatmap-legend.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ export class NodesHeatmapLegendComponent {

@Input()
public set nodes(nodes: List<Node>) {
this.stateCounter.updateCount(nodes);
this.stateCounter.updateCount(nodes, this.pool);
}

@Input()
public colors: any;

public expandedCategory: string;
public stateCounter: StateCounter;
public highlightedState: string = null;
public highlightedState: string;

@Output()
public selectedStateChange = new EventEmitter();
Expand All @@ -37,10 +38,20 @@ export class NodesHeatmapLegendComponent {
this.stateCounter = new StateCounter();
}

public selectState(state: string) {
if (state === this.highlightedState) {
this.highlightedState = null;
/**
* Emits state changes for when a state, category, or sub-state is selected.
*
* @param state
* @param categoryParent categories and sub-states will have one
*/
public selectState(state: string, categoryParent: string = "") {
if (state === this.expandedCategory) {
this.expandedCategory = "";
this.highlightedState = "";
} else if (state === this.highlightedState) {
this.highlightedState = "";
} else {
this.expandedCategory = categoryParent;
this.highlightedState = state;
}
this.selectedStateChange.emit(this.highlightedState);
Expand Down
20 changes: 13 additions & 7 deletions src/app/components/pool/graphs/heatmap/legend/nodes-heatmap-legend.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@
<div class="label">{{item.state}} ({{stateCounter.get(item.state) | async}})</div>
</bl-clickable>
<div *ngIf="item.category">
<bl-clickable class="legend-item category" (do)="selectState(item.category)" [attr.aria-selected]="item.category === highlightedState" [class.highlighted]="item.category === highlightedState"
<bl-clickable class="legend-item category" (do)="selectState(item.category, item.category)" [attr.aria-selected]="item.category === highlightedState" [class.highlighted]="item.category === highlightedState"
(contextmenu)="openContextMenu(item)">
<div [style.backgroundColor]="item.color" class="color"></div>
<div class="label">{{item.label}}</div>
</bl-clickable>
<bl-clickable *ngFor="let subitem of item.states;trackBy: trackState" class="legend-subitem state" (do)="selectState(subitem.state)" [attr.aria-selected]="subitem.state === highlightedState" [class.highlighted]="subitem.state === highlightedState"
(contextmenu)="openContextMenu(item)">
<div [style.backgroundColor]="colors.get(subitem.state)" class="color"></div>
<div class="label">{{subitem.state}} ({{stateCounter.get(subitem.state) | async}})</div>
<div class="label">{{item.label}} ({{stateCounter.getCountforCategory(item.category)}})</div>
<div class="category-expansion">
<i class="fa" [class.fa-caret-down]="item.category === expandedCategory" [class.fa-caret-left]="item.category !== expandedCategory"></i>
</div>
</bl-clickable>
<div *ngIf="item.category === expandedCategory">
<div *ngIf="item.subtitle" class="subtitle">{{item.subtitle}}</div>
<bl-clickable *ngFor="let subitem of item.states;trackBy: trackState" class="legend-subitem state" (do)="selectState(subitem.state, item.category)" [attr.aria-selected]="subitem.state === highlightedState" [class.highlighted]="subitem.state === highlightedState"
(contextmenu)="openContextMenu(item)">
<div [style.backgroundColor]="colors.get(subitem.state)" class="color"></div>
<div class="label">{{subitem.state}} ({{stateCounter.get(subitem.state) | async}})</div>
</bl-clickable>
</div>
</div>
</div>
8 changes: 8 additions & 0 deletions src/app/components/pool/graphs/heatmap/legend/nodes-heatmap-legend.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ bl-nodes-heatmap-legend {
cursor: pointer;
}

.subtitle {
padding-left: 12%;
}

.legend-item, .legend-subitem {
display: flex;
align-items: center;
Expand All @@ -44,4 +48,8 @@ bl-nodes-heatmap-legend {
width: $color-width;
height: $color-width;
}

.category-expansion {
padding-right: 5%;
}
}
72 changes: 34 additions & 38 deletions src/app/components/pool/graphs/heatmap/nodes-heatmap.component.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,41 +154,33 @@ describe("NodesHeatmapComponent", () => {
});
});

describe("Running task overlay", () => {
it("when there is space should show 2 green stripes", () => {
testComponent.nodes = createNodes(2);
testComponent.pool = new Pool({ id: "pool-4", taskSlotsPerNode: 4 });
fixture.detectChanges();
const tiles = svg.selectAll("g.node-group");
expect(tiles.size()).toBe(2);
tiles.each((d, i, groups) => {
const group = d3.select(groups[i]);
const bg = group.select("g.taskslots");
const taskRects = bg.selectAll("rect");
expect(taskRects.size()).toBe(2, "Should have 2 rect");
taskRects.each((d, i, rects) => {
const rect = d3.select(rects[i]);
expect(rect.attr("height")).not.toBe("0");
expect(rect.attr("style")).toContain("fill: rgb(56, 142, 60);");
});
});
});

it("when there is no space should combine green stripes", () => {
testComponent.nodes = createNodes(2);
testComponent.pool = new Pool({ id: "pool-100", taskSlotsPerNode: 300 });
fixture.detectChanges();
const tiles = svg.selectAll("g.node-group");
expect(tiles.size()).toBe(2);
tiles.each((d, i, groups) => {
const group = d3.select(groups[i]);
const bg = group.select("g.taskslots");
const taskRects = bg.selectAll("rect");
expect(taskRects.size()).toBe(1, "Should have only 1 rect");
taskRects.each((d, i, rects) => {
const rect = d3.select(rects[i]);
expect(rect.attr("height")).not.toBe("0");
expect(rect.attr("style")).toContain("fill: rgb(56, 142, 60);");
describe("Running task slot usage overlay", () => {
const params = [
{ usagePercent: "0-25%", runningTaskSlotsCount: 0, fill: "fill: rgb(177, 213, 212);" },
{ usagePercent: "0-25%", runningTaskSlotsCount: 25, fill: "fill: rgb(177, 213, 212);" },
{ usagePercent: "26-50%", runningTaskSlotsCount: 26, fill: "fill: rgb(140, 195, 176);" },
{ usagePercent: "26-50%", runningTaskSlotsCount: 50, fill: "fill: rgb(140, 195, 176);" },
{ usagePercent: "51-75%", runningTaskSlotsCount: 51, fill: "fill: rgb(78, 177, 124);" },
{ usagePercent: "51-75%", runningTaskSlotsCount: 75, fill: "fill: rgb(78, 177, 124);" },
{ usagePercent: "76-99%", runningTaskSlotsCount: 76, fill: "fill: rgb(34, 160, 66);" },
{ usagePercent: "76-99%", runningTaskSlotsCount: 99, fill: "fill: rgb(34, 160, 66);" },
{ usagePercent: "100%", runningTaskSlotsCount: 100, fill: "fill: rgb(23, 141, 23);" },
];
params.forEach((param) => {
it(`should be ${ param.usagePercent } task slot usage color`, () => {
testComponent.nodes = createNodes(1, true, 100, param.runningTaskSlotsCount);
testComponent.pool = new Pool({ id: "pool-1", taskSlotsPerNode: 100 });
fixture.detectChanges();
const tiles = svg.selectAll("g.node-group");
expect(tiles.size()).toBe(1);
tiles.each((d, i, groups) => {
const group = d3.select(groups[i]);
const bg = group.select("g.taskslots");
const taskRects = bg.selectAll("rect");
expect(taskRects.size()).toBe(1, "Should have 1 rect");

expect(taskRects).not.toBeFalsy("Should have a rect in taskslots group");
expect(taskRects.attr("style")).toContain(param.fill);
});
});
});
Expand Down Expand Up @@ -343,15 +335,19 @@ describe("NodesHeatmapComponent", () => {
});
});

function createNodes(count: number, dedicated = true) {
function createNodes(
count: number,
dedicated = true,
runningTasksCount = defaultRunningTasksCount,
runningTaskSlotsCount = defaultRunningTaskSlotsCount) {
const nodes: Node[] = [];
for (let i = 0; i < count; i++) {
nodes.push(Fixture.node.create({
id: `node-${i + 1}`,
state: NodeState.running,
isDedicated: dedicated,
runningTasksCount: defaultRunningTasksCount,
runningTaskSlotsCount: defaultRunningTaskSlotsCount,
runningTasksCount: runningTasksCount,
runningTaskSlotsCount: runningTaskSlotsCount,
}));
}
return List(nodes);
Expand Down
Loading

0 comments on commit f7b5fcf

Please sign in to comment.