🌍
English
Hello JanisV,
As everyone said it: great plugin. We really didn't have anything alike. Here are some issues:
1. There are some generated urls in fotorama-content.tpl with &slideshow or &slidestop. These do not work if $conf['question_mark_in_url '] is false. Would it be possible to use ? instead of & if there is no ? in the active frame url ?
2. I am concerned about performance for large number of items (flat mode gallery for example). On my site I have 4k images and it takes ~ 20 seconds for the slideshow to start on my mobile with fast wifi connection. There are several bottlenecks, but the most important is the fotorama javascript initialization. I think fotorama can load data from javascript objects a lot faster than from html code. What about replace inner html from div class="fotorama" with javascript ?
A second step (but a lot harder to implement) is to load partially the data set (e.g. a couple of hundred before/after current rank and as the slideshow advances towards the end of the loaded slice, we load the second slice ...
3. Is there a purpose of image_w and image_h variables in fotorama.tpl ?
Again really nice - I didn't play with all options, trying to find the best...
Offline
Also we can optimize the php code that generates the slideshow data:
1. MySql query is very slow for large number of items because of ORDER BY FIELD. I think it's faster to sort in php using $page['rank_of']. eg usort($picture, 'page_rank_compare');
2. while fetch_assoc loop
- $thumb_params should be computed outside the loop only if thumb is needed
- replace $type and $big_type with $type_params and $big_type_params (otherwise we search the params from type for every item)
- TITLE_ESC not used
What do you think ? If you want I could make the changes myself.
Thanks
Offline
Hi, rvelices!
Thank you for your code review. I'm apply this improvemnts.
About issues:
* image_w and image_h is required for stripped theme
* perfomance for large number of images - load with ajax was planned for future version (realy hard for implementation), about JS initialization I will check how it work with lazy loading. But thanks for this idea
Offline
JanisV wrote:
* perfomance for large number of images - load with ajax was planned for future version (realy hard for implementation), about JS initialization I will check how it work with lazy loading. But thanks for this idea
Thanks,
js init doc is here http://fotorama.io/customize/initialization/ (independent of lazy loading)
Offline
Thanks.
What about &slideshow ? (It breaks modus computation of available size for photos because the js in modus tests if the current page is in slideshow mode by looking at slideshow in in the query string. in this case the url is invalid anyway)
Thanks
PS. Do you want me to fix it ? (something like:
fotorama.activeFrame['url']+(fotorama.activeFrame['url'].indexOf('?')==-1 ? '?' : '&')+'slideshow='
)
Offline
rvelices wrote:
js init doc is here http://fotorama.io/customize/initialization/ (independent of lazy loading)
I have a problem with insert image URL into JS code: derivative->get_url() return escaped URL (& -> &) when original_url_protection enabled. Quick workaround is rollback replacing. Any good solution?
Offline
rvelices wrote:
Thanks.
What about &slideshow ? (It breaks modus computation of available size for photos because the js in modus tests if the current page is in slideshow mode by looking at slideshow in in the query string. in this case the url is invalid anyway)
Thanks
PS. Do you want me to fix it ? (something like:
fotorama.activeFrame['url']+(fotorama.activeFrame['url'].indexOf('?')==-1 ? '?' : '&')+'slideshow='
)
If you are have a fix for &slideshow - please apply it
Offline
JanisV wrote:
rvelices wrote:
js init doc is here http://fotorama.io/customize/initialization/ (independent of lazy loading)
I have a problem with insert image URL into JS code: derivative->get_url() return escaped URL (& -> &) when original_url_protection enabled. Quick workaround is rollback replacing. Any good solution?
Its because footer scripts are in cdata sections. I think you did it right
Offline
If you are have a fix for &slideshow - please apply it
Ill do it this weekend. Thanks for the green light.
I also saw the javascript init. Ill also test during the weekend.
Btw
Captions do not work the first time. They work when we change the full screen mode only. I think this is because of data-captions=false
Also in php when you write $x = new x there is no need to test that $x is not null (i m talking about derivative big_derivative and thumb)
Offline
rvelices wrote:
If you are have a fix for &slideshow - please apply it
Ill do it this weekend. Thanks for the green light.
&slideshow (question_mark_in_urls problem) fixed:)
Caption work only in fullscreen mode because in all themes slideshow pages has a image title at top of page. I proceeded from this.
MySql ORDER BY FIELD improvement for you (if you can compare performance). Also any other suggestions - welcome.
Offline
JanisV wrote:
Caption work only in fullscreen mode because in all themes slideshow pages has a image title at top of page. I proceeded from this.
Ok but then why do the caption appear if I go to fullscreen mode and then go back to normal mode ? (my config allow full screen: native, only full screen:false, Show caption with image title:true
JanisV wrote:
MySql ORDER BY FIELD improvement for you (if you can compare performance)
I'll look into it
JanisV wrote:
Also any other suggestions - welcome.
incompatible with IE8 because it does not have history.replaceState. All calls to history.replaceState should become if (history.replaceState) history.replaceState ... and it works
By the way, you could regroup common code from showend and fullscreenexit together ... It will avoid duplications and forgotten code (e.g. fotorama__info-icon').attr('href' is called only from one event and I don't think this is the intention)
Thanks for all the changes again
Last edited by rvelices (2014-10-10 15:31:52)
Offline
rvelices wrote:
incompatible with IE8 because it does not have history.replaceState. All calls to history.replaceState should become if (history.replaceState) history.replaceState ... and it works
By the way, you could regroup common code from showend and fullscreenexit together ... It will avoid duplications and forgotten code (e.g. fotorama__info-icon').attr('href' is called only from one event and I don't think this is the intention)
JavaScript code updated. May be best solution for IE8 is using history.js, but I don't realy want support this browser.
Ok, at this friday evening I make a release for playing with it at weekend. It contain many improvements - very big thank for you!
Offline
I think I have an idea of how to deal with a large number of items.
The easiest thing to do is to load lets say 100 items before and 500 after. When fotorama gets to the end, we reload the slideshow page at current + 400. A bit as we do today but just not on every page.
Do you think it's a good idea ?
I can try and do it.
Offline
Ok. I tested the js initialization. It goes roughly from 20s to 5s
Very good optimization.
Offline
rvelices wrote:
I think I have an idea of how to deal with a large number of items.
The easiest thing to do is to load lets say 100 items before and 500 after. When fotorama gets to the end, we reload the slideshow page at current + 400. A bit as we do today but just not on every page.
Do you think it's a good idea ?
I can try and do it.
Fotorama has a methods push and unshift, so reloading not required. Another problem with reloading - thumbnails will work incorrect at end of image set.
100 and 500 items is too little - it is only json data that very small. Real problem can be with 10'000 items.
Offline