Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image Gallery Bounces #1449

Closed
bhsmither opened this issue Jan 10, 2017 · 60 comments
Closed

Image Gallery Bounces #1449

bhsmither opened this issue Jan 10, 2017 · 60 comments
Assignees
Labels
Milestone

Comments

@bhsmither
Copy link
Contributor

Due to bad/ignorant planning, product images may not all be large enough such that when the GD class makes the various smaller sizes, everything ends up equal. A small image will not get rendered to a larger derivative.

Therefore, when hovering over a collection of images in the Gallery, the main image area changes dimensions (namely, height). The Gallery naturally sits at the bottom boundary of this area, and when the height changes, so does the position of the Gallery.

This looks sloppy.

The following is an inelegant solution, but contains the essentials. Suggest in Foundation's content.product.php, from:

<div class="small-5 medium-7 columns">            
   <a href="#" class="open-clearing" data-thumb-index="0"><img src="{$PRODUCT.medium}" alt="{$PRODUCT.name}" id="img-preview"></a>
   {if $GALLERY}

To:

<div class="small-5 medium-7 columns">
{literal}
<style scoped>
 .square { position: relative; width: 100%; }
 .square:after { content: ""; display: block; padding-bottom: 100%; }
</style>
{/literal}
   <div class="square">
      <a href="#" class="open-clearing" data-thumb-index="0"><img src="{$PRODUCT.medium}"
                         alt="{$PRODUCT.name}" id="img-preview" style="position:absolute;"></a>
   </div>
   {if $GALLERY}
@abrookbanks abrookbanks self-assigned this Jan 12, 2017
@abrookbanks abrookbanks added this to the 6.1.4 milestone Jan 12, 2017
@Dirty-Butter
Copy link

I'm getting a narrow sliver of the large image on a product listing:

element.style {
    max-height: 21px;
    min-height: 21px;
}

<a class="open-clearing" href="#" data-thumb-index="0" style="min-height: 21px; max-height: 21px;">

open-clearingpixelsissue

I'm not sure if this #1449 is the correct place that is causing this change in today's commit.

@bhsmither
Copy link
Contributor Author

I reject this solution.

While it does stop the gallery from bouncing around, it also sets (on page load and at no other time) the height of this tag based on the initial height of the image preview that is showing the first image of the gallery (main image) -- such as 225px.

Getting the preview to show any other image from the gallery (such as having a height of 350px) does not change this height, and could cut off some part of the image.

@abrookbanks
Copy link
Member

I can't think of a better solution unless it finds the height of the largest image somehow or maybe scales the image down.

abrookbanks added a commit that referenced this issue Jan 13, 2017
@abrookbanks
Copy link
Member

How about this then. The image will scale to fit so there is no cutoff and there is a minimum height set of 300px.

Movie: http://static.cubecart.com/issue-1449.mp4

@abrookbanks abrookbanks reopened this Jan 13, 2017
@abrookbanks
Copy link
Member

abrookbanks commented Jan 13, 2017

This needs media queries so that it affects medium and large only.

The fixed min height needs to be dynamic.

@Dirty-Butter
Copy link

Just made the new code changes on our test site. The product image is back showing large complete image - but it's actually bigger than it was on 6.1.3. Was that intended?

500px × 375px (scaled to 598px × 449px)

@bhsmither
Copy link
Contributor Author

I believe the goal is to make available a square area for an image that may be a maximum of 500 pixels high -- this is the 'maximum' for a "medium" image (config.xml). We know the GD library will make a derivative no more than 500 pixels on its longest side.

For the moment, assume that to be the height. We then know we need an area with a maximum height of 500 pixels and should be able to scale. But how to get it?

When the viewport is large, the medium-7 columns div holding preview and gallery given to us by the responsiveness of Foundation is actually 408 pixels wide, and when the viewport is at medium, that div is max at 553 pixels wide (depending on font size) and scales. So, a max 500 pixel wide image is a good size (see config.xml).

The CSS trick is create a new square div that is as wide as it's parent is wide, and as tall as it's parent is wide. That trick is the padding-bottom.

@abrookbanks
Copy link
Member

I personally think a square looks poor.

@Dirty-Butter
Copy link

I have a few images that are longer than the others. With this code in place the full image shows in the thumbnail on category view. But on product view, the bottom part of the image doesn't show unless I click on the image.

@bhsmither
Copy link
Contributor Author

I think the prime cause of this difficulty is that the shop owner does not properly prepare images (not that one would know to do this). We can amuse ourselves with debating "1:1 ratio is bad and golden ratio is good", but the task is to stabilize the area. Using the padding-bottom trick, we can make the area's height 62% of the width (golden* ratio). But this breaks when the image is taller than that. The only guarantee that every image will show in its entirety in the preview area is at 1:1.

@abrookbanks
Copy link
Member

The current solution here works really very well now without image clipping.

@bhsmither
Copy link
Contributor Author

What solution?

@abrookbanks
Copy link
Member

cf0bb4c

The only other option is to put the images down the left side like Amazon does.
screen shot 2017-01-16 at 09 29 18

@havenswift-hosting
Copy link

I like the Amazon layout and have had several people ask for that specific layout on their sites. How about having an option so the store owner can switch between current layout and the "Amazon" layout ?

@Dirty-Butter
Copy link

A choice would be the best of both worlds - I have one set in our collectibles store that has 28 gallery images! The left side would not be practical there.

@Dirty-Butter
Copy link

(cf0bb4c) still clips my longer than usual image. I can certainly live with that, but I was hoping you would come up with something that fixed the jerky issue without bothering the main image.

@abrookbanks
Copy link
Member

(cf0bb4c) still clips my longer than usual image. I can certainly live with that, but I was hoping you would come up with something that fixed the jerky issue without bothering the main image.

It should scale to fit. Have you tried a hard refresh or temp internet file clear?

@Dirty-Butter
Copy link

I cleared everything - initially it shows the clipped image, but if I refresh the page it shows the full length image. Obviously customers would not refresh the page. If it works correctly for you, then it must be something I've changed from stock that's causing it.

@abrookbanks abrookbanks modified the milestones: 6.1.4, 5.1.5 Jan 23, 2017
@abrookbanks
Copy link
Member

Closing for now to see how this gets on in real world tests.

I think it works very well.

@abrookbanks
Copy link
Member

Massive improvement now working great on all viewport sizes.

@Dirty-Butter
Copy link

With new code in place my 28 image listing looks like this:

https://dirtybutterestates.com/vintage-mustard-earthenware-pottery-coffee-set.html

Many will be very pleased with this change!

@havenswift-hosting
Copy link

The scrolling works well until you want to change direction a few times and then it hangs ?

@Dirty-Butter
Copy link

I can't get it to hang. But I'm undecided whether I would actually want this for our site. As tiny as the gallery pics are, I think I prefer the results when someone clicks on an image and then can scroll big photos horizontally. But like I said, this is an extreme number of images. I fell in love with this set and it doesn't bother me at all that it has not sold :)

@havenswift-hosting
Copy link

I agree that the thumbnail images are far too small (currently viewing on an iPad where it is impossible to see any detail at all). The scrolling is stuttering and then hangs consistently for me - maybe it is something to do with small viewport / iPad ?

@Dirty-Butter
Copy link

Dirty-Butter commented Feb 25, 2017

It works for me at 768 width on Responsive View. But I must not have the small code correctly installed, as there is no gallery at all. My normal small product page is way far from stock. Hopefully by the time 6.1.6 is released, this will be a choice - like the Info and Manufacturer codings.

Do you EVER sleep, Ian???

I'm putting my files back to normal now that you've seen the page.

@abrookbanks
Copy link
Member

Reopening to give choice by means of include not setting. Don't forget Foundation is a blueprint skin.

@abrookbanks abrookbanks reopened this Feb 25, 2017
@claudia39
Copy link

Since most of my images are 800px by 800px I put the code back from before any of the changes where made. There just doesn't seem to be enough room on the product page to have the gallery on the left of the main image.

abrookbanks added a commit that referenced this issue Feb 27, 2017
@Dirty-Butter
Copy link

Just tried the new code using vertical on our test store. - with STOCK content.product.php. There is a large empty space under the image when vertical is used, but a gigantic empty space when horizontal is used.

6 1 6spacingimageproduct

@abrookbanks
Copy link
Member

Hmmmm

@abrookbanks abrookbanks reopened this Mar 1, 2017
@Dirty-Butter
Copy link

Dirty-Butter commented Mar 1, 2017

Using Firebug: I get the empty box with the div after clearing-assembled

<div class="clearing-assembled">
<div>
<a class="clearing-close" href="#">×</a>
<div class="visible-img" style="display: none">
<img class="clearing-preload-next" style="display: none" src="data:image/gif;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAACADs%3D" alt="">
<img class="clearing-preload-prev" style="display: none" src="data:image/gif;base64,R0lGODlhAQABAAD/ACwAAAAAAQABAAACADs%3D" alt="">
<div class="carousel">
<ul class="clearing-thumbs small-block-grid-3 medium-block-grid-5 marg-top" data-clearing="">
</div>
</div>
</div>
```

@bhsmither
Copy link
Contributor Author

Has this issue evolved into something more than simply trying to stabilize the gallery's position?

Was there a discussion that I missed about the initial suggestion being disregarded?

@bhsmither
Copy link
Contributor Author

Well, that needs work on 'small'.

@bhsmither
Copy link
Contributor Author

Starting at CC613, suggest:
cubecart.css:

.square { position: relative; width: 100%; }
.square:after { content: ""; display: block; padding-bottom: 100%; }
a.open-clearing #img-preview { max-height: 100%; position: absolute; left: 50%; transform: translate(-50%, 0px); }

content.product.php:

From:
<a href="#" class="open-clearing" data-thumb-index="0"><img src="{$PRODUCT.medium}" alt="{$PRODUCT.name}" id="img-preview"></a>

To:
<div class="square">
<a href="#" class="open-clearing" data-thumb-index="0"><img src="{$PRODUCT.medium}" alt="{$PRODUCT.name}" id="img-preview"></a>
</div>

@Dirty-Butter
Copy link

Dirty-Butter commented Mar 4, 2017

I've been working on creating my edited product page that incorporates the coding offered here. It's my humble opinion that this part of the element.product.horizontal/vertical_gallery.php files would make more logical sense as part of the content.product.php file. Whatever cosmetic div's needing to be applied can easily be used there.

           {*
         Change include below to 'templates/element.product.horizontal_gallery.php'
         for horizontal gallery and vice versa.
      *}
      {include file='templates/element.product.horizontal_gallery.php'}
      {include file='templates/element.product.options.php'}
      {include file='templates/element.product.review_score.php'}
      {include file='templates/element.product.call_to_action.php'}
      <hr>
      <dl class="tabs" data-tab data-options="scroll_to_content:false">

@abrookbanks
Copy link
Member

The whole row has column count differences for vertical/horizontal so that wouldn't work sadly.

@Dirty-Butter
Copy link

That's a shame. I still have the large blank area under the main image. Are you not seeing that with totally stock code?

@abrookbanks
Copy link
Member

abrookbanks commented Mar 6, 2017

Are you not seeing that with totally stock code?

Nowp but I'll double check..

@Dirty-Butter
Copy link

horizontal is doing it very noticeably for me

@Dirty-Butter
Copy link

I commented out this section, which was modified by Bsmither - that took out the big blank (I wasn't having the image bounce issue that originally prompted this thread

.square { position: relative; width: 100%; }
.square:after { content: ""; display: block; padding-bottom: 100%; }
a.open-clearing #img-preview { max-height: 100%; position: absolute; left: 50%; transform: translate(-50%, 0px);
 }

@Dirty-Butter
Copy link

Dirty-Butter commented Mar 6, 2017

Sorry to keep harping on this one - but is there an automatic "no image" default image for the product page with this new coding? I see one on the homepage, but not when I open a test listing where I had played with the image and it's no longer available.

Bsmither found my problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants