Opened 11 years ago

Closed 9 years ago

#7272 closed enhancement (fixed)

the height of dojox.image.LightboxDialog shut be calculated and not hardcoded

Reported by: mafix Owned by: dante
Priority: high Milestone: 1.6
Component: DojoX Widgets Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by dante)

The height of the Footer of the Lightbox shut be dynamic readed from the style and not added static.

look at: http://bugs.dojotoolkit.org/browser/dojox/trunk/image/Lightbox.js (line: 337)

it's bad to change the javascript source be change the style of the Lightbox

Change History (10)

comment:1 Changed 11 years ago by dante

Description: modified (diff)
Milestone: tbdfuture
Status: newassigned

refs #7266 - the numLines thing might help, but I'd prefer to calculate the size as this ticket suggest. I want to say 1.3, but time may not permit it.

comment:2 Changed 11 years ago by dante

Description: modified (diff)

comment:3 Changed 10 years ago by dante

in [17205] I removed the 22px assumption and am using contentBox to calculate that now. the other part of this would be to add in support for the taller titles, and just calculate the size of the titleNode now too. ideally it would be fluid.

comment:4 Changed 10 years ago by dante

Milestone: future1.4
Resolution: fixed
Status: assignedclosed

titleNode is being calculated in [17213] ... allows multiline and light rich text titles.

comment:5 Changed 9 years ago by minobun

Resolution: fixed
Status: closedreopened

The height is still hardcoded. This patch should help to go remove it. At line 348 (function resizeTo) change this:

		var adjustSize = dojo.boxModel == "border-box" ? 
			dojo._getBorderExtents(this.domNode).w : 0,
			titleSize = forceTitle || { h:30 }
		;

to:

		var adjustSize = dojo.boxModel == "border-box" ? 
			dojo._getBorderExtents(this.domNode).w : 0;
		var titleSize;
		if (forceTitle) {
			titleSize = forceTitle;
		}
		else {
			var nodeMaxHeight = 0;
			for (var i = 0;  i < this.titleNode.children.length;  i++)  {
				var nInfo = dojo.position(this.titleNode.children[i]);
				if (nInfo['h'] > nodeMaxHeight) {
					nodeMaxHeight += nInfo['h'];
				}
			}
			titleSize = {h:nodeMaxHeight};
		} 

comment:6 Changed 9 years ago by minobun

Sorry

this.titleNode.children

is wrong it has to be:

this.titleNode.childNodes

for better compatibility.

comment:7 Changed 9 years ago by dante

Milestone: 1.41.6

comment:8 Changed 9 years ago by dante

Here is an alternate form of your idea:

Index: Lightbox.js
===================================================================
--- Lightbox.js	(revision 21800)
+++ Lightbox.js	(working copy)
@@ -361,12 +361,17 @@
 		
 	},
 
+	_calcTitleSize: function(){
+		var sizes = dojo.map(dojo.query("> *", this.titleNode).position(), function(s){ return s.h; });
+		return { h: Math.max.apply(Math, sizes) };
+	},
+	
 	resizeTo: function(/* Object */size, forceTitle){
 		// summary: Resize our dialog container, and fire _showImage
 		
 		var adjustSize = dojo.boxModel == "border-box" ? 
 			dojo._getBorderExtents(this.domNode).w : 0,
-			titleSize = forceTitle || { h:30 }
+			titleSize = forceTitle || this._calcTitleSize()
 		;
 		
 		this._lastTitleSize = titleSize;
@@ -567,6 +572,7 @@
 	onClick: function(groupData){
 		// summary: a stub function, called with the currently displayed image as the only argument
 	},
+	
 	_onImageClick: function(e){
 		if(e && e.target == this.imgNode){
 			this.onClick(this._lastGroup);

It differs slightly in that yours does a cumulative total of all heights, but only if the node height is larger than the previous cumulative total? Not sure I follow the reasoning there.

comment:9 Changed 9 years ago by minobun

You are right my code above has another little bug. It should

if (nInfo['h'] > nodeMaxHeight) {
	nodeMaxHeight = nInfo['h'];
}

instead of

if (nInfo['h'] > nodeMaxHeight) {
	nodeMaxHeight += nInfo['h'];
}

so it should do the same as your code. But yours seems more clean and dojo like.

comment:10 Changed 9 years ago by dante

Resolution: fixed
Status: reopenedclosed

(In [22508]) fixes #7272 again. reallllly calculate the titleNode.

Note: See TracTickets for help on using tickets.