Vojtech Szocs has posted comments on this change.
Change subject: webadmin: add dash - before items in confirmation pop up
......................................................................
Patch Set 2:
First, looking at ConfirmationModel usage throughout UiCommon, the items
(strings) always seem to be added without any "-" prefix, which is a good thing.
DefaultConfirmationPopupView is the default dialog view bound to
ConfirmationModel. It simply renders ConfirmationModel's items as they are:
for (String item : items) {
descriptionPanel.add(new Label(item));
}
On the other hand, RemoveConfirmationPopupView, which is the common dialog view
used for "remove" commands/operations, renders "-" prefix for each item:
String getItemTextFormatted(String itemText) {
return "- " + itemText; //$NON-NLS-1$
}
IIUC, Ramesh just modified DefaultConfirmationPopupView to render items the way
RemoveConfirmationPopupView renders them. So this patch seems safe and good to
me.
Please note that we also have lots of other (more specific) dialog views bound
to ConfirmationModel, i.e. views for following presenter widgets:
* DataCenterForceRemovePopupPresenterWidget
* HostManagementConfirmationPopupPresenterWidget
* ImportCloneDialogPresenterWidget
* ManualFencePopupPresenterWidget
* RecoveryStoragePopupPresenterWidget
* RemoveBrickPopupPresenterWidget
* RemoveConfirmationPopupPresenterWidget
* RolePermissionsRemoveConfirmationPopupPresenterWidget
* StorageDestroyPopupPresenterWidget
* StorageForceCreatePopupPresenterWidget
* SystemPermissionsRemoveConfirmationPopupPresenterWidget
* VmDiskRemovePopupPresenterWidget
* VmRemovePopupPresenterWidget
For now, above mentioned presenter widgets (and their views) are responsible
for proper rendering of ConfirmationModel's items.
If we want to consolidate (standardize) item rendering across *all* dialog
views bound to ConfirmationModel, we'd have to introduce some shared renderer
class and use it in all view classes; however this would mean lots of code
changes.
For now, I'm OK with current change, i.e. adapt DefaultConfirmationPopupView to
render items just like RemoveConfirmationPopupView renders them.
--
To view, visit http://gerrit.ovirt.org/20730
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib09a5ca23e81e2c11e64e08084691ebc03e2f602
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ramesh N <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
