mgr/dashboard: add permanent notifications #29530
Conversation
s0nea
force-pushed
the
wip-dashboard-permanent-notification
branch
5 times, most recently
from
97b278c
to
52b5c07
Compare
|
jenkins test dashboard |
|
jenkins test make check arm64 |
s0nea
force-pushed
the
wip-dashboard-permanent-notification
branch
3 times, most recently
from
ae464d8
to
1d19733
Compare
|
jenkins test docs |
s0nea
force-pushed
the
wip-dashboard-permanent-notification
branch
from
1d19733
to
65e0fb6
Compare
|
jenkins test submodules |
s0nea
force-pushed
the
wip-dashboard-permanent-notification
branch
from
65e0fb6
to
71ca18f
Compare
Add a ToastComponent in order to encapsulate all Toast related settings. This provides three advantages: * We can define our own Toast template * We need only one config class (CdNotificationConfig) * We need to deal with less HTML related strings in the *.ts files Signed-off-by: Tatjana Dehler <tdehler@suse.com>
Fixes: https://tracker.ceph.com/issues/40328 Signed-off-by: Tatjana Dehler <tdehler@suse.com>
Signed-off-by: Tatjana Dehler <tdehler@suse.com>
UI blocking notifications should only appear if an error occurs which makes the frontend unusable. The UI will be released as soon as the error is fixed. For example the UI would be blocked if the api is unreachable. Signed-off-by: Sebastian Krah <skrah@suse.com> (cherry picked from commit 662e17b)
s0nea
force-pushed
the
wip-dashboard-permanent-notification
branch
from
71ca18f
to
09f70b2
Compare
|
jenkins test dashboard |
There was a problem hiding this comment.
I know I am late with the suggestion - I apologize for not having noticed that earlier. But I don't think we should be misusing toasties to display this information.
Since we are now showing a "modal", we could instead use the bootstrap modal component.
By doing so we could reuse some of our components and styles, thus removing the need to change the current toast implementation and the amount of code needed.
The idea would be to create a service and a component for the permanent notifications.
The service would store all the data that would be displayed and the component would have the elements for the modal.
This service would then be called from the api-interceptor.service.ts, to know when and what to display.
What do you think? Sorry again for requesting such a major change at this point.
| ); | ||
| }); | ||
| } | ||
|
|
||
| private removeNotification() { |
There was a problem hiding this comment.
It took about 15s for the notification to be removed after the 1st successful request.
Although this is not critical, it would be nice to have a lower waiting time.
| -moz-box-shadow: none; | ||
| box-shadow: none; | ||
|
|
||
| .toast-title { |
There was a problem hiding this comment.
This should have the same background color as the modals we use. Same for the footer.
There was a problem hiding this comment.
Ahh yeah this might would make sense. I just looked up the background color of the dialog content, because at that time I had no intention to also style the title of the dialog. After I did style the title I did not double check whether the title uses another background color or not :)
| public notificationService: NotificationService | ||
| ) {} | ||
|
|
||
| intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> { | ||
| return next.handle(request).pipe( | ||
| tap(() => { | ||
| this.removeNotification(); |
There was a problem hiding this comment.
Not related to this line, but I think we should show a notification of the failure.
Otherwise it looks like nothing happened.
|
As mentioned in #29530 (review) and discussed afterwards, we decided to decline the PR as it's using the wrong approach. |
Fixes: https://tracker.ceph.com/issues/40328
Signed-off-by: Sebastian Krah skrah@suse.com
Signed-off-by: Tatjana Dehler tdehler@suse.com
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test docsjenkins render docs