Ran into a bit of a doozy getting a correct height in place as we have a responsive site where the images are going to be proportionally scaled down (with CSS, not an adaptive image technique, etc.) to fit on mobile screens. This is sort of connected to #1053688: Caption shows for only some slides

However, because the container (ie class="field-slideshow") has the width and height set inline based on the image file size (ie $variables['slides_max_width']), then if the image size changes all sorts of funkiness occurs (captions cut-off, image size jumping as slides move, etc.).

I've written a first attempt at a patch, which pulls the height out of the template file (would need to be pulled out of the .module file as well, but wasn't sure the right approach so skipped that for now), and then adds height into the JS. I'm guessing there are other cases this may break things for, but thought at least I'd get something rolling.

In the meantime, for those who want a fix without this patch here is what I did:

  1. Copied field_slideshow.tpl.php into my theme templates and removed the height there (line 11)
  2. Added the behavior below to my theme JS file to calculate and set the height (note it is currently only written for a single field slideshow per page, but could be easily extended - just model on the field_slideshow.js code)
    // Set field slideshow container height
    // We need explicit heights for Field Slideshow to correctly size,
    // so need to adjust based on the height of the image (223px on mobile, 345px all else)
    // as well as the caption height (which the Field Slideshow modules factor in with padding-bottom)
    Drupal.behaviors.mobileFPAutoImgSizeFix = {
        attach: function (context) {
            var max_height = 0;
            $('.field-slideshow .field-slideshow-slide').each(function() {
                $this = $(this);
                max_height = Math.max(max_height, $this.outerHeight(true));
            });
            var paddingBottom = parseInt($('.field-slideshow').css("padding-bottom"));
            $('.field-slideshow').height(max_height - paddingBottom);
        }
    };

Ultimately, it would be ideal if an image could just have width and height set to auto, but I know with the position absolute it makes that really tricky (impossible?), so we probably just need an alternative in the meantime.

CommentFileSizeAuthor
0002-Auto-generating-slideshow-container-height-instead-o.patch2.1 KBsokrplare

Comments

futurist’s picture

futurist commented

I had a similar issue where the caption was not displaying properly. The image style I am using in the slideshow uses the "Aspect switcher" effect which is provided by the ImageCache Actions module. Which means that there are actually two image styles used in the slideshow, one for landscape images and one for portrait (although derivative images are the same height from both).

The captions appeared only irregularly, and I found that it was because the slideshow container height was too small, so the captions were cut off. I had to switch to the Views Slideshow module for the time being, but I would definitely prefer if there was a solution with Field Slideshow because it addresses this use case (single node, multi-value image field) much better.

idflood’s picture

idflood commented

Thanks for reporting the issue and the patch, definitely a good starting point. Fieldslideshow needs to be "responsive compatible" so I will try to look at this as soon as possible.

idflood’s picture

idflood commented

Here is a related issue: #1228266: Responsive slideshow

idflood’s picture

idflood commented

Could you try to reproduce with the 1.7 version (make a backup of the db/file before)? The "responsive slideshow" patch has certainly fixed your issue.

idflood’s picture

idflood commented
Status: Active » Fixed

Marking this as fixed since the 1.7 or 1.8 version should fix the issue.

Feel free to reopen it if it's not the case.

aliyayasir’s picture

aliyayasir commented

Hi idflood,
i am using 1.8 and still found this issue

idflood’s picture

idflood commented
Version: 7.x-1.6 » 7.x-1.x-dev
Status: Fixed » Active
idflood’s picture

idflood commented
Status: Active » Closed (duplicate)

Marking this as duplicate of #1858602: Element heights calculated differently

I've posted a patch on the related issue. It's not the exact same issue but I'm confident that the patch should fix this one. It will certainly be added to a new 2.x branch of field_slideshow since it change a little the markup and css.

idflood’s picture

idflood commented
Issue summary: View changes

Touching up syntax

kopeboy’s picture

kopeboy
Primary language Italian
Location Milan
commented
Version: 7.x-1.x-dev » 7.x-2.0-beta1
Priority: Normal » Major
Status: Closed (duplicate) » Needs review

I'm sorry but you closed the linked issue as well...

Can we get this patch in please?

Still having the problem of height (too big when you have bigger images, which will send the pagers way too down) with 2.0-beta.

Status: Needs review » Needs work

The last submitted patch, 0002-Auto-generating-slideshow-container-height-instead-o.patch, failed testing.

ajits’s picture

ajits
he/him
Primary language Marathi
Location India
commented
Version: 7.x-2.0-beta1 » 7.x-2.x-dev

The patch seems to be not applicable. Could you re-roll it against 7.x-2.x so that others can review?

Agiss’s picture

Agiss commented

I solved this with CSS.

I want the slider to take the whole width of the browser window so, I give the .field-slideshow element the full width of the viewport- width: 100vw;.

I also want the slider to have a height value equal to my image height, at any time the window resizes, without having to reload the page.

My image's dimensions are 1920px width x 1080px height. I calculate the percentage difference of the image dimensions, using this formula- (width-height)/width.

In my case (1920-1080)/1920 gives me a 0.4375. I then take this number to calculate the height of the .field-slideshow element using the calc CSS function- height: calc(100vw - 43.75vw).

I am also adding the '!important' declaration, to overide any other value.

Here is my CSS

.field-slideshow {
width: 100vw !important;
height: calc(100vw - 44vw) !important;
}
lamp5’s picture

lamp5
Primary language Polish
Location Rzeszow
commented
Status: Needs work » Closed (outdated)