[Laszlo-dev] For Review: Change 20081001-bargull-c2C Summary: selection management fixes
andre.bargull at udo.edu
Thu Oct 2 02:48:25 PDT 2008
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:
> 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).
> 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
>> 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)
>> Release Notes:
>> 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
>> 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.
>> a testcase for every bug is attached at the bugreports
>> M WEB-INF/lps/lfc/helpers/LzDataSelectionManager.lzs
>> M WEB-INF/lps/lfc/helpers/LzSelectionManager.lzs
More information about the Laszlo-dev