Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mgr/dashboard: Unify Tasks and Notifications into a sidebar #29706

Merged
merged 1 commit into from Oct 7, 2019

Conversation

tspmelo
Copy link
Contributor

@tspmelo tspmelo commented Aug 16, 2019

Fixes: https://tracker.ceph.com/issues/37402

Signed-off-by: Tiago Melo tmelo@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test docs
  • jenkins render docs

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 16, 2019

localhost_4200_ (1)

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 16, 2019

I wrote some questions in the issue.

@tspmelo tspmelo requested a review from a team August 16, 2019 13:40
@votdev
Copy link
Member

votdev commented Aug 20, 2019

The commit message has a typo: s/Taks/Task/

@LenzGr LenzGr changed the title mgr/dashboard: Unify Taks and Notifications into a sidebar mgr/dashboard: Unify Tasks and Notifications into a sidebar Aug 20, 2019
@LenzGr
Copy link
Contributor

LenzGr commented Aug 20, 2019

Nit: there's a small typo in the commit message: s/Taks/Tasks/

@tspmelo tspmelo force-pushed the wip-notification-sidebar branch 3 times, most recently from 20bf1d1 to 4b739a9 Compare August 22, 2019 16:57
@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 26, 2019

jenkins retest this please

@tspmelo tspmelo marked this pull request as ready for review August 26, 2019 12:23
@tspmelo tspmelo removed the DNM label Aug 26, 2019
@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 27, 2019

jenkins retest this please

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 27, 2019

jenkins test make check

@callithea
Copy link
Member

Tested it locally, I only see processing long running tasks, the notifications e.g. about an updated pool are not displayed in the sidebar.. not sure if someone can reproduce this?

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 28, 2019

@callithea Do you mean the notification that a pool is updating or that it finished updating?

@callithea
Copy link
Member

@callithea Do you mean the notification that a pool is updating or that it finished updating?

@tspmelo I can only see that it's updating a pool when the update is still ongoing. But when the update is done I can't see any notifications that I upated the pool. Also I can't see something like you posted in the screenshot about "Created pool xy" when I created a pool.

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 28, 2019

Just tested the PR and got this:
notification

Are you getting any errors in the browser console?

@callithea
Copy link
Member

callithea commented Aug 28, 2019

Small update: I can see notifications for e.g. OSDs, but still don't see any when creating/editing pools/RBDs, need to investigate..
I can also see the pop up messages for all actions when it comes to creating/editing/deleting items. But they do not appear in the sidebar notifcation list (except for OSD notifications).

Screenshot_20190828_223937

@tspmelo
Copy link
Contributor Author

tspmelo commented Aug 29, 2019

jenkins test dashboard

@p-se
Copy link
Contributor

p-se commented Aug 29, 2019

Small update: I can see notifications for e.g. OSDs, but still don't see any when creating/editing pools/RBDs, need to investigate..

I can reproduce that issue. I see popup notifications when creating and deleting an OSD, but don't see any information about that in the sidebar.

Is that on purpose?

I can also see the pop up messages for all actions when it comes to creating/editing/deleting items. But they do not appear in the sidebar notifcation list (except for OSD notifications).

[screenshot removed]

@ricardoasmarques
Copy link
Contributor

+1 for Duration: xxx seconds.

@LenzGr
Copy link
Contributor

LenzGr commented Sep 10, 2019

the duration is not how long ago it finished, it's how long it took to finish the task.
So the timestamp and duration are showing 2 different information.

That was exactly my point ;) - but you're also putting a duration after the time stamp, that should either be improved by adding the string "ago" behind it, to make clear that these are two different time intervals.

The point I would like to clarify: do we need to print both the time stamp and the time elapsed since that time stamp, or is providing one of them enough?

We could either do a):

[Notification text]
Duration: [duration]
[Time stamp] ([time elapsed] ago)

or b):

[Notification text]
Duration: [duration]
[Time stamp]

or c):

[Notification text]
Duration: [duration]
([time elapsed] ago)

@tspmelo
Copy link
Contributor Author

tspmelo commented Sep 11, 2019

Current implementation:
Screenshot_20190911_1052331

the title overlay is darker, but for some reason, when I try to take a screenshot it fades...

@tspmelo
Copy link
Contributor Author

tspmelo commented Sep 12, 2019

It seems the e2e tests are losing the authentication, which doesn't happen when testing manually.
Will have to investigate.

@callithea
Copy link
Member

jenkins test dashboard

@callithea
Copy link
Member

jenkins test docs

@callithea
Copy link
Member

callithea commented Sep 18, 2019

Is it on purpose that the closing button ("X") in the top right corner is surrounded by a light blue box?

Screenshot_20190911_1052331

In Firefox it's surrounded by grey dots:

Screenshot_20190918_113502

This happens when opening the sidebar.

@tspmelo
Copy link
Contributor Author

tspmelo commented Sep 18, 2019

@callithea Not on purpose.
Browsers seem to auto focus the button each time the sidebar is opened.
I will try to disable it.

@callithea
Copy link
Member

@callithea Not on purpose.
Browsers seem to auto focus the button each time the sidebar is opened.
I will try to disable it.

Re-tested it locally, it's disabled now.

<p class="card-text text-muted">
<ng-container *ngIf="notification.duration">
<small>
<ng-container i18n>Duration:</ng-container> {{notification.duration | duration}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<ng-container i18n>Duration:</ng-container> {{notification.duration | duration}}
<ng-container i18n>Duration:</ng-container> {{ notification.duration | duration }}

@votdev
Copy link
Member

votdev commented Sep 24, 2019

A new notification shows a few seconds ago though the timestamp (using the browser dev tools) shows that the notification appears 5 minutes ago. The text changes after a page reload to 5 minutes ago. I used Chromium for testing. To me it looks like the notification sidebar is not re-rendered to display the correct time that has passed.

@tspmelo
Copy link
Contributor Author

tspmelo commented Sep 24, 2019

@votdev Fixed.

@tspmelo
Copy link
Contributor Author

tspmelo commented Sep 25, 2019

jenkins retest this please

@LenzGr LenzGr merged commit ed16537 into ceph:master Oct 7, 2019
@tspmelo tspmelo deleted the wip-notification-sidebar branch October 7, 2019 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants