#6219 closed enhancement (fixed)
BorderContainer: Add public drag notifications
Reported by: | ptwobrussell | Owned by: | Sam Foster |
---|---|---|---|
Priority: | high | Milestone: | 1.2 |
Component: | Dijit | Version: | 1.0 |
Keywords: | Cc: | Sam Foster, ptwobrussell, Adam Peller | |
Blocked By: | Blocking: |
Description (last modified by )
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
Attachments (3)
Change History (18)
comment:1 Changed 13 years ago by
Cc: | Sam Foster added |
---|---|
Milestone: | → 1.2 |
Summary: | Add public drag notifications to BorderContainer → BorderContainer: Add public drag notifications |
comment:3 Changed 13 years ago by
Owner: | set to Sam Foster |
---|
comment:5 Changed 13 years ago by
Cc: | ptwobrussell Adam Peller added |
---|
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
Changed 13 years ago by
Attachment: | 6219_splitterEvents.patch added |
---|
[PATCH][CLA] Naive implementation of the splitter events for further discusison
comment:6 Changed 13 years ago by
Description: | modified (diff) |
---|
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
comment:7 Changed 13 years ago by
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?
Changed 12 years ago by
Attachment: | 6219_getSplittersAndEvents.patch added |
---|
[PATCH] [CLA] adds getSplitter, getSplitters to BC, onMove, onMoveStart, onMoveEnd splitter methods. Plus simple test/use case
comment:8 Changed 12 years ago by
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.
Changed 12 years ago by
Attachment: | 6219_BCgetSplitter.patch added |
---|
[PATCH] [CLA] adds getSplitter to BC, adds a test case
comment:9 Changed 12 years ago by
Description: | modified (diff) |
---|
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.
comment:10 Changed 12 years ago by
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.
comment:11 Changed 12 years ago by
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
comment:12 Changed 12 years ago by
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.
comment:13 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
- 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
comment:14 Changed 12 years ago by
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.
Please list (some of) the use cases.