Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#12097 closed defect (fixed)

TabContainer not resized when belonging to a dijit.form.Form

Reported by: goriol Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc:
Blocked By: Blocking:

Description

With the following widget hierarchy:

<Dialog>
    <Form>
         <TabContainer>
    </Form>
</Dialog>

the TabContainer? doesn't display properly.

When Dialog displays it tries to call Form.resize(), expecting that Form will call resize() on it's children. But since Form isn't a layout widget nothing happens.

To work around the problem, one can add a resize() method to the form that just calls resize on it's child/children:

<div dojoType=dijit.form.Form>
     <script type="dojo/method" event="resize">
          dojo.forEach(this.getChildren(), function(child){ child.resize(); });
     </script>
</div>

Attachments (4)

tabContainer.html (2.4 KB) - added by goriol 9 years ago.
test case
managerPatch.patch (3.9 KB) - added by bill 8 years ago.
abandoned patch to solve this in ContentPane?
testform.html (4.8 KB) - added by ebengtso 8 years ago.
removed unecessary div
simple.html (1.1 KB) - added by bill 8 years ago.
stripped down testform.html (still showing rendering problem)

Download all attachments as: .zip

Change History (23)

Changed 9 years ago by goriol

Attachment: tabContainer.html added

test case

comment:1 Changed 9 years ago by goriol

To make the test case work, one can add the following code to the dojo.addOnLoad():

    dijit.form.Form.prototype.resize = function () {
        dojo.forEach(this.getChildren(), function (child) { child.resize(); });
    };

comment:2 Changed 8 years ago by bill

Owner: set to bill

Thanks. I started to work up a solution to this, but ran into an issue.

Dialog extends ContentPane. It's easy to make ContentPane.resize() skip over child elements without a resize() method and instead call resize() their grandchildren. That will solve the !Dialog example in the description of this ticket.

However, ContentPane has special behavior when it has a single layout widget as a child, and there's an issue with that:

<TabContainer>
    <ContentPane>
         <BorderContainer/>
    </ContentPane>
</TabContainer>

In the above case the TabContainer sets the size of the ContentPane, and then the ContentPane sets the BorderContainer to that same size, acting as a pass-through widget. Thus the BorderContainer size is synced with the size of the TabContainer, minus room for the tab labels.

We want the same behavior if there's an intermediate form widget. The BorderContainer's size below should still be synced with the TabContainer:

<TabContainer>
    <ContentPane>
          <Form>
              <BorderContainer/>
           </Form>
    </ContentPane>
</TabContainer>

On the other hand, if the BorderContainer parent were some other widget with it's own markup (something like a TitlePane) then the BorderContainer shouldn't be resized.

So, I'll probably just end up adding code to Form for it to pass through resize() calls the same way as ContentPane does, detecting whether or not there's a single child widget.

comment:3 Changed 8 years ago by bill

Milestone: tbd1.6
Status: newassigned

Changed 8 years ago by bill

Attachment: managerPatch.patch added

abandoned patch to solve this in ContentPane?

comment:4 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [23492]) Make Form a layout widget so that it can wrap a TabContainer etc. Fixes #12097 !strict.

comment:5 Changed 8 years ago by ebengtso

Resolution: fixed
Status: closedreopened

I upgraded dojo 1.5 to 1.6 and this breaks the display of containers inside the form

None of these works:

<div contentpane>

<div form>

<div contentpane>

<div BorderContainer?>

<div ContentPane?> </div>

</div>

</div>

</div>

</div>

or

<div contentpane>

<div contentpane>

<div BorderContainer?>

<div form>

<div ContentPane?> </div>

</div>

</div>

</div>

</div>

or

<div contentpane>

<div contentpane>

<div form>

<div tabcontainer>

<div ContentPane?>

<input/>

</div> <div ContentPane?>

<input/>

</div>

</div>

</div>

</div>

</div>

If I remove the dijit.form.Form, it gets displayed correctly.

comment:6 Changed 8 years ago by ebengtso

workaround:

dojo.addOnLoad(function() {

new dijit.form.Form({encType: 'application/x-www-form-urlencoded'},'myForm');

});

<div id="myForm"/>

creating the form programmatically on load renders the page correctly.

comment:7 Changed 8 years ago by bill

OK, can you attach a runnable test case using the "attach file" button? Thanks.

comment:8 Changed 8 years ago by ebengtso

Attached the test case. remove the attribute dojoType="dijit.form.Form" and the page gets displayed.

Thx

comment:9 Changed 8 years ago by bill

Ah, it's just that you have the region="center" on the wrong node, since region goes on children of a BorderContainer you need to put it on the dijit.form.Form, not it's child.

After that it works for you, right?

comment:10 Changed 8 years ago by ebengtso

Hi Bill, thx for that, the test case was invalid, I've fixed it, and the issue is reproduced

comment:11 Changed 8 years ago by bill

This test case is also invalid, for one thing it has a LayoutContainer with a plain DOMNode child rather than a layout widget child:

<div dojoType="dijit.layout.LayoutContainer" region="center">
    <div>

Not sure if there are other problems too.

There's also an unnecessary ContentPane wrapper for the form:

<div dojoType="dijit.layout.ContentPane" layoutAlign="client" style="margin-top:0px; padding-top:0px; ">
  <div id="myForm" dojoType="dijit.form.Form" >

That could simply be:

<div dojoType="dijit.form.Form" layoutAlign="client" style="margin-top:0px; padding-top:0px; ">

Not an error per-se, but just unnecessarily complex.

Changed 8 years ago by ebengtso

Attachment: testform.html added

removed unecessary div

comment:12 Changed 8 years ago by ebengtso

I removed the unecessary DIV.

The bug is the one you pointed out: the Form inside the ContentPane?.

<div dojoType="dijit.layout.ContentPane" layoutAlign="client" style="margin-top:0px; padding-top:0px; ">
  <div id="myForm" dojoType="dijit.form.Form" >

If I remove that node it fixes the issue. My problem is that I'm using a framework/container that embeds my Form within the ContentPane?, thus I provide the Form element and all parent nodes are provided by the framework.

As you said, not an error per-se, so I propose to leave this issue open.

comment:13 Changed 8 years ago by bill

Ah OK, I see how the TabContainer with one tab labeled "Flow" doesn't show up correctly unless I comment out the dijit.form.Form widget start and end <div>. I'll take a look.

By the way your test is missing an ending </div> or two, but that's not what's causing the problem.

Changed 8 years ago by bill

Attachment: simple.html added

stripped down testform.html (still showing rendering problem)

comment:14 Changed 8 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [23570]) Make sure that dijit.form.Form passes startup() call down to it's children. Fixes #12097 !strict.

comment:15 Changed 8 years ago by bill

(In [23902]) Before [23882], ContentPane would only start it's top-level widgets, so it was necessary for Form to start it's own children. That's no longer necessary, and it's also inconsistent for _ContentPaneResizeMixin to start it's children even though it doesn't define isContainer: true.

There's also a regression in the Form_state.html robot test, appearing from [23882]. The issue was whether _FormMixin.startup() runs before or after the Form's children are started. The test_Form_state.html test depends on _FormMixin.startup() running first, so that by the time submitButton.startup() runs, it can call Form.isValid() and get the right answer. This changeset thus rolls back [23570] (except for the added tests, etc.), fixing those two issues.

Refs #12097 !strict.

comment:16 Changed 8 years ago by bill

(In [23908]) On second thought, make Form.startup() start children. It's consistent for widgets supporting resize() to also support startup(), and also useful for apps that don't use the parser.

Refs #12097 !strict.

comment:17 Changed 8 years ago by bill

(In [23961]) Fixes for regressions when dijit.form.Form is a top-level widget (i.e. not a child of a layout container) yet contains layout children.

Fixes #12372, refs #12097 !strict.

Fixes in trunk/ and on 1.6/ branch.

comment:18 Changed 8 years ago by bill

(In [23962]) Fixes for regressions when dijit.form.Form is a top-level widget (i.e. not a child of a layout container) yet contains layout children.

Fixes #12372, refs #12097 !strict.

Fixes in trunk/ and on 1.6/ branch.

comment:19 Changed 8 years ago by bill

(In [23963]) Fixes for regressions when dijit.form.Form is a top-level widget (i.e. not a child of a layout container) yet contains layout children.

Fixes #12372, refs #12097 !strict.

Fixes in 1.6/ branch (previous two checkins partially failed).

Note: See TracTickets for help on using tickets.