Opened 13 years ago
Closed 13 years ago
#7598 closed defect (fixed)
"Janky" resize in border container for IE
Reported by: | Dustin Machi | Owned by: | Dustin Machi |
---|---|---|---|
Priority: | high | Milestone: | 1.2 |
Component: | Dijit | Version: | 1.2beta |
Keywords: | border container resize | Cc: | |
Blocked By: | Blocking: |
Description
When the 'janky' branch of bc's _layoutChildren gets called it fails to pass on a correct/complete box to the children's resize method. A test file is attached.
This method, in the janky branch is declared
var resizeWidget = function(widget, dim){ if(widget){ (widget.resize ? widget.resize(dim) : dojo.marginBox(widget.domNode, dim)); } };
followed by several lines:
if(leftSplitter){ leftSplitter.style.height = sidebarHeight; } if(rightSplitter){ rightSplitter.style.height = sidebarHeight; } resizeWidget(this._leftWidget, {h: sidebarHeight}, dim.left); resizeWidget(this._rightWidget, {h: sidebarHeight}, dim.right); if(topSplitter){ topSplitter.style.width = sidebarWidth; } if(bottomSplitter){ bottomSplitter.style.width = sidebarWidth; } resizeWidget(this._topWidget, {w: sidebarWidth}, dim.top); resizeWidget(this._bottomWidget, {w: sidebarWidth}, dim.bottom); resizeWidget(this._centerWidget, dim.center);
all execpt the final resizeWidget call is incorrect. There are only two params to the resizeWidget call. I believe the middle 'object' can just be removed. In my local tests so far I am getting data through that is now close to correct. They height thus far always appears to be 3px off, but this may be something in the app I am working on not the bc.
Attachments (2)
Change History (13)
Changed 13 years ago by
Attachment: | bcPatch.diff added |
---|
comment:1 Changed 13 years ago by
Owner: | set to Adam Peller |
---|---|
Status: | new → assigned |
comment:2 Changed 13 years ago by
I think I have a related issue, but my test is more complex:
Dojo 1.1.1 example: http://jburke.dojotoolkit.org/bugs/7598/ComplexBorderContainer.html
Dojo 1.2beta1 example: http://jburke.dojotoolkit.org/bugs/7598/ComplexBorderContainer12.html
It seems like the margins set on the parent widget (a my.RoundRect? in my example) that contains the middle BorderContainer? are not taken into account. If I manually set equivalent margins on the center border container, things looked better, but the height sizing was still inconsistent.
I was able (at least in the 1.1.1 code) to change the code so that Firefox used the janky path, and the problem was reproducible in that browser.
comment:3 Changed 13 years ago by
The problem isn't the calls to resizeWidget() but rather the definition of resizeWidget(). It should be taking three arguments, to match _LayoutWidget.resize() (which takes two arguments, plus the implicit pointer to the widget itself):
resize: function(newSize, currentSize){
comment:5 Changed 13 years ago by
OK, I take back my comment, because:
- if you resize the browser window need to pass both width and height, because both have changed
- in the case where one dimension has changed (for example when you move the splitter for the bottom pane) then the height has changed but we are passing the width for second argument
- (as dustin pointed out to me) the code is currently passing the new size as the third argument to resizeWindow(), not the current size
Anyway, I think Dustin was right originally, sorry for the confusion.
comment:6 Changed 13 years ago by
Owner: | changed from Adam Peller to Dustin Machi |
---|---|
Status: | assigned → new |
comment:9 Changed 13 years ago by
[15216] broke mail demo. Click "new message" on IE7 and see the "To" and "Subject" fields overlapping editor.
comment:10 Changed 13 years ago by
OK, I take back all my comments above. I was half asleep when I wrote them. It turns out that (knock on wood) the code was functionally basically correctly originally.
BorderContainer is supposed to query the height of the top and bottom panes rather than setting it. It queries the height of the top and bottom panes and then adjusts the height of the center pane to fill the remaining space. The only time it sets the height of the top/bottom panes is when the user drags the splitter, and that's handled in the Splitter class with a dojo.marginBox() call. (Note that at some point this code should be refactored because it's inefficient and dangerous to set the size and then query it immediately after; there are TODO comments in the code for this.)
The API comments for resize(), however, were very confusing (my fault). I'm going to checkin a fix to those comments and revert most of the changes to the BorderContainer class.
The only caveat is that the first argument to resize() might contain only height or only width (or be null). That's the way I always intended the API to be. Unfortunately it wasn't documented very well.
comment:11 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch file