12:55 p.m.
----- Original Message -----
From: "Daniel Erez" <derez(a)redhat.com>
To: "Einav Cohen" <ecohen(a)redhat.com>
Cc: "Gilad Chaplik" <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>, "Vojtech Szocs"
<vszocs(a)redhat.com>, "Lior Vernia" <lvernia(a)redhat.com>,
"engine-devel" <engine-devel(a)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" <ecohen(a)redhat.com>
> To: "Daniel Erez" <derez(a)redhat.com>, "Gilad Chaplik"
> <gchaplik(a)redhat.com>, "Tomas Jelinek" <tjelinek(a)redhat.com>,
> "Vojtech Szocs" <vszocs(a)redhat.com>, "Lior Vernia"
<lvernia(a)redhat.com>
> Cc: "engine-devel" <engine-devel(a)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" <ecohen(a)redhat.com>
> > To: "Daniel Erez" <derez(a)redhat.com>, "Gilad
Chaplik"
> > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > "Vojtech Szocs" <vszocs(a)redhat.com>, "Lior Vernia"
<lvernia(a)redhat.com>
> > Cc: "engine-devel" <engine-devel(a)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
> >
>