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

[P T Withington]

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
>
>