search

pedroabreu / ion-gallery

Ionic gallery directive
MIT License
124 stars 61 forks source link

Support resizing of images when adding more on the fly #30

Closed ggoforth closed 8 years ago

ggoforth commented 8 years ago

So, to clarify, If I have only one image in the gallery, it displays full width (as it should). When I add a second, it adds another full width image below the first. Would be nice if when adding that second image it scaled it down and added the second image on the same row.

I'll also note that it works as expected once you have more than one image already in a row.

pedroabreu commented 8 years ago

I need to have a look at the logic again but, as I see it, it should add it to the same row depending on the row size defined.

troyshu commented 8 years ago

fyi, I'm noticing the same behavior that @ggoforth noticed.

troyshu commented 8 years ago

In case anyone's interested, created a PR (https://github.com/pedroabreu/ion-gallery/pull/35) for another use case that avoids this issue: a boolean flag for the developer when he/she needs the gallery thumbnails to be a fixed size (e.g. 3 images per row), regardless of the number of images in the gallery currently. This is currently my use case. @pedroabreu let me know what you think!

This doesn't fully solve the bug @ggoforth has noticed, if the desired behavior is to display the thumbnail full-width if the gallery only has one image, display the thumbnails with 50% width if the gallery has two images, etc. all the way up to the specified "row_size" config option.

An initial investigation suggests that it has to do with how the logic in "getRowSize" works (determines the number of thumbnails per row of the gallery), and how the thumbnails are scaled in the gallery.

Leaving this info here for whoever tries to fix this issue (I probably won't be able to take another look for a while).

ggoforth commented 8 years ago

I'm good with, and actually think I prefer @troyshu suggestion.

troyshu commented 8 years ago

Thanks @ggoforth! @pedroabreu let me know what you think when you have the chance. Maybe this should be the default behavior, since the current behavior isn't as expected?

pedroabreu commented 8 years ago

I've been thinking about this and I think the way it's implemented right now it really doesn't make any sense (To be fair, I don't know why I did it that way) I think the default behaviour should be, defining a row size and keep it like that regardless of the number of images.

Example: If row size is the default one (3) it should render the thumbnails in a 3 image row, even if only one image. If you add another one, it should render both in a 3 image row (and so on). So the row size will always be a fixed size.

As a solution, I propose to keep both solutions but setting this new way of rendering as the default one (following @troyshu PR). Just because it might be already used as is by someone and it probably wouldn't be nice to break it (although I would like to see a use case where it's used)

I'll have a look at the PR today and if so I'll merge it.

Thanks to both btw

troyshu commented 8 years ago

Sounds good!

pedroabreu commented 8 years ago

Fixed with latest PR #39