----- Original Message ----- > From: "Daniel Erez" <de...@redhat.com> > To: "Einav Cohen" <eco...@redhat.com> > Cc: "Gilad Chaplik" <gchap...@redhat.com>, "Tomas Jelinek" > <tjeli...@redhat.com>, "Vojtech Szocs" > <vsz...@redhat.com>, "Lior Vernia" <lver...@redhat.com>, "engine-devel" > <engine-devel@ovirt.org> > Sent: Friday, October 18, 2013 12:06:02 PM > Subject: Re: ui code: possible problem in 'remove' confirmation dialog > > > > ----- Original Message ----- > > From: "Einav Cohen" <eco...@redhat.com> > > To: "Daniel Erez" <de...@redhat.com>, "Gilad Chaplik" > > <gchap...@redhat.com>, "Tomas Jelinek" <tjeli...@redhat.com>, > > "Vojtech Szocs" <vsz...@redhat.com>, "Lior Vernia" <lver...@redhat.com> > > Cc: "engine-devel" <engine-devel@ovirt.org> > > Sent: Thursday, October 17, 2013 4:24:56 PM > > Subject: Re: ui code: possible problem in 'remove' confirmation dialog > > > > [apologies - previous e-mail sent prematurely by mistake] > > > > Hi, > > > > Looking at the current code: It seems that we cannot set the message within > > a > > 'remove' confirmation dialog if its HashName starts with "remove_" - it is > > being set "statically" to the "Are you sure you want to remove the > > following > > items?" [constants.removeConfirmationPopupMessage] message [1] > > > > I don't have a major problem with relying on the HashName for setting a > > *default* message [2] - but I think that not allowing to override this > > message if the developer chooses to do so is an incorrect behavior. > > > > I would like to change the behavior so that the user would be able to > > override the message displayed in that dialog, even if its hash-name is > > set to 'remove_'. > > > > thoughts? objections? > > > > ---- > > Thanks, > > Einav > > > > > > [1] From RemoveConfirmationPopupView.java, line 86: > > > > public void setMessage(String message) { > > if (getHashName() != null && getHashName().startsWith("remove_")) > > { > > //$NON-NLS-1$ > > super.setMessage(constants.removeConfirmationPopupMessage()); > > } else { > > super.setMessage(message); > > } > > } > > Not sure why we prevent override in this case,
IIRC the current RemoveConfirmationPopupView.setMessage implementation reflects the assumption that all remove confirm dialogs [hashName=remove_*] have the same message, i.e. "Are you sure you want to remove the following items?". However, looking at VmListModel#remove (VM main tab / Remove button) it first sets [hashName=remove_virtual_machine] and then sets message like "Virtual Machine(s)". In practice, RemoveConfirmationPopupView.setMessage will still make the dialog message generic, i.e. "Are you sure ... following items?". > I guess it should probably be as simple as: > > public void setMessage(String message) { > super.setMessage(message != null ? message : > constants.removeConfirmationPopupMessage()); > } For that, we'd have to analyze and modify existing UiCommon models to call setMessage with appropriate value, i.e. VmListModel#remove currently yields RemoveConfirmationPopupView.setMessage("Virtual Machine(s)") > > Just need to make sure that no one is currently overriding > it by mistake to avoid possible regressions. I think the general pattern used in UiCommon models when dealing with remove confirm dialogs is following: - set wnd hashName to "remove_<entity>" i.e. "remove_virtual_machine" - set wnd message to "<specific item(s)>" i.e. "Virtual Machine(s)" Alternative to Daniel's suggestion, following above mentioned pattern: if (message != null) { // expecting message to be "<specific item(s)>" // should yield "Are you sure you want to remove <specific item(s)>?" super.setMessage(messages.removeConfirmationPopupMessage(message)); } else { super.setMessage(message); } > > > > > [2] in fact, I don't mind that this would be the default message for > > this dialog, even if its hash-name is not set to something that starts > > with 'remove_'. > > > > ----- Original Message ----- > > > From: "Einav Cohen" <eco...@redhat.com> > > > To: "Daniel Erez" <de...@redhat.com>, "Gilad Chaplik" > > > <gchap...@redhat.com>, "Tomas Jelinek" <tjeli...@redhat.com>, > > > "Vojtech Szocs" <vsz...@redhat.com>, "Lior Vernia" <lver...@redhat.com> > > > Cc: "engine-devel" <engine-devel@ovirt.org> > > > Sent: Thursday, October 17, 2013 9:20:07 AM > > > Subject: ui code: possible problem in 'remove' confirmation dialog > > > > > > Hi, > > > > > > Looking at the current code: It seems that we cannot set the message > > > within > > > a > > > 'remove' confirmation > > > dialog if its HashName starts with "remove_" - it is being set > > > "statically[1] > > > I don't have a major problem with relying on the HashName for setting a > > > *default* message (in fact, > > > I don't have a problem > > > > > > [1] From RemoveConfirmationPopupView.java, line 86: > > > > > > public void setMessage(String message) { > > > if (getHashName() != null && getHashName().startsWith("remove_")) > > > { > > > //$NON-NLS-1$ > > > super.setMessage(constants.removeConfirmationPopupMessage()); > > > } else { > > > super.setMessage(message); > > > } > > > } > > > > > > ---- > > > Regards, > > > Einav Cohen Baum > > > RHEV-M Engineering - UX Team Manager > > > Red Hat, Inc. > > > 314 Littleton Road > > > Westford, MA 01886 > > > T [internal]: (81) 31046 > > > T [external]: (+1) 978 589 1046 > > > IRC: ecohen @ > > > - RHAT [internal]: #rhev-dev #boston #westford #tlv > > > - OFTC [external]: #ovirt > > > > > > _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel