pedroabreu / ion-gallery

Ionic gallery directive
MIT License
124 stars 61 forks source link

ion-gallery does not work with ionic 1.2.0/1.2.1 #27

Closed SebastianSchirmer closed 8 years ago

SebastianSchirmer commented 8 years ago

Hi there!

First of all, thanks very much for your effort and work, great stuff!

About a week ago, Ionic 1.2.0 got released and we noticed that ion-gallery does not work with this release.

The regexp match in line https://github.com/pedroabreu/ion-gallery/blob/master/dist/ion-gallery.js#L275:

var match = element[0].getElementsByClassName('scroll')[0].style.webkitTransform.match(/scale\(([^)]+)\)/);

is always null, therefore

match[1]

throws an error.

Could it be that the regexp must rather match 'matrix' instead of 'scale'?

In addition to that error, pinch & zoom does not work anymore when using the Ionic 1.2.0/1.2.1 release version. Currently I have no clue why that is.

Any feedback is very much appreciated.

SebastianSchirmer commented 8 years ago

As Ionic 1.2.0 uses native scrolling by default, overflow-scroll must be set to "false" in the directive in the slider template. Doing so, everything works fine.

katche70 commented 8 years ago

Where to set? i added

<ion-content overflow-scroll="false">

to slider.html but no success

SebastianSchirmer commented 8 years ago

It must be set to the directive in slider.html: https://github.com/pedroabreu/ion-gallery/blob/master/src/templates/slider.html#L16

katche70 commented 8 years ago

uhh, don't know what i'm maikig wrong

i changed $templateCache in ion.gallery.js but no success

$templateCache.put("slider.html","<ion-modal-view class=\"imageView modal-slide\" ng-class=\"hide.bars ? 'content-dark' : 'content-light'\" >\n  <ion-header-bar class=\"bar-energized animate galeryviewheader headerView\" ng-show=\"!hideAll\">\n   </button> <button class=\"button button-icon ion-android-arrow-back closemodal-btn\" ng-click=\"closeModal()\"></button>\n  </ion-header-bar>\n  <ion-content overflow-scroll=\"false\"  class=\"has-no-header\" scroll=\"false\">\n  \n <ion-slide-box does-continue=\"true\" active-slide=\"selectedSlide\" show-pager=\"false\"  on-slide-changed=\"slideChanged($index)\">\n      <ion-slide ng-repeat=\"single in slides track by $index\">\n        <ion-scroll class=\"gallery-slide-view\" direction=\"xy\"\n                    locking=\"false\" \n                    zooming=\"true\"\n                    min-zoom=\"1\"\n                    scrollbar-x=\"false\"\n                    scrollbar-y=\"false\"\n           overflow-scroll=\"false\"           ion-slide-action\n                    delegate-handle=\"slide-{{$index}}\"\n                    >\n                 <img adaptive ng-src=\"{{single.src}}\">\n        </div>\n        <div ng-if=\"single.sub.length > 0\" class=\"image-subtitle\" ng-show=\"!hideAll\">\n            <span  ng-click=\"editImageDes($event)\" ng-bind-html=\'single.sub\'></span>\n    \n    </div>    </ion-scroll>\n      </ion-slide>\n    </ion-slide-box>\n  </ion-content>\n <div ng-show=\"!hideAll\" class='bar bar-footer animate'> <button class=\"button pull-right button-icon    ion-android-delete \" ng-click=\"delImages()\"></div> \n</ion-modal-view>");}]);

https://github.com/pedroabreu/ion-gallery/blob/master/dist/ion-gallery.js#L480

SebastianSchirmer commented 8 years ago

Should work like that... hmm

pedroabreu commented 8 years ago

I'm running Ionic, v1.2.0-nightly-1823 and only get the error when I try to zoom. Double tap to zoom doesn't work as well.

Setting the overflow-scroll to false won't have any effect on this (At least with the version I'm on)

I'll continue to investigate this

EDIT: From the quick look I had, The zoom property is no longer set in that CSS property. Funny that I can't find it anywhere else... And I just noticed that the current slider will be depreciated, being replaced by swyper. Also need to add support for that (Or keep this as a fallback for older ionic version for now)

pedroabreu commented 8 years ago

Fixed with the latest commit. Will need a small code refactor in the future... Zoom feature is a bit confusing

Thanks @leschirmeur for spotting the issue and pointing a solution