Flow Theme Zoom Bilder BUG

Das klappt sehr gut, vielen Dank - gleich mal integriert.

Was mir aber noch aufgefallen ist: Auf dem Smartphone kann man bei > 5 Bildern gar keine Gallery mehr sehen. Nachvollziehbar hier:

Einfach auf dem Smartphone mal die einzige Variante auswählen. Dann verschwindet die Gallery komplett.

Gerne.

Zum Fix: Es wurde noch eine Datei verändert bzw. hinzugefügt, damit bei Klick auf die Pfeile nicht ständig die Hauptbilder verschwinden.

Bei Gelegenheit schaue ich mir das auch noch an.

Klasse, habe mir die aktuelle Version gezogen. Das mit dem Bild-Verschwinden beim Klick auf den Pfeil ist mir auch aufgefallen, wollte aber eines nach dem anderen angehen. :smiley:

Danke!

Kommando zurück! Es funktioniert leider nicht vollständig.

Blöderweise habe ich nur mit einem klein gezogenen Fenster getestet, und nicht mit einem echten Mobile Device oder halt in den Entwicklertools mit einem entsprechenden Emulator.

Mein Fehler, sorry.

Zu den Problemen:

[QUOTE=Phillinger;187802]Ja, schon seit November letzten Jahres:

https://bugs.oxid-esales.com/view.php?id=6554
und
https://bugs.oxid-esales.com/view.php?id=6552

Auch auf GitHub existiert ein Pull Request:

Seitdem tut sich irgendwie wenig bis nix. Wir haben jetzt die 4.10.4 und es ist immer noch broken. :frowning:

(Wo meldet man denn jetzt eigentlich Bugs? Als Issue in GitHub oder doch weiter über den Bugtracker?)[/QUOTE]

[U]Bug 6552[/U]:
[I](Seite nach Schließen des Zooms nicht mehr benutzbar, nur neu laden hilft)[/I]
Mit dem Fix von vanilla_thunder ist das behoben: Der Zoom schließt ordentlich, die Seite bleibt benutzbar. (Siehe auch Kommentar im Ticket)

[U]Bug 6554[/U]:
[I](Gallery broken wenn > 4 Bilder)[/I]
Auf mobile devices:

Hier schmeißt die Console in den Entwicklertools (z.B. Chrome) folgenden Fehler, wenn man z.B. ein Nexus 5X emuliert:


Uncaught ReferenceError: Carousel is not defined
    at HTMLDocument.<anonymous> (details.min.js?1498765689:1)
    at k (jquery.min.js?1498765690:2)
    at Object.fireWith [as resolveWith] (jquery.min.js?1498765690:2)
    at Function.ready (jquery.min.js?1498765690:2)
    at HTMLDocument.g (jquery.min.js?1498765690:1)

/edit: Das passiert auch bei Artikeln mit < 4 Bildern, nur fällt es da halt nicht auf, weil das Carousel nicht benötigt wird.

Auf dem Desktop sieht wirft er dann den im Bug-Eintrag geworfenen Fehler.


details.min.js?1498765689:1 Uncaught TypeError: c.flexslider is not a function
    at Object.Flow.initDetailsEvents (details.min.js?1498765689:1)
    at HTMLDocument.eval (eval at globalEval (jquery.min.js?1498765690:1), <anonymous>:2:71)
    at k (jquery.min.js?1498765690:2)
    at Object.add [as done] (jquery.min.js?1498765690:2)
    at _.fn.init._.fn.ready (jquery.min.js?1498765690:2)
    at eval (eval at globalEval (jquery.min.js?1498765690:1), <anonymous>:2:15)
    at eval (<anonymous>)
    at Function.globalEval (jquery.min.js?1498765690:1)
    at _.fn.init.domManip (jquery.min.js?1498765690:2)
    at _.fn.init.append (jquery.min.js?1498765690:2)

Auf dem Desktop kann man sich hier mit diesem Fix von le_shatai behelfen. Einfach die morepics.tpl entsprechend ändern, dann läuft das auf dem Desktop fehlerfrei.

[B]Aber weiterhin bleibt der Carousel-Fehler auf den mobile devices.[/B]

Getestet habe ich das auf einem frisch aufgezogenen CE 4.10.4 und nur vanilla_thunders und le_shatai Bugfixes. Kann das jemand hier nachvollziehen?

So, das waren jetzt einige Edits, aber jetzt sollte es passen.

mein Fix dürfte bei 6554 eigentlich nicht helfen, da dort der Flexslider erst gar nicht geladen wird.
als ergänzung zum Fix von le_shatai:
benötigt werden nur diese beiden Zeilen:


aus morepics.tpl und verschoben werden sollten sie nach productmain.tpl, etwa in die Zeile 86:

und der Code drum herum

[{if $iMorePics > 4}]
...
[{/if}]

muss weg, damit der Flexslider für mobile Devices immer geladen wird, auch < 4 Bildern

[QUOTE=vanilla thunder;188132]mein Fix dürfte bei 6554 eigentlich nicht helfen, da dort der Flexslider erst gar nicht geladen wird.[/QUOTE]
Korrekt, habe ich in meinem Posting entsprechend korrigiert (siehe edits). Danke!

[QUOTE=vanilla thunder;188132]
als ergänzung zum Fix von le_shatai:
benötigt werden nur diese beiden Zeilen:


aus morepics.tpl und verschoben werden sollten sie nach productmain.tpl, etwa in die Zeile 86:

und der Code drum herum

[{if $iMorePics > 4}]
...
[{/if}]

muss weg, damit der Flexslider für mobile Devices immer geladen wird, auch < 4 Bildern[/QUOTE]
Ich habe das einfach dorthin kopiert, natürlich ohne die if-Abfrage, sodass es immer geladen wird.

Dann funktioniert alles - wie gesagt - aber nur auf dem Desktop. Auf mobile fehlt ihm die Carousel-Klasse.

[QUOTE=vanilla thunder;188132]

[{if $iMorePics > 4}]
...
[{/if}]

[/QUOTE]

Das ist auch im Fix, also dem anderen :), drin bzw. nicht mehr drin.

Moinsen,
auch ich muss mich unbedingt hier mal wieder einklinken, da ich es zum einen auch nicht besonders erbaulich finde, das dieser Bug noch immer nicht final behoben wurde. Zum anderen versuche ich selbst seit ca. 2 Tagen, es zumindest zu verstehen, aber das Ganze ist eine völlig krude Sache, die immer verwirrender wird. Bisher stochern alle nur ein wenig im Heuhaufen herum, scheint mir (ich natürlich eingeschlossen) und niemand weiß so richtig, wo der Hund begraben liegt. Um diese These zu untermauern, schaut euch mal folgendes an (OXID 4.10.4):


Dort klappt es nun nämlich seit kurzem, evtl. aber auch schon länger, da kam ich leider irgendwann durcheinander. Also habe ich mal den Quellcode vergleichen mit meinem eigentlich identischen Test-Shop und kam nun auf folgendes:
Sobald OXID im Demo-Mode läuft (config.inc.php: $this->blDemoShop = 1) UND die Tags aktiviert sind (Admin-Grundeinstellungen) und nur dann(!) funktioniert der Photoswipe-Zoom perfekt und zwar OHNE jeglichen Patch (schaltet mal die Tags im Demo-Admin aus, dann seht ihr, was ich meine)! Und genau darauf will ich hinaus: eure bisherigen Mühen absolut in Ehren, keine Frage! Aber ich halte es für den falschen Ansatz, zu versuchen, externe JS-Bibliotheken zu patchen, denn es gibt einen eindeutigen Beweis, dass dies nicht unbedingt nötig ist! Oftmals holt man sich damit nämlich wieder neue Probleme ins Boot, wie man bei allen bisherigen Versuchen ja merken konnte. Allerdings ist es auch irre mühsam, alle erdenklichen Varianten in allen möglichen Browser durchzutesten, dass lässt sich ja leider nicht automatisieren… :frowning:
Wenn man diesen Quellcode-Vergleich jedenfalls auf die Spitze treibt, kann man folgendes schaffen, und spätestens hier steige ich aus, denn das widerspricht jeder Logik: ich habe es geschafft (durch Auskommentieren der Tags-JS-Includes), zwei HTML-Quelltexte zu bekommen, die bis auf einige Leerzeilen (durch diese Auskommentierung) absolut identisch sind, und trotzdem funktioniert der Zoom mit dem einen, mit dem anderen aber nicht, und zwar reproduzierbar im FF! Weiterhin sind alle Cookies und die Session identisch, wo kann den jetzt bitte schön noch ein Unterscheid sein, der dieses Browserverhalten erklärt??? Da wird man ja gaga… :eek:

So, ein weiterer Versuch, das Ganze mal in eine andere (und hoffentlich erfolgreichere als bisher) Richtung zu lenken, und ich will hier echt keinem auf den Schlipps treten: danke nochmal für alle bisherigen Mühen + Viele Grüße!

PS: ups, die letzten 3-4 Posts habe ich noch nicht berücksichtigt, ahne aber nichts gutes… :wink:
Außerdem geht es hier wohl auch um mehrere verschiedene Bugs, bei den Mobile-Problemen bin ich noch nicht angelangt. Und ich habe für die Detailseite noch einen weiteren FF-Bug in Petto, der noch nie erwähnt wurde. Dazu aber mehr an anderer Stelle. Ich würde nur erstmal so gerne wissen: was haben die Tags mit dem Zoom zu tun? Und noch etwas: im Prinzip wird duch den Demo-Mode die Einbindung der Tags-JS-Includes wieder unterbunden, aber es reicht eben NICHT aus, einfach stattdessen diese Auszukommentieren, wie oben beschrieben. Das ist total schräg!

achsooo, ich war bei der " c.flexslider is not a function" Fehlermeldung.
Ich habe keine Ahnung, was dieses Carousel sein soll. Könnte bootstrap carousel sein.
ich würde einfach den ganzen mobile carousel Quatsch rausnehmen:

Außerdem hier mal ein Patch für die morepics.tpl wg. der Sache auf Seite 5 dieses Threads, bis jmd. das Ganze mal auslagert in eine Core-Klasse, wie auch im Kommentar der productmain.tpl zitiert:

[{* ToDo: This logical part should be ported into a core function. *}]

Ich habe den Code nur von dort kopiert und um den Bildindex ergänzt. Statt:

[{assign var="aPictureInfo" value=$oPictureProduct->getMasterZoomPictureUrl($iPicNr)|@getimagesize}]

kann man dieses nehmen:

[{if $oConfig->getConfigParam('sAltImageUrl') || $oConfig->getConfigParam('sSSLAltImageUrl')}]
                    [{assign var="aPictureInfo" value=$oPictureProduct->getMasterZoomPictureUrl($iPicNr)|@getimagesize}]
                [{else}]
                    [{assign var="sPic" value="oxarticles__oxpic"|cat:$iPicNr}]
                    [{assign var="sPictureName" value=$oPictureProduct->$sPic->value}]
                    [{assign var="aPictureInfo" value=$oConfig->getMasterPicturePath("product/`$iPicNr`/`$sPictureName`")|@getimagesize}]
                [{/if}]

Leider funktioniert dies aber wiederum nicht im Demo-Mode (siehe Zoom-Bug oben), nur zur Warnung… :wink:
(oder wie schafft man es, das erste und zweite “assign” im {else} so zu vereinen, dass es keine smarty-secure-Warnung gibt?

[QUOTE=vanilla thunder;188136]achsooo, ich war bei der " c.flexslider is not a function" Fehlermeldung.
Ich habe keine Ahnung, was dieses Carousel sein soll. Könnte bootstrap carousel sein.
ich würde einfach den ganzen mobile carousel Quatsch rausnehmen:

Dann verschwindet auf den mobiles zwar die Fehlermeldung, aber es wird gar keine Gallery mehr geladen, d.h. man sieht nur noch Bild 1.

Ich werd’ noch irre. Mitmacher hat da schon recht, zum Verrückt werden. :smiley:

Für mich sieht es gar nicht so eindeutig aus.
In der Console gibts einen Javascript Fehler, weil im Demo Modul, warum auch immer, oxtags.min.js nicht geladen wird, also bricht an dieser Stelle die Ausführung von Javascript ab.
Der Photoswipe Bug ist dadurch bedingt, dass Photoswipe Javascript beim Laden der Varianten nochmal ausgeführt wird, wozu es im Demo-Modus durch den erwähnten Fehler eben nicht mehr kommt.

Oder anders gesagt, für Nicht-Programmierer: Seinem Kind die Beine zu brechen verhindert zwar, dass er später ein schlechter Tänzer wird, aber nur weil er gar kein Tänzer mehr werden kann.

Okay, mit der “Eindeutigkeit” irgendwelcher Dinge wollte ich mich auch gar nicht allzu weit aus dem Fenster lehnen, aber reden wir trotzdem über dasselbe? Ich bekomme nämlich in keiner Konstellation irgendeinen JS-Fehler in die Console (das macht es ja so schwierig + alle Logs sind auch clean), und wenn ja, wie du andeutest, dann würde der Zoom ja gar nicht mehr gehen. Tut er aber doch fehlerfrei, oder was meinst du genau?
Aber kurz sorry, da ich vorhin deine Infos zu Bug 6552 nicht alle auf dem Zettel hatte. Das hatte ich noch nachgeholt und frage mich nun, ob dein Patch der details.min.js wirklich 100% funzt? Würde ich gerne testen, hast du evtl. ein ZIP für mich (bei github kann ich es nicht kompiliert laden oder)?
Die photoswipe-Patches hatten nämlich bisher alle andere Macken. Entweder wurde Scrollen oder Wischen nicht berücksichtigt, oder es fehlt die Zoom-Navi, oder wenn die geht, dann fehlen einzelne Bilder in der Zoom-Galerie. Das macht es ja nicht unbedingt besser, auch wenn der Zoom dann schließt. :wink:

[QUOTE=vanilla thunder;188140]Seinem Kind die Beine zu brechen verhindert zwar, dass er später ein schlechter Tänzer wird, aber nur weil er gar kein Tänzer mehr werden kann.[/QUOTE]
Oh Gott. :smiley:

@Mitmacher:
Der Zoom-Fehler wirft soweit ich weiß keine JS-Fehler in der Konsole. Das Problem war eigentlich nur, dass das Bild “unsichtbar” noch da war und man deswegen nichts mehr anklicken konnte - weil ja das Bild noch drüber lag, nur halt unsichtbar.

Wenn man etwas runterscrollte, konnte man z.B. im Footer die Links anklicken. Soweit runter reichte das Bild halt nicht, um den Bereich noch zu überdecken.

@phillinger
Ja, schon klar, ich weiß, aber trotzdem danke für die Erklärung! :slight_smile:
Und bei dir geht es tatsächlich in allen Konstellationen mit dem VT-patch (details.min.js)? Und hattest du dir das selbst kompiliert mit grunt?

PS: ich habe es gerade mal selbst versucht, aber da kommen haufenweise Fehler (beim Kompilieren mit grunt), wenn ich dieses hinzufüge:

gallery.listen('destroy', function() {
   setTimeout(function() { $('.pswp').removeClass().addClass('pswp'); }, 10);
});

das ist aber doch korrektes JS, ominös…

PPS: peinlich, ich hatte bei mir aus Versehen die Pluszeichen aus GitHub am Anfang von 2 Zeilen mitkopiert, ähem. Also alles gut, der Code ist korrekt und funzt, wie unten bereits bemerkt… :slight_smile:

[QUOTE=Mitmacher;188145]Und bei dir geht es tatsächlich in allen Konstellationen mit dem VT-patch (details.min.js)? Und hattest du dir das selbst kompiliert mit grunt?[/QUOTE]
Also der Zoom-Bug ist bei mir überall weg - egal ob mobile, desktop, > 4 Bilder oder nicht.

Ich nutze einfach die photoswipe.min.js von vanilla_thunder, die er hier verlinkt.

das da ist OXID.Design, ich bin vanilla thunder :smiley:

meine details.min.js (nicht photoswipe.min.js) ist das hier:
https://raw.githubusercontent.com/vanilla-thunder/flow_theme/3858f56bce728bd7309caccff2cb378b06a236fa/out/flow/src/js/pages/details.min.js
einfach copy-paste in die eigene details.min.js oder drüber bügeln.

Ach du Schande, da habe ich ja toll zur Verwirrung beigetragen. Sorry!

das da ist OXID.Design, ich bin vanilla thunder

haha, genau! Und danke nochmal für den Link, da kam ich wohl auch durcheinander.

Ach du Schande, da habe ich ja toll zur Verwirrung beigetragen. Sorry!

ja, hatte ich mir zwar irgendwann schon fast gedacht, verwirrt hattest du mich allerdings tatsächlich kurz! nun denn, hauptsache wir werden uns alle irgendwann mal einig… :wink:

Hat denn irgendwer den OD-Patch (also photoswipe von OXID-Design) 100% fehlerfrei am Laufen (nicht dass ich hier unnötig pauschalisiere und die genannten Bugs nur bei mir auftreten)? Ich fände es aber eh besser, an der details zu schrauben, da die halt zu OXID gehört. So, genug blabla, einfach getestet, und was soll ich sagen: sieht in der Tat erstmal gut aus! Das wäre ja schon mal genial als Workaround oder von mir aus auch finale Lösung. Den Flexslider-Bug sollte man wohl aber auch noch angehen, kann ich nur gerade nicht testen…