Opened 7 years ago

Closed 4 years ago

#15606 closed defect (patchwelcome)

[ccla] dijit/_HasDropDown - with maxHeight -1, properly calculate height post-display

Reported by: Brian Arnold Owned by: bill
Priority: high Milestone: 1.13
Component: Dijit Version: 1.7.3
Keywords: Cc:
Blocked By: Blocking:

Description

This work is covered under both SitePen?'s CCLA and my personal CLA.

We have a simple example where some style is being added to the dijitMenuPopup class that adds additional height to the node, but the maximum height is being calculated based on the size of the node before those classes are in play.

There is also a note that says the following in _HasDropDown.js:

			// TODO: isn't maxHeight dependent on the return value from dijit.popup.open(),
			// ie, dependent on how much space is available (BK)

This patch looks to resolve that TODO. It checks if maxHeight is in play, and if a resize of height was performed before (as there are scenarios where the dropdown isn't overheight and so the height resize isn't necessary) and then it recalculates the size.

It's a little frustrating that it does have potential to cause two resizes in a row, but the classes in play aren't actually on the wrapping nodes until the menu is opened, so it seems that there's not much choice.

I'm attaching a reduced test case that demonstrates the issue (use a 1024x768 browser size or smaller to see it) as well as a patch. This patch is written against the 1.7 branch, but looking at trunk, it should be able to easily be applied to both the 1.7 branch as well as trunk.

Attachments (2)

maxHeight-miscalculation.html (5.0 KB) - added by Brian Arnold 7 years ago.
A reduced test case that demonstrates the problem against the 1.7 branch
_HasDropDown-maxHeight.patch (891 bytes) - added by Brian Arnold 7 years ago.
A patch to add recalculation to height

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Brian Arnold

A reduced test case that demonstrates the problem against the 1.7 branch

Changed 7 years ago by Brian Arnold

A patch to add recalculation to height

comment:1 Changed 7 years ago by Brian Arnold

Summary: dijit/_HasDropDown - with maxHeight -1, properly calculate height post-display[ccla] dijit/_HasDropDown - with maxHeight -1, properly calculate height post-display

comment:2 Changed 7 years ago by bill

Cc: bill removed
Milestone: 1.7.41.8

I see, you're adding a border etc. on dijitMenuPopup. I wasn't expecting anyone to do that, although admittedly I added some other styling in [21266].

I think what you are doing is technically possible by creating a wrapper widget, but admittedly that's heavy-handed.

comment:3 Changed 7 years ago by bill

Brian, if you can update your patch to change dijit/tests/popup.html to test a tall popup with a border, I'll check this in. (You shouldn't make popup.html depend on high level widgets like dijit/form/Select or dijit/Menu since dijit/popup is infrastructure code.)

comment:4 Changed 7 years ago by bill

Milestone: 1.82.0

comment:5 Changed 7 years ago by bill

Priority: undecidedhigh

comment:6 Changed 7 years ago by bill

After my changes in [31001], this is a different ball of wax.

comment:7 Changed 4 years ago by dylan

Milestone: 2.01.12
Resolution: patchwelcome
Status: newclosed

Given that no one has shown interest in creating a patch in the past 3+ years, I'm closing this as patchwelcome.

Note: See TracTickets for help on using tickets.