[Laszlo-dev] For Review: Change 20081001-bargull-c2C Summary: selection management fixes

[André Bargull]

Thanks for the review!
I've addressed each point you mentioned and checked in the revised files.

On 10/1/2008 11:11 PM, P T Withington wrote:
> Comments:
> 
> I would use `===' in the places where you are looking up nodes, just to 
> be clear that you are looking for an identical match.
> 
> My convention for marking things that should be fixed when another bug 
> that is fixed is to say:
> 
>   TODO: [YYYY-MM-DD ptw] (LPP-NNNN) Do this when that is fixed.
> 
> In theory, then someone can easily grep the code for LPP-NNNN when they 
> fix it.  There should also be a noted in the bug description saying that 
> there are TODO's in the code that need to be addressed when the bug is 
> fixed.  IWBNI we all used the same format... although I realize others 
> use more free-form TODO's.
> 
> I wonder if getSelected should return a copy of the selected array, 
> rather than the actual array?
> 
> It would be more obvious to me if lastRange were named either 
> lastRangeStart or lastSelect (which is what it really is -- it is the 
> anchor any range-select gesture is relative to).
> 
> Approved.
> 
> On 2008-10-01, at 12:04EDT, André Bargull wrote:
> 
>> Change 20081001-bargull-c2C by bargull at dell--p4--2-53 on 2008-10-01 
>> 16:02:23
>> in /home/Admin/src/svn/openlaszlo/trunk
>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: selection management fixes
>>
>> New Features:
>>
>> Bugs Fixed: LPP-7094, LPP-7095, LPP-7096, LPP-7097, LPP-7098
>>
>> Technical Reviewer: ptw
>> QA Reviewer: (pending)
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>> LPP-7094: clearSelection() changed in Lz(Data)SelectionManager:
>> - store the previous selection and then create fresh variables to hold 
>> new selections
>> - this avoids a possible non-terminating state (see testcase at 
>> bugreport)
>> LPP-7095: fixed by LPP-7094 bugfix
>> LPP-7096: select() changed in LzSelectionManager:
>> - removed "if (s!=o)", which was meant as a speed-up (?), but turned 
>> out to be buggy and which was never true for LzDataSelectionManager,
>> because s is a LzView, whereas o is a LzDataNode.
>> LPP-7097: unselect() changed in LzDataSelectionManager:
>> - only call "setSelected(false)" when actually found a valid selection
>> LPP-7098: makeSelected() changed in LzDataSelectionManager:
>> - set first "singleClone" and then call "setSelected(true)"
>> - applied same fix in unselect() and __LZsetSelected()
>>
>> Added indentation for LPP-2623.
>> Added typing for LPP-6049.
>> Used LzNode#__LZUID instead of LzNode#getUID() now that there is no 
>> longer the requirement to call the getter.
>>
>>
>> Tests:
>> a testcase for every bug is attached at the bugreports
>>
>> Files:
>> M WEB-INF/lps/lfc/helpers/LzDataSelectionManager.lzs
>> M WEB-INF/lps/lfc/helpers/LzSelectionManager.lzs
>>
>> Changeset: 
>> http://svn.openlaszlo.org/openlaszlo/patches/20081001-bargull-c2C.tar
>>
>>
> 
>