This is very cool stuff!

Bugs:

1) draggable:  There's a paste-oh in your doc -- two open <canvas> with only 
one close </canvas>

2) draggable:  Please use the regex '\\s*,\\s*' for your split pattern.

3) dragmanager:  A component should not be using a private LFC api 
(mouseevent).  Maybe we can motivate André to finish his prototype code for 
http://jira.openlaszlo.org/jira/browse/LPP-6034 and solve the "event might not 
be defined" problem more elegantly?

Issues:

1) I wonder if we should be more generic in some of the terms, instead of using 
`drag`* and `drop`*, use `source`* and `destination`*?  In particular, 
`droptest` might be better called something like `validdestination` and 
`dragtest` be `validsource`?

2) Should draggables also register with the dragmanager, so for instance the 
dragmanager could (optionally) highlight them when you mouse over them to 
indicate that they are draggable?

  a) I don't think you want the debug warning when there are no valid 
destinations for a source.  This could very much be a normal thing, and it 
would be indicated by the above:  the source would not highlight when you mouse 
over it, because it has no matching destinations (at the time).

Comments:

1) LzTrack, you could make unregistering more efficient by storing the index of 
the view in the list instead of just true in the UID hash.

2) And echoing André's comment, I suppose it is a stylistic choice, but in 
general we tend to use === only when it's additional semantics is necessary.  I 
think using it everywhere makes code harder to read, because most readers will 
trip over it and think "why id he using identity here?"

On 2010-12-29, at 13:38, Max Carlson wrote:

> Sending with the correct QA reviewer name...
> 
> Change maxcarlson-20101229-tsP by maxcarl...@friendly on 2010-12-29 09:42:25 
> PST
>    in /Users/maxcarlson/openlaszlo/trunk-clean
>    for http://svn.openlaszlo.org/openlaszlo/trunk
> 
> Summary: Add draggable and droppable mixins
> 
> Bugs Fixed: LPP-9591 - Add dragable and dropable mixin
> 
> Technical Reviewer: ptw
> QA Reviewer: ffeng
> 
> Release Notes: New draggable and droppable mixins add seamless drag and drop 
> capability to any view.
> 
> Overview: Adds draggable, droppable and a global dragmanager.
> 
> Details: LzTrack - Prevent duplicate registration for a given view/group.  == 
> -> ===.
> 
> lzx-autoincludes - Add autoinclude entries for draggable and droppable mixins.
> 
> boxmodel - Remove explicit include from doc example, as it's no longer needed.
> 
> draggable/library - All dependencies for draggable/droppable
> 
> draggable - Implements the draggable mixin.  See the docs for how to use.
> 
> droppable - Implements the droppable mixin.  See the docs for how to use.
> 
> dragmanager - Tracks global drag state, delegates events to draggable and 
> droppable instances.
> 
> Tests: Inline doc example and examples/draggable.lzx work consistently across 
> all runtimes.
> 
> Files:
> M       WEB-INF/lps/lfc/services/LzTrack.lzs
> M       WEB-INF/lps/misc/lzx-autoincludes.properties
> M       lps/components/mixins/boxmodel.lzx
> A       lps/components/mixins/draggable
> A       lps/components/mixins/draggable/library.lzx
> A       lps/components/mixins/draggable/draggable.lzx
> A       lps/components/mixins/draggable/dragmanager.lzx
> A       lps/components/mixins/draggable/droppable.lzx
> A       examples/draggable.lzx
> 
> Changeset: 
> http://svn.openlaszlo.org/openlaszlo/patches/maxcarlson-20101229-tsP.tar


Reply via email to