Skip to content

Commit b9bc49f

Browse files
nizamial09nizamial09
committed
mgr/dashboard: fix alert broken for multiple alerts
After carbon the alert panel was broken when there are multiple alerts present (telemetry, motd, password expiration). Applying carbon banner to the existing alert banner Fixes: https://tracker.ceph.com/issues/66512 Signed-off-by: Nizamudeen A <[email protected]>
1 parent 414085c commit b9bc49f

File tree

13 files changed

+193
-152
lines changed

13 files changed

+193
-152
lines changed

‎src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.html‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,16 @@
22
<cd-navigation>
33
<div class="container-fluid h-100"
44
[ngClass]="{'dashboard': (router.url == '/dashboard' || router.url == '/dashboard_3' || router.url == '/multi-cluster/overview'), 'rgw-dashboard': (router.url == '/rgw/overview')}">
5-
<cd-context></cd-context>
5+
<!-- ************************ -->
6+
<!-- ALERTS BANNER -->
7+
<!-- ************************ -->
8+
<div class="cd-alert-container"
9+
[ngClass]="{'ms-4 me-4': (router.url == '/dashboard' || router.url == '/dashboard_3' || router.url == '/multi-cluster/overview'), 'm-3': (router.url == '/rgw/overview')}">
10+
<cd-pwd-expiration-notification></cd-pwd-expiration-notification>
11+
<cd-telemetry-notification></cd-telemetry-notification>
12+
<cd-motd></cd-motd>
13+
</div>
14+
<cd-context></cd-context>
615
<cd-breadcrumbs></cd-breadcrumbs>
716
<router-outlet></router-outlet>
817
</div>

‎src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.scss‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,8 @@
1616
.rgw-dashboard {
1717
background-color: vv.$body-bg-alt;
1818
}
19+
20+
.cd-alert-container {
21+
display: flex;
22+
flex-direction: column;
23+
}

‎src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.spec.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,56 @@ describe('WorkbenchLayoutComponent', () => {
3232
it('should create', () => {
3333
expect(component).toBeTruthy();
3434
});
35+
36+
describe('showTopNotification', () => {
37+
const notification1 = 'notificationName1';
38+
const notification2 = 'notificationName2';
39+
40+
beforeEach(() => {
41+
component.notifications = [];
42+
});
43+
44+
it('should show notification', () => {
45+
component.showTopNotification(notification1, true);
46+
expect(component.notifications.includes(notification1)).toBeTruthy();
47+
expect(component.notifications.length).toBe(1);
48+
});
49+
50+
it('should not add a second notification if it is already shown', () => {
51+
component.showTopNotification(notification1, true);
52+
component.showTopNotification(notification1, true);
53+
expect(component.notifications.includes(notification1)).toBeTruthy();
54+
expect(component.notifications.length).toBe(1);
55+
});
56+
57+
it('should add a second notification if the first one is different', () => {
58+
component.showTopNotification(notification1, true);
59+
component.showTopNotification(notification2, true);
60+
expect(component.notifications.includes(notification1)).toBeTruthy();
61+
expect(component.notifications.includes(notification2)).toBeTruthy();
62+
expect(component.notifications.length).toBe(2);
63+
});
64+
65+
it('should hide an active notification', () => {
66+
component.showTopNotification(notification1, true);
67+
expect(component.notifications.includes(notification1)).toBeTruthy();
68+
expect(component.notifications.length).toBe(1);
69+
component.showTopNotification(notification1, false);
70+
expect(component.notifications.length).toBe(0);
71+
});
72+
73+
it('should not fail if it tries to hide an inactive notification', () => {
74+
expect(() => component.showTopNotification(notification1, false)).not.toThrow();
75+
expect(component.notifications.length).toBe(0);
76+
});
77+
78+
it('should keep other notifications if it hides one', () => {
79+
component.showTopNotification(notification1, true);
80+
component.showTopNotification(notification2, true);
81+
expect(component.notifications.length).toBe(2);
82+
component.showTopNotification(notification2, false);
83+
expect(component.notifications.length).toBe(1);
84+
expect(component.notifications.includes(notification1)).toBeTruthy();
85+
});
86+
});
3587
});

‎src/pybind/mgr/dashboard/frontend/src/app/core/layouts/workbench-layout/workbench-layout.component.ts‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, OnDestroy, OnInit } from '@angular/core';
1+
import { Component, HostBinding, OnDestroy, OnInit } from '@angular/core';
22
import { Router } from '@angular/router';
33

44
import { Subscription } from 'rxjs';
@@ -9,6 +9,9 @@ import { AuthStorageService } from '~/app/shared/services/auth-storage.service';
99
import { FaviconService } from '~/app/shared/services/favicon.service';
1010
import { SummaryService } from '~/app/shared/services/summary.service';
1111
import { TaskManagerService } from '~/app/shared/services/task-manager.service';
12+
import { TelemetryNotificationService } from '../../../shared/services/telemetry-notification.service';
13+
import { MotdNotificationService } from '~/app/shared/services/motd-notification.service';
14+
import _ from 'lodash';
1215

1316
@Component({
1417
selector: 'cd-workbench-layout',
@@ -17,16 +20,22 @@ import { TaskManagerService } from '~/app/shared/services/task-manager.service';
1720
providers: [FaviconService]
1821
})
1922
export class WorkbenchLayoutComponent implements OnInit, OnDestroy {
23+
notifications: string[] = [];
2024
private subs = new Subscription();
2125
permissions: Permissions;
26+
@HostBinding('class') get class(): string {
27+
return 'top-notification-' + this.notifications.length;
28+
}
2229

2330
constructor(
2431
public router: Router,
2532
private summaryService: SummaryService,
2633
private taskManagerService: TaskManagerService,
2734
private multiClusterService: MultiClusterService,
2835
private faviconService: FaviconService,
29-
private authStorageService: AuthStorageService
36+
private authStorageService: AuthStorageService,
37+
private telemetryNotificationService: TelemetryNotificationService,
38+
private motdNotificationService: MotdNotificationService
3039
) {
3140
this.permissions = this.authStorageService.getPermissions();
3241
}
@@ -38,8 +47,36 @@ export class WorkbenchLayoutComponent implements OnInit, OnDestroy {
3847
}
3948
this.subs.add(this.summaryService.startPolling());
4049
this.subs.add(this.taskManagerService.init(this.summaryService));
50+
51+
this.subs.add(
52+
this.authStorageService.isPwdDisplayed$.subscribe((isDisplayed) => {
53+
this.showTopNotification('isPwdDisplayed', isDisplayed);
54+
})
55+
);
56+
this.subs.add(
57+
this.telemetryNotificationService.update.subscribe((visible: boolean) => {
58+
this.showTopNotification('telemetryNotificationEnabled', visible);
59+
})
60+
);
61+
this.subs.add(
62+
this.motdNotificationService.motd$.subscribe((motd: any) => {
63+
this.showTopNotification('motdNotificationEnabled', _.isPlainObject(motd));
64+
})
65+
);
4166
this.faviconService.init();
4267
}
68+
showTopNotification(name: string, isDisplayed: boolean) {
69+
if (isDisplayed) {
70+
if (!this.notifications.includes(name)) {
71+
this.notifications.push(name);
72+
}
73+
} else {
74+
const index = this.notifications.indexOf(name);
75+
if (index >= 0) {
76+
this.notifications.splice(index, 1);
77+
}
78+
}
79+
}
4380

4481
ngOnDestroy() {
4582
this.subs.unsubscribe();

‎src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.html‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
<div class="cd-navbar-main">
22
<!-- ************************ -->
3-
<!-- NOTIFICATIONS/ALERTS -->
3+
<!-- NOTIFICATIONS -->
44
<!-- ************************ -->
5-
<cd-pwd-expiration-notification></cd-pwd-expiration-notification>
6-
<cd-telemetry-notification></cd-telemetry-notification>
7-
<cd-motd></cd-motd>
85
<cd-notifications-sidebar></cd-notifications-sidebar>
96
<!-- ************************ -->
107
<!-- HEADER -->

‎src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.spec.ts‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -214,56 +214,4 @@ describe('NavigationComponent', () => {
214214
});
215215
}
216216
});
217-
218-
describe('showTopNotification', () => {
219-
const notification1 = 'notificationName1';
220-
const notification2 = 'notificationName2';
221-
222-
beforeEach(() => {
223-
component.notifications = [];
224-
});
225-
226-
it('should show notification', () => {
227-
component.showTopNotification(notification1, true);
228-
expect(component.notifications.includes(notification1)).toBeTruthy();
229-
expect(component.notifications.length).toBe(1);
230-
});
231-
232-
it('should not add a second notification if it is already shown', () => {
233-
component.showTopNotification(notification1, true);
234-
component.showTopNotification(notification1, true);
235-
expect(component.notifications.includes(notification1)).toBeTruthy();
236-
expect(component.notifications.length).toBe(1);
237-
});
238-
239-
it('should add a second notification if the first one is different', () => {
240-
component.showTopNotification(notification1, true);
241-
component.showTopNotification(notification2, true);
242-
expect(component.notifications.includes(notification1)).toBeTruthy();
243-
expect(component.notifications.includes(notification2)).toBeTruthy();
244-
expect(component.notifications.length).toBe(2);
245-
});
246-
247-
it('should hide an active notification', () => {
248-
component.showTopNotification(notification1, true);
249-
expect(component.notifications.includes(notification1)).toBeTruthy();
250-
expect(component.notifications.length).toBe(1);
251-
component.showTopNotification(notification1, false);
252-
expect(component.notifications.length).toBe(0);
253-
});
254-
255-
it('should not fail if it tries to hide an inactive notification', () => {
256-
expect(() => component.showTopNotification(notification1, false)).not.toThrow();
257-
expect(component.notifications.length).toBe(0);
258-
});
259-
260-
it('should keep other notifications if it hides one', () => {
261-
component.showTopNotification(notification1, true);
262-
component.showTopNotification(notification2, true);
263-
expect(component.notifications.length).toBe(2);
264-
component.showTopNotification(notification2, false);
265-
expect(component.notifications.length).toBe(1);
266-
expect(component.notifications.includes(notification1)).toBeTruthy();
267-
});
268-
});
269217
});

‎src/pybind/mgr/dashboard/frontend/src/app/core/navigation/navigation/navigation.component.ts‎

Lines changed: 1 addition & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Component, HostBinding, OnDestroy, OnInit } from '@angular/core';
1+
import { Component, OnDestroy, OnInit } from '@angular/core';
22
import { Router } from '@angular/router';
33

44
import * as _ from 'lodash';
@@ -14,22 +14,16 @@ import {
1414
FeatureTogglesMap$,
1515
FeatureTogglesService
1616
} from '~/app/shared/services/feature-toggles.service';
17-
import { MotdNotificationService } from '~/app/shared/services/motd-notification.service';
1817
import { PrometheusAlertService } from '~/app/shared/services/prometheus-alert.service';
1918
import { SummaryService } from '~/app/shared/services/summary.service';
20-
import { TelemetryNotificationService } from '~/app/shared/services/telemetry-notification.service';
2119

2220
@Component({
2321
selector: 'cd-navigation',
2422
templateUrl: './navigation.component.html',
2523
styleUrls: ['./navigation.component.scss']
2624
})
2725
export class NavigationComponent implements OnInit, OnDestroy {
28-
notifications: string[] = [];
2926
clusterDetails: any[] = [];
30-
@HostBinding('class') get class(): string {
31-
return 'top-notification-' + this.notifications.length;
32-
}
3327

3428
permissions: Permissions;
3529
enabledFeature$: FeatureTogglesMap$;
@@ -54,9 +48,7 @@ export class NavigationComponent implements OnInit, OnDestroy {
5448
private router: Router,
5549
private summaryService: SummaryService,
5650
private featureToggles: FeatureTogglesService,
57-
private telemetryNotificationService: TelemetryNotificationService,
5851
public prometheusAlertService: PrometheusAlertService,
59-
private motdNotificationService: MotdNotificationService,
6052
private cookieService: CookiesService,
6153
private settingsService: SettingsService
6254
) {
@@ -91,26 +83,6 @@ export class NavigationComponent implements OnInit, OnDestroy {
9183
this.summaryData = summary;
9284
})
9385
);
94-
/*
95-
Note: If you're going to add more top notifications please do not forget to increase
96-
the number of generated css-classes in section topNotification settings in the scss
97-
file.
98-
*/
99-
this.subs.add(
100-
this.authStorageService.isPwdDisplayed$.subscribe((isDisplayed) => {
101-
this.showTopNotification('isPwdDisplayed', isDisplayed);
102-
})
103-
);
104-
this.subs.add(
105-
this.telemetryNotificationService.update.subscribe((visible: boolean) => {
106-
this.showTopNotification('telemetryNotificationEnabled', visible);
107-
})
108-
);
109-
this.subs.add(
110-
this.motdNotificationService.motd$.subscribe((motd: any) => {
111-
this.showTopNotification('motdNotificationEnabled', _.isPlainObject(motd));
112-
})
113-
);
11486
this.subs.add(
11587
this.multiClusterService.subscribeClusterTokenStatus((resp: object) => {
11688
this.clusterTokenStatus = resp;
@@ -165,19 +137,6 @@ export class NavigationComponent implements OnInit, OnDestroy {
165137
this.rightSidebarOpen = !this.rightSidebarOpen;
166138
}
167139

168-
showTopNotification(name: string, isDisplayed: boolean) {
169-
if (isDisplayed) {
170-
if (!this.notifications.includes(name)) {
171-
this.notifications.push(name);
172-
}
173-
} else {
174-
const index = this.notifications.indexOf(name);
175-
if (index >= 0) {
176-
this.notifications.splice(index, 1);
177-
}
178-
}
179-
}
180-
181140
onClusterSelection(value: object) {
182141
this.multiClusterService.setCluster(value).subscribe(
183142
(resp: any) => {

0 commit comments

Comments
 (0)