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 Adds multiple selection in osd table #29662

Merged

Conversation

@pooja-gautam
Copy link
Contributor

commented Aug 14, 2019

Signed-off-by: Pooja Gautam pooja.gautam@ts.fujitsu.com
Fixes: https://tracker.ceph.com/issues/38091

multiselect

@pooja-gautam pooja-gautam requested a review from ceph/dashboard as a code owner Aug 14, 2019
@@ -177,6 +177,7 @@ export class OsdListComponent implements OnInit {
}
];
this.columns = [
{ prop: 'selected', name: '', checkboxable: true, width: 30, canAutoResize: false },

This comment has been minimized.

Copy link
@votdev

votdev Aug 14, 2019

Contributor

Not necessary if you use selectionType='multi'.

</div>
<ng-container i18n><strong>OSD {{ selection.first().id }}</strong> will be
<ng-container i18n><strong>OSD {{ selection.getIds().toString() }}</strong> will be

This comment has been minimized.

Copy link
@votdev

votdev Aug 14, 2019

Contributor

Use lodash to get the ID's of the selected OSD's and then the list pipe to format it nicely. Don't forget to make _ public, otherwise you can not access lodash in the template file.

Suggested change
<ng-container i18n><strong>OSD {{ selection.getIds().toString() }}</strong> will be
<ng-container i18n><strong>OSD {{ _.map(selection.selected, (s) => s.id) | list }}</strong> will be

This comment has been minimized.

Copy link
@votdev

votdev Aug 14, 2019

Contributor

Alternatively you could implement a new pipe that works like the Jinja2 attr filter.

<ng-container i18n><strong>OSD {{ selection.selected | attr:'id' | list }}</strong> will be

@LenzGr LenzGr added the needs-review label Aug 15, 2019
@ricardoasmarques

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Please add Fixes: https://tracker.ceph.com/issues/38091 to the commit message.

@tspmelo tspmelo requested a review from p-se Aug 29, 2019
@pooja-gautam pooja-gautam force-pushed the pooja-gautam:wip-dashboard-multiselect-osd-table branch 2 times, most recently from 95adbb7 to 953f17a Aug 30, 2019
@pooja-gautam

This comment has been minimized.

Copy link
Contributor Author

commented Aug 30, 2019

Updated look:

1

2

@epuertat

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2019

jenkins test dashboard

@pooja-gautam pooja-gautam force-pushed the pooja-gautam:wip-dashboard-multiselect-osd-table branch from 953f17a to 58a1dba Sep 2, 2019
@pooja-gautam pooja-gautam force-pushed the pooja-gautam:wip-dashboard-multiselect-osd-table branch 2 times, most recently from 00dc6d8 to 5e260e4 Sep 2, 2019
@pooja-gautam

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

jenkins test dashboard

@pooja-gautam pooja-gautam force-pushed the pooja-gautam:wip-dashboard-multiselect-osd-table branch 2 times, most recently from 4bdeadf to 1b7392c Sep 3, 2019
@pooja-gautam

This comment has been minimized.

Copy link
Contributor Author

commented Sep 4, 2019

@votdev I've addressed the suggested changes.

@pooja-gautam pooja-gautam force-pushed the pooja-gautam:wip-dashboard-multiselect-osd-table branch from 1b7392c to fb179b2 Sep 18, 2019
Fixes: https://tracker.ceph.com/issues/38091

Signed-off-by: Pooja <pooja.gautam@ts.fujitsu.com>
@pooja-gautam

This comment has been minimized.

Copy link
Contributor Author

commented Sep 18, 2019

Latest look:

image

@pooja-gautam

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

@tspmelo @votdev @Devp00l Please take a look at the updated version.

@votdev
votdev approved these changes Sep 19, 2019
@p-se
p-se approved these changes Sep 20, 2019
@LenzGr LenzGr merged commit b2ca51a into ceph:master Sep 20, 2019
6 checks passed
6 checks passed
Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
ceph dashboard tests ceph dashboard tests succeeded
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
@LenzGr

This comment has been minimized.

Copy link
Contributor

commented Sep 20, 2019

Thanks a lot for your contribution, @pooja-gautam !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.