8:20 a.m.
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
8:24 a.m.
[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);
}
}
[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
9:37 a.m.
New subject: [Engine-devel] 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 9:24:56 AM
Subject: Re: [Engine-devel] 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?
I agree with you. It should be overridable, with "Are you sure" as the default.
Greg
----
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);
}
}
[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
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
5:06 a.m.
----- 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,
I guess it should probably be as simple as:
public void setMessage(String message) {
super.setMessage(message != null ? message :
constants.removeConfirmationPopupMessage());
}
Just need to make sure that no one is currently overriding
it by mistake to avoid possible regressions.
[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
>
8:44 a.m.
New subject: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog
----- Original Message -----
From: "Daniel Erez" <derez(a)redhat.com>
Sent: Friday, October 18, 2013 6:06:02 AM
----- 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,
I guess it should probably be as simple as:
public void setMessage(String message) {
super.setMessage(message != null ? message :
constants.removeConfirmationPopupMessage());
}
the above would mean that in order to utilize the default message,
we would need to explicitly call to 'setMessage()' with 'null' as
a parameter, doesn't it?
maybe we should do the following instead:
public void setMessage(String message) {
super.setMessage(message);
}
public String getMessage() {
return super.getMessage() != null ? super.getMessage() :
constants.removeConfirmationPopupMessage();
}
[of course, the above means that if someone would explicitly sets
the message to 'null', the dialog would end up with the default
message; but I think it is acceptable. so:
- if you want the default message: don't set message / set it to 'null'
- if you want an empty message: set message to an empty string
- if you want a message other than default: set it to whatever you want]
?
Just need to make sure that no one is currently overriding
it by mistake to avoid possible regressions.
>
> [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
> >
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
12:44 a.m.
New subject: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog
----- Original Message -----
From: "Einav Cohen" <ecohen(a)redhat.com>
To: "Daniel Erez" <derez(a)redhat.com>, "Greg Sheremeta"
<gshereme(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Friday, October 18, 2013 4:44:25 PM
Subject: Re: [Engine-devel] ui code: possible problem in 'remove' confirmation
dialog
> ----- Original Message -----
> From: "Daniel Erez" <derez(a)redhat.com>
> Sent: Friday, October 18, 2013 6:06:02 AM
>
>
>
> ----- 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,
> I guess it should probably be as simple as:
>
> public void setMessage(String message) {
> super.setMessage(message != null ? message :
> constants.removeConfirmationPopupMessage());
> }
the above would mean that in order to utilize the default message,
we would need to explicitly call to 'setMessage()' with 'null' as
a parameter, doesn't it?
IINM, setMessage is called explicitly with model's getMessage
upon popup revealing. So we don't need to call setMessage with
null again.
maybe we should do the following instead:
public void setMessage(String message) {
super.setMessage(message);
}
public String getMessage() {
return super.getMessage() != null ? super.getMessage() :
constants.removeConfirmationPopupMessage();
}
[of course, the above means that if someone would explicitly sets
the message to 'null', the dialog would end up with the default
message; but I think it is acceptable. so:
- if you want the default message: don't set message / set it to 'null'
Just don't set a message, no need to set it to null.
- if you want an empty message: set message to an empty string
- if you want a message other than default: set it to whatever you want]
?
+1
>
> Just need to make sure that no one is currently overriding
> it by mistake to avoid possible regressions.
>
> >
> > [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
> > >
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
>
>
4:55 a.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
> >
>
7:12 a.m.
----- Original Message -----
From: "Vojtech Szocs" <vszocs(a)redhat.com>
To: "Daniel Erez" <derez(a)redhat.com>
Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad Chaplik"
<gchaplik(a)redhat.com>, "Tomas Jelinek" <tjelinek(a)redhat.com>,
"Lior Vernia" <lvernia(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Monday, October 21, 2013 12:55:05 PM
Subject: Re: ui code: possible problem in 'remove' confirmation dialog
----- 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)")
Isn't it a cleanup that should be done either way?
(I assume this message isn't currently used on any context...).
But indeed, it will require making sure that no one is
currently overriding the setMessage() by mistake to
avoid such possible regressions.
>
> 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);
}
What about default value (when message is null)?
Do you mean we should add another abstract class in
the hierarchy that would default it to
"Are you sure you want to remove the following items?".
[btw, we might have some localization issues with this
pattern as we've encountered with 'no items to display';
so just need to check it out].
>
> >
> > [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
> > >
> >
>
7:57 a.m.
----- Original Message -----
From: "Daniel Erez" <derez(a)redhat.com>
To: "Vojtech Szocs" <vszocs(a)redhat.com>
Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad Chaplik"
<gchaplik(a)redhat.com>, "Tomas Jelinek" <tjelinek(a)redhat.com>,
"Lior Vernia" <lvernia(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Monday, October 21, 2013 2:12:22 PM
Subject: Re: ui code: possible problem in 'remove' confirmation dialog
----- Original Message -----
> From: "Vojtech Szocs" <vszocs(a)redhat.com>
> To: "Daniel Erez" <derez(a)redhat.com>
> Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad Chaplik"
> <gchaplik(a)redhat.com>, "Tomas Jelinek" <tjelinek(a)redhat.com>,
> "Lior Vernia" <lvernia(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
> Sent: Monday, October 21, 2013 12:55:05 PM
> Subject: Re: ui code: possible problem in 'remove' confirmation dialog
>
>
>
> ----- 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)")
Isn't it a cleanup that should be done either way?
(I assume this message isn't currently used on any context...).
Yes, I guess some sort of cleanup will be required anyway.
For me, the question is if we want to give models the ability to specify full remove
confirm msg (i.e. "Are you sure...") or just the item part of it (i.e.
"Virtual Machine(s)").
From localization perspective, full remove confirm msg seems more
flexible but will add more msg keys, however as you wrote we'd need some more changes
in UiCommon models anyway.
But indeed, it will require making sure that no one is
currently overriding the setMessage() by mistake to
avoid such possible regressions.
>
> >
> > 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);
> }
What about default value (when message is null)?
The setMessage code I've mentioned above was based on assumption that UiCommon models
such as VmListModel will always follow convention like
removeConfirmDialogModel.setMessage("item(s)") .. but maybe such assumption is
too strong.
In general, you're right, there should be some default value handling included (I
forgot about this, sorry).
Do you mean we should add another abstract class in
the hierarchy that would default it to
"Are you sure you want to remove the following items?".
[btw, we might have some localization issues with this
pattern as we've encountered with 'no items to display';
so just need to check it out].
OK, in that case we can follow Daniel's suggestion (setMessage code that just
delegates to super.setMessage unless message == null for which there'd be some default
msg), along with updating relevant UiCommon models (i.e. strings used when calling
setMessage).
>
> >
> > >
> > > [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
> > > >
> > >
> >
>
8:28 a.m.
New subject: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog
For me, the question is if we want to give models the ability to
specify full
remove confirm msg (i.e. "Are you sure...") or just the item part of it
(i.e. "Virtual Machine(s)").
>From localization perspective, full remove confirm msg seems more flexible
>but will add more msg keys, however as you wrote we'd need some more
>changes in UiCommon models anyway.
we *must* be able to specify full remove confirm msg only - we should not use
message "particles", as they cause translation-hell (sigular vs. plural, etc.) -
we had multiple problems with that in the past - that's why we changed the message
here from "are you sure ... <object-type>?" to the more generic "are
you sure ...
items?" [http://gerrit.ovirt.org/#/c/14621]
thanks.
> ----- Original Message -----
> From: "Vojtech Szocs" <vszocs(a)redhat.com>
> Sent: Monday, October 21, 2013 8:57:16 AM
>
>
> ----- Original Message -----
> > From: "Daniel Erez" <derez(a)redhat.com>
> > To: "Vojtech Szocs" <vszocs(a)redhat.com>
> > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > "Lior Vernia" <lvernia(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
> > Sent: Monday, October 21, 2013 2:12:22 PM
> > Subject: Re: ui code: possible problem in 'remove' confirmation dialog
> >
> >
> >
> > ----- Original Message -----
> > > From: "Vojtech Szocs" <vszocs(a)redhat.com>
> > > To: "Daniel Erez" <derez(a)redhat.com>
> > > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > > "Lior Vernia" <lvernia(a)redhat.com>,
"engine-devel"
> > > <engine-devel(a)ovirt.org>
> > > Sent: Monday, October 21, 2013 12:55:05 PM
> > > Subject: Re: ui code: possible problem in 'remove' confirmation
dialog
> > >
> > >
> > >
> > > ----- 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)")
> >
> > Isn't it a cleanup that should be done either way?
> > (I assume this message isn't currently used on any context...).
>
> Yes, I guess some sort of cleanup will be required anyway.
>
For me, the question is if we want to give models the ability to
specify full
remove confirm msg (i.e. "Are you sure...") or just the item part of it
(i.e. "Virtual Machine(s)").
>From localization perspective, full remove confirm msg seems more flexible
>but will add more msg keys, however as you wrote we'd need some more
>changes in UiCommon models anyway.
>
> > But indeed, it will require making sure that no one is
> > currently overriding the setMessage() by mistake to
> > avoid such possible regressions.
> >
> > >
> > > >
> > > > 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);
> > > }
> >
> > What about default value (when message is null)?
>
> The setMessage code I've mentioned above was based on assumption that
> UiCommon models such as VmListModel will always follow convention like
> removeConfirmDialogModel.setMessage("item(s)") .. but maybe such
assumption
> is too strong.
>
> In general, you're right, there should be some default value handling
> included (I forgot about this, sorry).
>
> > Do you mean we should add another abstract class in
> > the hierarchy that would default it to
> > "Are you sure you want to remove the following items?".
> > [btw, we might have some localization issues with this
> > pattern as we've encountered with 'no items to display';
> > so just need to check it out].
>
> OK, in that case we can follow Daniel's suggestion (setMessage code that just
> delegates to super.setMessage unless message == null for which there'd be
> some default msg), along with updating relevant UiCommon models (i.e.
> strings used when calling setMessage).
>
> >
> > >
> > > >
> > > > >
> > > > > [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
> > > > > >
> > > > >
> > > >
> > >
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
>
>
8:39 a.m.
New subject: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog
----- Original Message -----
From: "Einav Cohen" <ecohen(a)redhat.com>
To: "Vojtech Szocs" <vszocs(a)redhat.com>
Cc: "Daniel Erez" <derez(a)redhat.com>, "engine-devel"
<engine-devel(a)ovirt.org>
Sent: Monday, October 21, 2013 3:28:56 PM
Subject: Re: [Engine-devel] ui code: possible problem in 'remove' confirmation
dialog
> For me, the question is if we want to give models the ability to specify
> full
> remove confirm msg (i.e. "Are you sure...") or just the item part of it
> (i.e. "Virtual Machine(s)").
>
> >From localization perspective, full remove confirm msg seems more flexible
> >but will add more msg keys, however as you wrote we'd need some more
> >changes in UiCommon models anyway.
we *must* be able to specify full remove confirm msg only - we should not use
message "particles", as they cause translation-hell (sigular vs. plural,
etc.) -
we had multiple problems with that in the past - that's why we changed the
message
here from "are you sure ... <object-type>?" to the more generic "are
you sure
...
items?" [http://gerrit.ovirt.org/#/c/14621]
Thanks Einav, now it's clear to me. I think we can go with Daniel's suggestion, in
general we need to inspect/update all setMessage calls on ConfirmationModel instances.
thanks.
> ----- Original Message -----
> From: "Vojtech Szocs" <vszocs(a)redhat.com>
> Sent: Monday, October 21, 2013 8:57:16 AM
>
>
> ----- Original Message -----
> > From: "Daniel Erez" <derez(a)redhat.com>
> > To: "Vojtech Szocs" <vszocs(a)redhat.com>
> > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > "Lior Vernia" <lvernia(a)redhat.com>, "engine-devel"
> > <engine-devel(a)ovirt.org>
> > Sent: Monday, October 21, 2013 2:12:22 PM
> > Subject: Re: ui code: possible problem in 'remove' confirmation dialog
> >
> >
> >
> > ----- Original Message -----
> > > From: "Vojtech Szocs" <vszocs(a)redhat.com>
> > > To: "Daniel Erez" <derez(a)redhat.com>
> > > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > > "Lior Vernia" <lvernia(a)redhat.com>,
"engine-devel"
> > > <engine-devel(a)ovirt.org>
> > > Sent: Monday, October 21, 2013 12:55:05 PM
> > > Subject: Re: ui code: possible problem in 'remove' confirmation
dialog
> > >
> > >
> > >
> > > ----- 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)")
> >
> > Isn't it a cleanup that should be done either way?
> > (I assume this message isn't currently used on any context...).
>
> Yes, I guess some sort of cleanup will be required anyway.
>
> For me, the question is if we want to give models the ability to specify
> full
> remove confirm msg (i.e. "Are you sure...") or just the item part of it
> (i.e. "Virtual Machine(s)").
>
> >From localization perspective, full remove confirm msg seems more flexible
> >but will add more msg keys, however as you wrote we'd need some more
> >changes in UiCommon models anyway.
>
> > But indeed, it will require making sure that no one is
> > currently overriding the setMessage() by mistake to
> > avoid such possible regressions.
> >
> > >
> > > >
> > > > 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);
> > > }
> >
> > What about default value (when message is null)?
>
> The setMessage code I've mentioned above was based on assumption that
> UiCommon models such as VmListModel will always follow convention like
> removeConfirmDialogModel.setMessage("item(s)") .. but maybe such
assumption
> is too strong.
>
> In general, you're right, there should be some default value handling
> included (I forgot about this, sorry).
>
> > Do you mean we should add another abstract class in
> > the hierarchy that would default it to
> > "Are you sure you want to remove the following items?".
> > [btw, we might have some localization issues with this
> > pattern as we've encountered with 'no items to display';
> > so just need to check it out].
>
> OK, in that case we can follow Daniel's suggestion (setMessage code that
> just
> delegates to super.setMessage unless message == null for which there'd be
> some default msg), along with updating relevant UiCommon models (i.e.
> strings used when calling setMessage).
>
> >
> > >
> > > >
> > > > >
> > > > > [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
> > > > > >
> > > > >
> > > >
> > >
> >
> _______________________________________________
> Engine-devel mailing list
> Engine-devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/engine-devel
>
>
>
10:03 a.m.
New subject: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog
[top posting]
opened a bugzilla in order to track this issue:
Bug 1021584 - message in the 'remove' confirmation dialog cannot be overridden in
some cases
[https://bugzilla.redhat.com/show_bug.cgi?id=1021584]
many thanks, everyone, for your input - I highly appreciate it!
----
Regards,
Einav
----- Original Message -----
From: "Vojtech Szocs" <vszocs(a)redhat.com>
To: "Einav Cohen" <ecohen(a)redhat.com>
Cc: "engine-devel" <engine-devel(a)ovirt.org>
Sent: Monday, October 21, 2013 9:39:04 AM
Subject: Re: [Engine-devel] ui code: possible problem in 'remove' confirmation
dialog
----- Original Message -----
> From: "Einav Cohen" <ecohen(a)redhat.com>
> To: "Vojtech Szocs" <vszocs(a)redhat.com>
> Cc: "Daniel Erez" <derez(a)redhat.com>, "engine-devel"
> <engine-devel(a)ovirt.org>
> Sent: Monday, October 21, 2013 3:28:56 PM
> Subject: Re: [Engine-devel] ui code: possible problem in 'remove'
> confirmation dialog
>
> > For me, the question is if we want to give models the ability to specify
> > full
> > remove confirm msg (i.e. "Are you sure...") or just the item part of
it
> > (i.e. "Virtual Machine(s)").
> >
> > >From localization perspective, full remove confirm msg seems more
> > >flexible
> > >but will add more msg keys, however as you wrote we'd need some more
> > >changes in UiCommon models anyway.
>
> we *must* be able to specify full remove confirm msg only - we should not
> use
> message "particles", as they cause translation-hell (sigular vs. plural,
> etc.) -
> we had multiple problems with that in the past - that's why we changed the
> message
> here from "are you sure ... <object-type>?" to the more generic
"are you
> sure
> ...
> items?" [http://gerrit.ovirt.org/#/c/14621]
Thanks Einav, now it's clear to me. I think we can go with Daniel's
suggestion, in general we need to inspect/update all setMessage calls on
ConfirmationModel instances.
>
> thanks.
>
> > ----- Original Message -----
> > From: "Vojtech Szocs" <vszocs(a)redhat.com>
> > Sent: Monday, October 21, 2013 8:57:16 AM
> >
> >
> > ----- Original Message -----
> > > From: "Daniel Erez" <derez(a)redhat.com>
> > > To: "Vojtech Szocs" <vszocs(a)redhat.com>
> > > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > > "Lior Vernia" <lvernia(a)redhat.com>,
"engine-devel"
> > > <engine-devel(a)ovirt.org>
> > > Sent: Monday, October 21, 2013 2:12:22 PM
> > > Subject: Re: ui code: possible problem in 'remove' confirmation
dialog
> > >
> > >
> > >
> > > ----- Original Message -----
> > > > From: "Vojtech Szocs" <vszocs(a)redhat.com>
> > > > To: "Daniel Erez" <derez(a)redhat.com>
> > > > Cc: "Einav Cohen" <ecohen(a)redhat.com>, "Gilad
Chaplik"
> > > > <gchaplik(a)redhat.com>, "Tomas Jelinek"
<tjelinek(a)redhat.com>,
> > > > "Lior Vernia" <lvernia(a)redhat.com>,
"engine-devel"
> > > > <engine-devel(a)ovirt.org>
> > > > Sent: Monday, October 21, 2013 12:55:05 PM
> > > > Subject: Re: ui code: possible problem in 'remove'
confirmation
> > > > dialog
> > > >
> > > >
> > > >
> > > > ----- 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)")
> > >
> > > Isn't it a cleanup that should be done either way?
> > > (I assume this message isn't currently used on any context...).
> >
> > Yes, I guess some sort of cleanup will be required anyway.
> >
> > For me, the question is if we want to give models the ability to specify
> > full
> > remove confirm msg (i.e. "Are you sure...") or just the item part of
it
> > (i.e. "Virtual Machine(s)").
> >
> > >From localization perspective, full remove confirm msg seems more
> > >flexible
> > >but will add more msg keys, however as you wrote we'd need some more
> > >changes in UiCommon models anyway.
> >
> > > But indeed, it will require making sure that no one is
> > > currently overriding the setMessage() by mistake to
> > > avoid such possible regressions.
> > >
> > > >
> > > > >
> > > > > 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);
> > > > }
> > >
> > > What about default value (when message is null)?
> >
> > The setMessage code I've mentioned above was based on assumption that
> > UiCommon models such as VmListModel will always follow convention like
> > removeConfirmDialogModel.setMessage("item(s)") .. but maybe such
> > assumption
> > is too strong.
> >
> > In general, you're right, there should be some default value handling
> > included (I forgot about this, sorry).
> >
> > > Do you mean we should add another abstract class in
> > > the hierarchy that would default it to
> > > "Are you sure you want to remove the following items?".
> > > [btw, we might have some localization issues with this
> > > pattern as we've encountered with 'no items to display';
> > > so just need to check it out].
> >
> > OK, in that case we can follow Daniel's suggestion (setMessage code that
> > just
> > delegates to super.setMessage unless message == null for which there'd be
> > some default msg), along with updating relevant UiCommon models (i.e.
> > strings used when calling setMessage).
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > [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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > _______________________________________________
> > Engine-devel mailing list
> > Engine-devel(a)ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/engine-devel
> >
> >
> >
>
_______________________________________________
Engine-devel mailing list
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel
3846
days inactive
3850
days old
11 comments
4 participants
participants (4)
-
Daniel Erez -
Einav Cohen -
Greg Sheremeta -
Vojtech Szocs