BorderContainer: Add public drag notifications

[ptwobrussell]

Following discussion in #dojo, create a getSplitter api on the BC to allow connection to the invidiual splitter events, and a test case showing it in action

[bill]

Please list (some of) the use cases.

[Adam Peller]

and why standard DOM event notifications aren't sufficient

[Adam Peller]

oh, are we talking about the splitters?

[Sam Foster]

I was going to approach this by adding a topic name to the properties of the splitter that we can publish events to. This is of use when you might want content to go into a different state while the splitter is being moved, persist the BC state, defer other updates to the page and so on. There might also be times when you want to abort the drag - though I'm not sure this ticket covers all of that.

If anyone has suggestions for the new property name - it needs to be on the BC prototype, I think and passed onto the splitters' ctor. And, any expectations for what the event payload would be. I'm thinking the event name (dragstart/end/move) container's region, and splitter position

[Sam Foster]

[PATCH][CLA] Naive implementation of the splitter events for further discusison

[Sam Foster]

Its important that if we do allow a user to subscribe to splitter events including ther ondrag/onmove event - which fires very frequently - that we dont incurr a performance loss disproportionate to the amount of work that handler really needs to do. With that in mind, it would be ideal to be pretty granular about exactly which splitter, and which phase of the drag you were interested in recieving events from. That presents a potential problem in the proliferation of topics - particulaly if you wanted to e.g. subscribe to any splitter drag event

[bill]

Hmm, I'd need to be convinced that topics are better than just connect points on the widget.

The other question is how this is an improvement over monitoring onResize on the individual panes?

[Sam Foster]

[PATCH] [CLA] adds getSplitter, getSplitters to BC, onMove, onMoveStart, onMoveEnd splitter methods. Plus simple test/use case

[Sam Foster]

6219_getSplittersAndEvents.patch implements I think what we discussed. I made getSplitter(/*String*/region) and getSplitters. on the splitter class, onMoveStart, onMove and onMoveEnd stubs are called for the respective drag phases.

I'm open to ideas on what the event payload should be. Currently its type,target (the splitter widget), x, y. The coords are the top/left style values (ie. relative to the BC not the window). I could mixin to the native event if that would be more useful - I'd have to add a prefix or something to the artificial event property names at that point.

[Sam Foster]

[PATCH] [CLA] adds getSplitter to BC, adds a test case

[Sam Foster]

6219_BCgetSplitter.patch reworked to just add a getSplitter method to BorderContainer?, a couple caching/optimizing vars in Splitter. Also fixes an IE bug with BC.getChildren which was wanting to call filter on an array. The watchedBC test case shows connects to the _startDrag, _endDrag and domNode.mousemove to update the UI - which is the actual goal of this ticket.

[bill]

Yah, looks reasonable to me; hoping Adam will check it.

The thing about

var
       a,
       b,
       c

(without repeating "var"). I like it but Adam tells me that it confuses shrinksafe. Or at least prevents it from doing optimizations of shortening the variable names.

[Sam Foster]

With Adam's blessing I'll roll back the var _{optimization and commit this.}

The filter thing btw highlights a testing hole - that broken getChildren (ie IE) method had gone undetected apparently

[bill]

OK thanks. About the filter() thing, I thought that Container.getChildren() was returning NodeList, as per the code:

dojo.query("> [widgetId]", this.containerNode).map(dijit.byNode)

(I thought that map() returned something NodeList-esque even if it didn't contain nodes. Hmm, I guess not. Thanks for catching it.

[Sam Foster]

(In [14839]) fixes #6219

• adds getSplitter method to BorderContainer?
• fixes a bug with getChildren in IE (getChildren returns an array not a NodeList?)
• adds splitter-watching test case

!strict

[Adam Peller]

Sorry for the late review, but yes, what Bill said... each var needs its own 'var' stmt for now, unfortunately. See #1768.

I wasn't sure it was worth a local var for two uses of this.horizontal -- the actual savings is probably minimal, but I guess it couldn't hurt.

Also, we need API docs for getSplitter, as simple as it might be.

[Adam Peller]

(In [15026]) doc public method, style changes. Refs #6219 !strict