Re: [Engine-devel] ui code: possible problem in 'remove' confirmation dialog

20 Oct 2013


      ----- Original Message -----
...
From: "Einav Cohen" <ecohen@redhat.com>
To: "Daniel Erez" <derez@redhat.com>, "Greg Sheremeta" <gshereme@redhat.com>
Cc: "engine-devel" <engine-devel@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@redhat.com>
Sent: Friday, October 18, 2013 6:06:02 AM
----- Original Message -----
...
From: "Einav Cohen" <ecohen@redhat.com>
To: "Daniel Erez" <derez@redhat.com>, "Gilad Chaplik"
<gchaplik@redhat.com>, "Tomas Jelinek" <tjelinek@redhat.com>,
"Vojtech Szocs" <vszocs@redhat.com>, "Lior Vernia" <lvernia@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,
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@redhat.com>
To: "Daniel Erez" <derez@redhat.com>, "Gilad Chaplik"
<gchaplik@redhat.com>, "Tomas Jelinek" <tjelinek@redhat.com>,
"Vojtech Szocs" <vszocs@redhat.com>, "Lior Vernia" <lvernia@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