Last modified: 2011-07-04 21:36:43 UTC
The new gallery layout of 1.17 breaks when you have thumbnails for audio and video. The blocks have a fixed height, and this prevents dynamically loading elements like OggPlayer and mwEmbed from adapting the content and benefitting from auto sizing. Perhaps min-height instead of height ?
Created attachment 8128 [details] screenshot of broken layout
min-heigth sounds good. I'll make a patch
Min-height might cause the inline error messages to stretch a very larger area.... We should probably verify that.
Created attachment 8134 [details] Patch No, the error messages are generated seperately, see ImageGallery,php L257-L272. Here's the patch (had to create a new css rule to avoid regressing to a previous bug)
ehm, mind explaining the css rule ? perhaps even with a comment ? Might be helpful if one done someone figure "what the f is this for".
Created attachment 8135 [details] screenshot of rendering with patch
If the image height is lower than the line-height, the margin-top is applied to the top of the line. a very short image will not follow for 3-4px laters, thus leading to a bigger distance from the top than it should be. vertical-align:text-top moves the picture up, so this problem doesn't happen
Applied in r82181
Trevor said this didn't work on older browsers, that we have to support. Could you test r82215 and see if it works for you?
reop
Created attachment 8152 [details] better patch Nope, this leads to the first screenshot again. Here's a better patch: Remove the css rule again, don't set a fixed height at all, but fix the margin calculation for small images
Created attachment 8153 [details] Better combined patch, fixes 27458 as well
Created attachment 8154 [details] screenshot of rendering with patch2
see r82309
The patch has two problems. First you can't rely on the line-height being 17px, since it's actually 1.5em. Secondly, rounding the margin gives you different div heights for even and odd pixel thumbnails. I've attempted to fix the first problem with r91427 and the second problem with r91426. Could others test and verify that this works for all cases. Thanks!
