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: Update selected items on table refresh #21099

Merged
merged 1 commit into from Apr 12, 2018

Conversation

ricardoasmarques
Copy link
Contributor

After updating table data (either by pressing refresh button or by automatic refresh), selected rows should be updated to guarantee that user will see the updated details of the selected row.

Signed-off-by: Ricardo Marques rimarques@suse.com

Copy link
Contributor

@tspmelo tspmelo left a comment

Choose a reason for hiding this comment

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

I think you need to add trackBy to some tables or save the state some other way.
ATM if you select a row in the OSD page, the details keep being recreated.

Copy link

@Devp00l Devp00l left a comment

Choose a reason for hiding this comment

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

Which tables are affected by this problem?

The detail view should update itself if it's data has changed, through the usage of Observables through Angular.

@ricardoasmarques
Copy link
Contributor Author

@tspmelo I've added a new table input updateSelectionOnRefresh, so you can control if you want selection details update on table refresh, or not (false by default).

@ricardoasmarques
Copy link
Contributor Author

@Devp00l this will affect all pages where you have a Details tab, that displays information from the selected row (without doing a new server request).

In particual, it affects my WIP RBD details page:

screenshot from 2018-04-09 12-06-19

And also my WIP RBD snapshots page (because snapshots list is included in the row data, so I don't have to do a separate server request).

screenshot from 2018-04-09 12-12-16

Do you still think that Observables are applicable here?

@@ -67,6 +67,9 @@ export class TableComponent implements AfterContentChecked, OnInit, OnChanges, O
// e.g. 'single' or 'multi'.
@Input() selectionType: string = undefined;

// If `true` selected item details will be updated on table refresh
@Input() updateSelectionOnRefresh = false;
Copy link
Member

Choose a reason for hiding this comment

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

I suggest to set it to true by default and force the developer to explicitly set it to false if he does not like this behaviour for his table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@votdev I've changed the default value to true

@Devp00l
Copy link

Devp00l commented Apr 11, 2018

@ricardoasmarques I was assuming that angular would update the details accordingly as it uses observables in the background. Also the ngx-table should update the selection accordingly. Maybe it's a bug in the table?

Signed-off-by: Ricardo Marques <rimarques@suse.com>
Copy link

@Devp00l Devp00l left a comment

Choose a reason for hiding this comment

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

lgtm

@LenzGr LenzGr added this to the mimic milestone Apr 12, 2018
@LenzGr LenzGr merged commit 04ca8e4 into ceph:master Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants