Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#10113 closed defect (fixed)

[patch] [ccla] TabContainer: with doLayout=false width problem

Reported by: Pete Smith Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.4.0b
Keywords: Cc: Rob Retchless
Blocked By: Blocking:

Description (last modified by bill)

I have tabContainers with doLayout=false in 1.3.2. The tabContainers are within a parent tables' td. The current behavior is, they auto width to their parent. With the new 1.4.0b1 tabs, they blow out super wide. If I set the width of the tabContainer with a style, they obey. But I want it to be the equiv of 100% width, and just take up the parent container's space. Here is my code how I do it. It seems that I must explicitly set width or they go crazy.

        var tabs = new dijit.layout.TabContainer({
          doLayout: false
        },
        tabcontainer);

must be:

        var tabs = new dijit.layout.TabContainer({
          doLayout: false,
style: 'width: 900px'
        },
        tabcontainer);

Attachments (3)

test_TabContainer_noLayout.html (4.7 KB) - added by Pete Smith 10 years ago.
_TabContainerBase.js.patch (762 bytes) - added by Adam Peller 9 years ago.
patch from Rob Retchless (IBM, CCLA)
TabContainer.patch (6.5 KB) - added by bill 9 years ago.
New patch from Rob that fixes the problem.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 10 years ago by bill

Component: GeneralDijit
Owner: anonymous deleted
Summary: TabContainer with doLayout=false doesn't fill containing elementTabContainer: with doLayout=false doesn't fill containing element

Thanks, please attach a full test case using the "attach file" button, rather than just a code snippet. Otherwise I can't reproduce the problem.

Changed 10 years ago by Pete Smith

comment:2 Changed 10 years ago by Pete Smith

I have attached the change. All I did from the standard test file, was remove the style="width:900px" and add the containing table. I spent about an hour messing around with the 1.3.2 version of this file, which behaves perfectly. This is a regression from the current 1.3.2 behavior. I need the tabs to simply fill out the parent td's width as they normally would. I don't want to specify a width on the tabs in doLayout=false mode.

comment:3 Changed 10 years ago by Pete Smith

the width of the tabList is being blown out, check this html see the 51006px?

<div dojoattachpoint="tablistWrapper" class="dijitTabListWrapper dijitTabContainerTopNone dijitAlignClient" style="height: 24px; left: 0px; top: 0px; right: auto; bottom: auto; width: 51006px;">

comment:4 Changed 10 years ago by bill

Milestone: tbd1.4
Owner: set to bill
Summary: TabContainer: with doLayout=false doesn't fill containing elementTabContainer: with doLayout=false width problem

Oh, that's because of the ScrollingTabController, you can actually specify that TabContainer can have a different controllerWidget (specifically TabController instead of ScrollingTabController), which should solve the problem, but maybe that should be the default for doLayout=false.

The thing is that (as you said) scrolling tab labels work fine as long as a width is specified, so not sure if I just want to turn them off just because doLayout=false. But it's also hard to reliably test if a width has been specified since it could be specified indirectly via CSS. Perhaps just check if it's directly specified.

comment:5 Changed 10 years ago by Pete Smith

controllerWidget: 'dijit.layout.TabController?'

fixed it for me. For feedback though, the noLayout behavior seems to be the most basic type of behavior. All the scrolling fanciness should take extra config and options.

comment:6 Changed 10 years ago by bill

Right, noLayout is the most basic type of behavior. But we discussed in http://thread.gmane.org/gmane.comp.web.dojo.devel/11273 at length about what the default behavior should be and ended up making scrolling the default, since that's what most developers want.

comment:7 Changed 10 years ago by bill

Resolution: fixed
Status: newclosed

(In [20710]) Fix TabContainer? width problem with doLayout=false. If no width is specified then use explicit 100%. Fixes #10113 !strict.

comment:8 Changed 10 years ago by bill

(In [20906]) Only apply the width: 100% in doLayout=false mode, to fix rendering problems when a TabContainer is embedded in a BorderContainer.

Fixes #10401, refs #10113 !strict.

comment:9 Changed 9 years ago by Rob Retchless

Resolution: fixed
Status: closedreopened

I'm not sure why this bug was resolved, because this is still broken. If I don't provide a width to a doLayout=false TabContainer?, natural width is the expectation. Right now, it busts out to 50000px wide. Definitely a bug.

I've dug through the code, and found the following options to fix it:

a) In _TabContainerBase#layout, in the doLayout=false else block (line 121), just display="none" the tablist before you test the width of this.domNode, then display="", then call tablist.resize() with the tested width. That way the super-wide scroller div doesn't get in the way of the this.domNode width test.

b) A slightly more performant option would be for ScrollingTabController? to be display: none by default, and only to show itself the first time resize is called. The only issue here is that future onresize events would have to display none it temporarily so that the width is calculated properly. So maybe it's best to just go with option a).

comment:10 Changed 9 years ago by Rob Retchless

Thought of some extra rationale: We don't want to turn off scrolling tabs (although that's an excellent workaround)...we want the scrolling tabs to work with doLayout=false. The main reason people make tabs doLayout=false is to achieve a natural height and width content area. Scrolling tabs are still desirable in this case, and should not undermine the natural width. The fix options in my previous comment address this.

comment:11 Changed 9 years ago by bill

Cc: Rob Retchless added

I closed it because everything that worked before scrolling tabs were implemented still works, albeit that you have to add the controllerWidget: 'dijit.layout.TabController' parameter.

But do you have a CLA? If so please attach the patch for (a) and I'll try it out.

comment:12 Changed 9 years ago by bill

Resolution: wontfix
Status: reopenedclosed

Changed 9 years ago by Adam Peller

Attachment: _TabContainerBase.js.patch added

patch from Rob Retchless (IBM, CCLA)

comment:13 Changed 9 years ago by Adam Peller

added patch from Rob

comment:14 Changed 9 years ago by bill

Milestone: 1.41.6
Resolution: wontfix
Status: closedreopened
Summary: TabContainer: with doLayout=false width problem[patch] [ccla] TabContainer: with doLayout=false width problem

comment:15 Changed 9 years ago by bill

Unfortunately, after applying this patch, when I load test_TabContainer_noLayout.html [on IE8], press the "add tab" button a bunch of times, and then press the right arrow to scroll left, nothing happens.

Changed 9 years ago by bill

Attachment: TabContainer.patch added

New patch from Rob that fixes the problem.

comment:16 Changed 9 years ago by bill

Description: modified (diff)

comment:17 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [23312]) Patch from Rob Retchless (IBM, CCLA) to fix width problem for doLayout=false scrolling tablist TabContainer inside of a table with no explicit width specified. Fixes #10113 !strict. Thanks!

comment:18 Changed 9 years ago by bill

(In [23313]) Fix test failures, refs #10113

Note: See TracTickets for help on using tickets.