Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#16394 closed defect (fixed)

height: 100% broken for children of StackContainer

Reported by: ben hockey Owned by: bill
Priority: undecided Milestone: 1.9
Component: Dijit Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

r29904 introduces an extra div around children of StackContainer?. this breaks previous layouts where the child had styling of height: 100%;. i've attached a test case where this happens.

Attachments (2)

16394.html (1.1 KB) - added by ben hockey 7 years ago.
16394.diff (1.3 KB) - added by ben hockey 7 years ago.

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by ben hockey

Attachment: 16394.html added

comment:1 Changed 7 years ago by ben hockey

applying height: 100%; to that wrapper node fixes this problem. i wonder if that is the correct solution for this.

comment:2 Changed 7 years ago by ben hockey

aargh! height: 100%; doesn't work unless it's only applied to the wrapper of the currently selected child because otherwise the "hidden" wrappers still have height: 100%.

comment:3 Changed 7 years ago by ben hockey

using _hideChild and _showChild we can apply and remove height: 100%; to page._wrapper - this seems to be working for me.

comment:4 Changed 7 years ago by bill

Owner: changed from bill to ben hockey
Status: newpending

I see... yes it requires users to change their custom CSS, but what do you suggest?

comment:5 Changed 7 years ago by bill

PS: why are you setting height: 100% on a StackContainer child. Isn't it StackContainer's job to set the height on its children?

Changed 7 years ago by ben hockey

Attachment: 16394.diff added

comment:6 Changed 7 years ago by ben hockey

Status: pendingnew

Attachment (16394.diff) added by ticket reporter.

comment:7 Changed 7 years ago by ben hockey

Replying to bill:

PS: why are you setting height: 100% on a StackContainer child. Isn't it StackContainer's job to set the height on its children?

i find that CSS does (or did) a pretty good job setting the height ;)

StackContainer? won't set the height if the child does not have a resize method and the CSS did everything i needed - with less code.

anyhow, what i'm suggesting is in the patch i've attached.

comment:8 Changed 7 years ago by ben hockey

Owner: changed from ben hockey to bill
Status: newassigned

comment:9 Changed 7 years ago by bill

OK. Perhaps StackContainer should be showing/hiding the wrapper node, instead of the child widget. In that case you could keep your old CSS.

comment:10 Changed 7 years ago by ben hockey

i had thought of it showing and hiding the wrapper but that still won't do it (unless you also somehow set the height of the wrapper to 100%) because the wrapper needs to expand to the height of the container so that the domNode of the child widget can expand to the same height.

comment:11 Changed 7 years ago by bill

Well, you could set the height of the wrapper to 100% yourself, in CSS, similar to what you've been doing all along. My point was that the wrapper's height doesn't need to be changed dynamically; it can stay at 100% whether or not the pane is selected.

comment:12 Changed 7 years ago by ben hockey

yes - that would probably be good enough.

comment:13 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [30088]:

Hide and show StackContainer's child's wrapper, rather than child itself, to allow app to do CSS styling of children and their wrappers to height: 100%.

Also fixing setting of aria-label. It shouldn't be set on the node unless there's a label defined (otherwise you end up with aria-label="undefined", and also need to worry about escaping special characters.

Fixes #16394, refs #16242, #16244 !strict

comment:13 Changed 7 years ago by bill

In [30132]:

fix test failure caused by [30088], refs #16394

comment:14 Changed 7 years ago by bill

In [30136]:

fix test failure caused by [30088], refs #16394

comment:15 Changed 6 years ago by bill

Milestone: tbd1.9
Note: See TracTickets for help on using tickets.