You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@pivot.apache.org by Edvin Syse <ed...@sysedata.no> on 2011/06/14 22:13:53 UTC
Update TabData - supposed to call listeners manually?
The TabData#title on my TabPane tab might change during editing of a
domain object, and I find I have to do the following to update the tab
correctly:
tabData.setText("New text for tab");
TabPane tabPane = (TabPane) getParent(); // The actual tab
(this) is a BoxPane
for (TabPaneAttributeListener l :
tabPane.getTabPaneAttributeListeners())
l.tabDataChanged(tabPane, this, tabData);
If I don't notify the listers, the TabData is re-rendered if I hover
over the Tab, but if the new text takes more space, the tab isn't
expanded to show all the text. I suspect this is as it should be, but
maybe there is a better way to notify TabData changes?
-- Edvin
Re: Update TabData - supposed to call listeners manually?
Posted by Greg Brown <gk...@verizon.net>.
> The more I think about it, the more I believe that there shouldn't be any comparison and that the caller should decide. If you call the method, you want a redraw, so any equals() method would only add overhead, and possibly stop the update you explicitly called for.
I tend to agree. I also don't think that this change needs to be applied to *all* setters - only setters that take a value without a specific type (i.e. Object) and fire change events. Setters with a more specific type have knowledge about the implications of performing an equality check and can decide whether it is appropriate or not. Untyped setters can't make such a determination so should not attempt to perform the check. This is also consistent with the behavior of List#update().
Re: Update TabData - supposed to call listeners manually?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 14.06.2011 23:03, skrev Greg Brown:
>> It would have to do null-checks, or switch to if (!tabData.equals(previousTabData)), but even then, ButtonData would need an equals() method. If the equals() method checked the text and userData properties (and skip the Image) that would work :)
>
> That's a good point, too. I definitely wouldn't want to incur the performance penalty of a byte-by-byte image comparison! Though it is probably OK to do a reference equality check for images, since they are often shared within an app.
The more I think about it, the more I believe that there shouldn't be
any comparison and that the caller should decide. If you call the
method, you want a redraw, so any equals() method would only add
overhead, and possibly stop the update you explicitly called for.
Re: Update TabData - supposed to call listeners manually?
Posted by Greg Brown <gk...@verizon.net>.
> It would have to do null-checks, or switch to if (!tabData.equals(previousTabData)), but even then, ButtonData would need an equals() method. If the equals() method checked the text and userData properties (and skip the Image) that would work :)
That's a good point, too. I definitely wouldn't want to incur the performance penalty of a byte-by-byte image comparison! Though it is probably OK to do a reference equality check for images, since they are often shared within an app.
Re: Update TabData - supposed to call listeners manually?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 14.06.2011 22:56, skrev Edvin Syse:
>
>
> Den 14.06.2011 22:49, skrev Roger L. Whitcomb:
>> Maybe TabPane should be testing:
>> if (!previousTabData.equals(tabData))
>> instead of using "=="??
>>
>
> It would have to do null-checks, or switch to if
> (!tabData.equals(previousTabData)), but even then, ButtonData would need
> an equals() method. If the equals() method checked the text and userData
> properties (and skip the Image) that would work :)
.. or maybe it should be the caller's responsibility to check, and only
call setTabData if you actually either change the tabData object or one
of it's properties? :) I don't know enough about Pivot internals to mean
anything about the ramifications, though.
Re: Update TabData - supposed to call listeners manually?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 14.06.2011 22:49, skrev Roger L. Whitcomb:
> Maybe TabPane should be testing:
> if (!previousTabData.equals(tabData))
> instead of using "=="??
>
It would have to do null-checks, or switch to if
(!tabData.equals(previousTabData)), but even then, ButtonData would need
an equals() method. If the equals() method checked the text and userData
properties (and skip the Image) that would work :)
-- Edvin
RE: Update TabData - supposed to call listeners manually?
Posted by "Roger L. Whitcomb" <Ro...@ingres.com>.
Maybe TabPane should be testing:
if (!previousTabData.equals(tabData))
instead of using "=="??
Roger Whitcomb | Architect, Engineering | Roger.Whitcomb@ingres.com |
Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 |
USA +1 650-587-5596 | fax: +1 650-587-5550
-----Original Message-----
From: Edvin Syse [mailto:edvin@sysedata.no]
Sent: Tuesday, June 14, 2011 1:43 PM
To: user@pivot.apache.org
Subject: Re: Update TabData - supposed to call listeners manually?
Den 14.06.2011 22:37, skrev Greg Brown:
> You should call:
>
> tabData.setText("New text for tab");
> TabPane.setTabData(component, tabData);
>
> That will fire the appropriate event.
That's the first thing I tried. It doesn't work because
TabPane#setTabData will only call the listeners if previousTabData !=
tabData. I don't change the TabData (ButtonData) object, I just change
the text property.
I can ofcourse create a brand new ButtonData object, but that doesn't
feel right. What do you think?
> FYI, the fact that you can call the listeners yourself is a bug. The
contents of the listener list are not supposed to be visible. :-)
OK :)
RE: Update TabData - supposed to call listeners manually?
Posted by "Roger L. Whitcomb" <Ro...@ingres.com>.
I had this same issue and ended up creating a new object every time.
But, what if "setTabData" just checked for !equals()? And then have
ButtonData implement "equals()" with "equals()" checks on the text, icon
and userData? The "equals()" for "text" (String) does the right thing,
icon (Image) just does reference equality, as does the default for
"userData" (often null anyway).
But, having "setTabData" always do the notify might not be a bad thing,
although you might have to weigh the probability that the objects really
are the same vs. the cost of doing an unnecessary repaint.
Roger Whitcomb | Architect, Engineering | Roger.Whitcomb@ingres.com |
Ingres | 500 Arguello Street | Suite 200 | Redwood City | CA | 94063 |
USA +1 650-587-5596 | fax: +1 650-587-5550
-----Original Message-----
From: Edvin Syse [mailto:edvin@sysedata.no]
Sent: Tuesday, June 14, 2011 2:05 PM
To: user@pivot.apache.org
Subject: Re: Update TabData - supposed to call listeners manually?
Den 14.06.2011 23:01, skrev Greg Brown:
>>>
> b) A logical equality check may simply be unnecessary overhead. If a
caller has explicitly invoked a setter for a property, we might
reasonably assume that the caller genuinely wants to update the
property's value. Calling an equals() method on the value in that case
just incurs a needless performance hit.
>
> What do you think?
That's exactly my though! "If you called it, you probably meant it." And
the equals check is unnecessary overhead as you say :)
-- Edvin
Re: Update TabData - supposed to call listeners manually?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 14.06.2011 23:01, skrev Greg Brown:
>>>
> b) A logical equality check may simply be unnecessary overhead. If a caller has explicitly invoked a setter for a property, we might reasonably assume that the caller genuinely wants to update the property's value. Calling an equals() method on the value in that case just incurs a needless performance hit.
>
> What do you think?
That's exactly my though! "If you called it, you probably meant it." And
the equals check is unnecessary overhead as you say :)
-- Edvin
Re: Update TabData - supposed to call listeners manually?
Posted by Greg Brown <gk...@verizon.net>.
>> You should call:
>>
>> tabData.setText("New text for tab");
>> TabPane.setTabData(component, tabData);
>>
>> That will fire the appropriate event.
>
> That's the first thing I tried. It doesn't work because TabPane#setTabData will only call the listeners if previousTabData != tabData. I don't change the TabData (ButtonData) object, I just change the text property.
That's a good one!
There is a similar issue with List#update(), but there the event is always fired regardless of equality. I wonder if TabPane#setTabData() (as well as Button#setButtonData(), etc.) should do the same and always fire the event. These methods could check for logical equality but:
a) If a property allows null, logical equality checks get kind of ugly.
b) A logical equality check may simply be unnecessary overhead. If a caller has explicitly invoked a setter for a property, we might reasonably assume that the caller genuinely wants to update the property's value. Calling an equals() method on the value in that case just incurs a needless performance hit.
What do you think?
G
Re: Update TabData - supposed to call listeners manually?
Posted by Edvin Syse <ed...@sysedata.no>.
Den 14.06.2011 22:37, skrev Greg Brown:
> You should call:
>
> tabData.setText("New text for tab");
> TabPane.setTabData(component, tabData);
>
> That will fire the appropriate event.
That's the first thing I tried. It doesn't work because
TabPane#setTabData will only call the listeners if previousTabData !=
tabData. I don't change the TabData (ButtonData) object, I just change
the text property.
I can ofcourse create a brand new ButtonData object, but that doesn't
feel right. What do you think?
> FYI, the fact that you can call the listeners yourself is a bug. The contents of the listener list are not supposed to be visible. :-)
OK :)
Re: Update TabData - supposed to call listeners manually?
Posted by Greg Brown <gk...@verizon.net>.
You should call:
tabData.setText("New text for tab");
TabPane.setTabData(component, tabData);
That will fire the appropriate event.
FYI, the fact that you can call the listeners yourself is a bug. The contents of the listener list are not supposed to be visible. :-)
G
On Jun 14, 2011, at 4:13 PM, Edvin Syse wrote:
> The TabData#title on my TabPane tab might change during editing of a domain object, and I find I have to do the following to update the tab correctly:
>
> tabData.setText("New text for tab");
> TabPane tabPane = (TabPane) getParent(); // The actual tab (this) is a BoxPane
> for (TabPaneAttributeListener l : tabPane.getTabPaneAttributeListeners())
> l.tabDataChanged(tabPane, this, tabData);
>
> If I don't notify the listers, the TabData is re-rendered if I hover over the Tab, but if the new text takes more space, the tab isn't expanded to show all the text. I suspect this is as it should be, but maybe there is a better way to notify TabData changes?
>
> -- Edvin