Attached is a screenshot of a image gallery with a longer description. Because image.module adds a hardcoded "style='height: XXpx' to each "li", some text can appear outside the box of the gallery.
I notice that this is cause by:
$content .= '<li style="height : '.$height .'px">';
I don't think this is needed: you can do a:
ul.galleries li p.last {
clear: left;
}
inside image.css to expand the box of the gallery as needed. Only downside is that the "last modified" text always appears below the image.
Another way of doing this in CSS only is to do:
$content .= '<li style="minheight : '.$height .'px">';
Unfortunately, IE doesn't understand minheight afaik.
I'll create a patch.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | image_gallery_25054.patch | 3.63 KB | drewish |
| #7 | image_8.patch | 1.62 KB | Robrecht Jacques |
| #2 | image-gallery-fixed.png | 47.46 KB | Robrecht Jacques |
| #1 | image_7.patch | 1.33 KB | Robrecht Jacques |
| image-gallery.png | 61.17 KB | Robrecht Jacques |
Comments
Comment #1
Robrecht Jacques commentedPatch attached.
Comment #2
Robrecht Jacques commentedAttached a screenshot of how it looks after the patch is attached (in Firefox).
Comment #3
walkah commentedi'm for sure not a css wiz - and don't like the height : XX hack, for sure. I have no problem with the count, etc being below the image. I like this patch ... but how does IE handle the clear:left (since i'm on the road and don't have access to IE at the moment).
Comment #4
Robrecht Jacques commentedIt will come to no surprise that IE doesn't handle it well :-( It seems to understand "clear: left" ok and the size of the box is ok too (was already ok, it seems IE interprets "height: Xpx" as "minheight: Xpx" ??), but for some strange reason it doesn't always display the text next to the image. IE knows the text is there, because when you move your mouse over it to select the text, it then displays it ok. Also when you resize your window it suddenly displays it... or sometimes it doesn't :-( Argh!
I will look into this again later (after the weekend), trying to find a suitable CSS solution that works in IE and makes the box big enough if there is too much text in Opera and Firefox.
Comment #5
walkah commentedthanks for your work on this robert... i'll look again at it again as well when i've got a more complete set of browsers to throw at it.
Comment #6
Bèr Kessels commentedI personally vote for completely removing the height. It creates more trouble then it solves. And in the end, this really is a Theme/style issue. which the hardcoded height in the HTML makes rather impossible. For inside-HTML-Style is one of the the last levels in cascading CSS. In other words: if you have CSS in your HTML, you can only override it in your theme by "hacking" teh styles with !imprtant, which is not advicesd for it raise accessability issues.
So, in short, just remove the height completely, and let themes fiddle with the height, if they want to.
Comment #7
Robrecht Jacques commentedBèr, I agree that this is a theming issue, but at least the default "theme_image_gallery" should provide a good default, workable in all browsers. I think we all agree that the "style='height=XXpx'" shouldn't belong inside the HTML code.
Now, my patch with "clear:right" on p.count/last didn't work in IE, but is actually how this thing should be done. Just like Firefox and Opera implement it. I did some searching and it seems there is a bug (called "Peek-A-Boo") in IE that hides the text next to a float in some situations. See: http://www.positioniseverything.net/explorer/peekaboo.html (which I found reading and re-reading http://css.maxdesign.com.au/floatutorial/ -- hint: look at the "floats and clear" page).
This new patch implements a work-around for IE by positioning the "li" and floated "img" relative.
With this:
Comment #8
Robrecht Jacques commented:-( Guess what happens when you don't try "preview" first.
What I wanted to say:
Bèr, I agree that this is a theming function, but at least the default implementation of "theme_image_gallery" should be viewable in all browsers. I think we can all agree that this
style="height:Xpx"has to go.My patch was "right", but there is a bug in IE6 that hides the text next to a float in some cases. See: http://www.positioniseverything.net/explorer/peekaboo.html.
My new patch (comment #7) will use the "relative positioning" work-around from that page so it works in IE, Firefox and Opera. And the box expands to include all text what was my initial bug-report.
If theme developers want to override the CSS behaviour, they can. Or they can reimplement the "theme_image_gallery" function.
(previewing... looks ok... submit)
Comment #9
Bèr Kessels commentedSorry for the misunderstanding. your patch is exactly what I meant ! Off course we need good defaults, but these defaults should be overridable, what your patch nitroduces, or even fixes!
big +1
Comment #10
Robrecht Jacques commentedComment #11
azzegai commentedIs there a reason why nothing has been done about this bug yet? There are at least three other bugs that have been filed since this one all relating to this problem.
Comment #12
peterthevicar commentedThanks Robrecht. I applied the patch at #7 by hand to image_gallery.module and it was nearly there. I found I also had to comment out the line
279 $content .= ' style="height : '.$height .'px; width : '.$width.'px;"';
This one is a definite +1 for me too.
ATB, Peter
Comment #13
drewish commentedpeterthevicar, how about posting an updated patch rather than just describing what needs to be done? it sure doesn't look like it'd apply to HEAD any longer.
Comment #14
drewish commentedokay here's a re-roll for HEAD
Comment #15
drewish commentedi've marked the issues azzegai referenced as duplicates of this.
Comment #16
drewish commentedmarked http://drupal.org/node/110275 as a duplicate
Comment #17
drewish commentedi've committed this to HEAD and 5.
Comment #18
(not verified) commentedComment #19
Marko B commentedthink the version label is wrong in this post?! this couldnt be ver 6 issue in 2005?