[#LPP-3560] radiogroup and selecting an item

OpenLaszlo

radiogroup and selecting an item

Created: 16/Feb/07 01:40 PM Updated: 20/Aug/07 12:15 PM

Component/s:

Components - LZ

Affects Version/s:

OL4B1, 3.3.3

Fix Version/s:

4.0.5WaffleCone

Time Tracking:

Not Specified

Severity:	Minor
Fixed in Change#:	6,135
Runtime:	N/A
Flags:	External
Fix in hand:	True

Description

« Hide

The mechanism in a radiogroup to select an item was build somehow overcautious.

First, in the definition of the class "radiobutton" (components/lz/radio.lzx) the handler for the "onclick"-Event shouldn't be needed.
As "radiobutton" is derived from "baselistitem" it will invoke the method "doClick" on every "onclick" automatically
(see baselistitem#_selectonevent, baselistitem#setSelectOnEvent() and baselistitem#doClick() ).
(Also baselistitem#setSelectOnEvent could lead to possible memory leaks if this method is called several times as a new LzDelegate will be created everytime, but the old one won't invoke "unregisterAll", but this scenario isn't that "real-life", because most times the developer doesn't call this method...)

Second, in radiogroup#_setvalue(), the call to radiogroup#select() isn't needed everytime (e.g. it's necessary for the applyData-stuff, but unnecessary for manual user-selection).
This could be the stacktrace if the users selects a radiobutton:
radiobutton#onclick -> radiobutton#doClick() -> radiogroup#select() -> radiogroup#_setvalue() -> radiogroup#select() -> radiogroup#_setvalue()
You can see that radiogroup#select() will invoke radiogroup#_setvalue(), which will invoke another time radiogroup#select(), which will invoke again radiogroup#_setvalue().
So that's really unnecessary and this will lead to fire the event "onselect" for a radiogroup two times.

The comment in the method radiogroup#applyData (it's: "try to find the relevant radio button, or clear selection if none" ) sounds like all this select- and clearSelection-stuff in radiogroup#_setvalue was just for making applyData work with radiogroups.
So to fix this issue, I'd propose to put the relevant selection-stuff in applyData.

All

Comments

Work Log

Change History

Sort Order:

Ascending order - Click to sort in descending order

[ Permlink | « Hide ]

Guy Rouillier - 15/Jun/07 12:05 AM

The onselect double firing is still present in OL 4.0.2. I found a workaround on the forums here: http://forum.openlaszlo.org/showthread.php?p=31289#post31289

This is a significant bug - 4.0 would have been a great time to fix it. Without debugging, it's easy to miss, causing hidden extra processing.

[ Permlink | « Hide ]

Max Carlson - 17/Aug/07 03:02 AM

testcase:
<canvas debug="true" >

  <dataset name="site_ds">
    <menu mname="AAA" value="0" statename="stateA" />
    <menu mname="BBB" value="10" statename="stateB" />
  </dataset>

  <datapointer name="readState" xpath="" />

  <radiogroup name="menu_group" datapath="site_ds:/" >
    <attribute name="dirty" value="null" />
    
    <handler name="onselect" args="sel" >
if( dirty != sel ){
canvas.switchState( sel.datapath );
canvas.readState.setFromPointer( sel.datapath );
dirty = sel;
}
    </handler>
    <radiobutton name="m" datapath="menu" text="$path{'@mname'}" value="$path{'@value'}" />
  </radiogroup>

  
  <method name="switchState" args="dpMenu">
    Debug.write( dpMenu.getNodeAttribute( "mname" ) );
    Debug.write( dpMenu.getNodeAttribute( "value" ) );
    Debug.write( dpMenu.getNodeAttribute( "statename" ) );
    Debug.write( "---" );
  </method>

</canvas>

[ Permlink | « Hide ]

Max Carlson - 17/Aug/07 05:28 PM

Author: max
Date: 2007-08-17 17:08:10 -0700 (Fri, 17 Aug 2007)
New Revision: 6135

Modified:
   openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx
Log:
Change 20070817-maxcarlson-j by maxcarlson@plastik on 2007-08-17 13:59:45 PDT
    in /Users/maxcarlson/openlaszlo/wafflecone
    for http://svn.openlaszlo.org/openlaszlo/branches/wafflecone

Summary: Clean up radio

New Features:

Bugs Fixed: ~~LPP-3560~~ - radiogroup and selecting an item

Technical Reviewer: jcrowley
QA Reviewer: promanik
Doc Reviewer: (pending)

Documentation:

Release Notes:

Details: Per ~~LPP-3560~~, move duplicate selection code to applyData() and remove extra onclick handler.


Tests: See ~~LPP-3560~~.

Modified: openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx
===================================================================
--- openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx 2007-08-17 23:26:58 UTC (rev 6134)
+++ openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx 2007-08-18 00:08:10 UTC (rev 6135)
@@ -33,6 +33,12 @@
         <method name="applyData" args="d">
             //try to find the relevant radio button, or clear selection if none
             this._setvalue( d );
+ var item = null;
+ if (d != null) {
+ item = this.getItem( d );
+ }
+ if ( item ) this.select( item );
+ else this.clearSelection();
         </method>

         
@@ -44,8 +50,6 @@
                     item = this.getItem( val );
                 }
                 this.value = val;
- if ( item ) this.select( item );
- else this.clearSelection();
             } else {
                 this.value = val;
             }
@@ -156,13 +160,6 @@
         </method>

         
- <handler name="onclick">
- if (!this.selected) {
- parent.select(this);
- }
- </handler>
-
- 
         <method name="setHilite" args="dohilite">
             _title.setAttribute('fgcolor',
                 dohilite ? style.texthilitecolor : style.textcolor);

_______________________________________________
Laszlo-checkins mailing list
Laszlo-checkins@openlaszlo.org
http://www.openlaszlo.org/mailman/listinfo/laszlo-checkins

[ Show » ]

Max Carlson - 17/Aug/07 05:28 PM Author: max Date: 2007-08-17 17:08:10 -0700 (Fri, 17 Aug 2007) New Revision: 6135 Modified: openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx Log: Change 20070817-maxcarlson-j by maxcarlson@plastik on 2007-08-17 13:59:45 PDT in /Users/maxcarlson/openlaszlo/wafflecone for http://svn.openlaszlo.org/openlaszlo/branches/wafflecone Summary: Clean up radio New Features: Bugs Fixed: ~~LPP-3560~~ - radiogroup and selecting an item Technical Reviewer: jcrowley QA Reviewer: promanik Doc Reviewer: (pending) Documentation: Release Notes: Details: Per ~~LPP-3560~~, move duplicate selection code to applyData() and remove extra onclick handler. Tests: See ~~LPP-3560~~. Modified: openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx =================================================================== --- openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx 2007-08-17 23:26:58 UTC (rev 6134) +++ openlaszlo/branches/wafflecone/lps/components/lz/radio.lzx 2007-08-18 00:08:10 UTC (rev 6135) @@ -33,6 +33,12 @@ <method name="applyData" args="d"> //try to find the relevant radio button, or clear selection if none this._setvalue( d ); + var item = null; + if (d != null) { + item = this.getItem( d ); + } + if ( item ) this.select( item ); + else this.clearSelection(); </method>  @@ -44,8 +50,6 @@ item = this.getItem( val ); } this.value = val; - if ( item ) this.select( item ); - else this.clearSelection(); } else { this.value = val; } @@ -156,13 +160,6 @@ </method>  - <handler name="onclick"> - if (!this.selected) { - parent.select(this); - } - </handler> - -  <method name="setHilite" args="dohilite"> _title.setAttribute('fgcolor', dohilite ? style.texthilitecolor : style.textcolor); _______________________________________________ Laszlo-checkins mailing list Laszlo-checkins@openlaszlo.org http://www.openlaszlo.org/mailman/listinfo/laszlo-checkins

[ Permlink | « Hide ]

Mamye Kratt - 20/Aug/07 12:15 PM

(wafflecone branch local build r6155)
Closing.