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

Galleria template with image title and GPS position #45

Merged
merged 1 commit into from Sep 1, 2015

Conversation

3 participants
@cbosdo
Contributor

cbosdo commented Dec 2, 2013

These changes are some small improvements on the galleria template. The one moving title infos to the layer is only a matter of taste.

The UserComment exif data is not the best defined thing in the world... we can face some weird values depending on the image editor.

@saimn

This comment has been minimized.

Owner

saimn commented Dec 3, 2013

Hi @cbosdo,
thanks for this, it is very promising ! Some comments :

  • Could you add some comments to explain the UserComment thing ? it seems also that this is not python 3 compatible, could you have a look ? py3 compat is not yet released, but I really want to have it in the next release.
  • I have just tested the open layers viewer, maybe it is still a work in progress but after I close the map I can't navigate in the images.
  • The open layers viewer is a nice idea, but I think it deserve to be a separate theme. Users of the galleria may not want to have to load the openlayers js/css. Could you split your PR in two : one with just the OSM link, and the other with a new theme with openlayers ?
  • I have committed a fix for images with bad exif datas, could you rebase your PR(s) on it ?
    Thanks!
@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Dec 4, 2013

Hi Simon,

On Tue, 2013-12-03 at 15:34 -0800, Simon Conseil wrote:

Hi @cbosdo,
thanks for this, it is very promising ! Some comments :

  * Could you add some comments to explain the UserComment thing ?
    it seems also that this is not python 3 compatible, could you
    have a look ? py3 compat is not yet released, but I really
    want to have it in the next release.

We'll go into troubles with UserComment when the string is encoded. The
value can be either ASCII string or Unicode string (or a japanese
encoding). From what I tested with Unicode we have different values
depending on the editor writing it.

  • F-spot: ASCII\x00\x00\x00Unicode\x00the string encoded in UTF-16 Big endian
  • Gthumb: the string encoded in UTF-8 Little Endian
    I read somewhere on the web that some Windows applications deliberately
    don't care the Exif bit direction property and always write Little
    Endian.

So there is no really safe way to make sure we get the string right...
and PIL doesn't help here as it may badly parse it and provide it as
Unicode string.

I attached 2 sample pictures showing these problems.

And... indeed I didn't count on python3 here.

  * I have just tested the open layers viewer, maybe it is still a
    work in progress but after I close the map I can't navigate in
    the images.

The openLayer is still work in progress and I'm pretty sure I haven't
put it in the pull request.

  * The open layers viewer is a nice idea, but I think it deserve
    to be a separate theme. Users of the galleria may not want to
    have to load the openlayers js/css. Could you split your PR in
    two : one with just the OSM link, and the other with a new
    theme with openlayers ?

Well, I'm about to transform the openLayers work I did as a galleria
extension. What we could do is add a theme option in sigal to enable map
display on that theme.

  * I have committed a fix for images with bad exif datas, could
    you rebase your PR(s) on it ? Thanks!

Sure.

Cedric

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Jul 9, 2014

So, after a few month inactivity on that one, I managed to address your comments

  • It's not building for python 3
  • the openlayers work is finished
  • I added an option defaulting to false to enable the map in the galleria theme
  • I rebased on a recent master
@saimn

This comment has been minimized.

Owner

saimn commented Jul 11, 2014

Hi Cédric,
thanks for the update ! I'm quite busy at the moment but I will look and try your code as soon as possible.

for image in self.images:
if image.hasLocation():
return True
return False

This comment has been minimized.

@saimn

saimn Jul 11, 2014

Owner

This can be simplified: return any(image.hasLocation() for image in self.images)

For the sake of consistency and pep8 respect, could you also change the function names to lowercase with words separated by underscores ? has_location, show_map.

@@ -132,6 +132,9 @@ def __init__(self, filename, path, settings):
if self.exif is not None and 'dateobj' in self.exif:
self.date = self.exif['dateobj']
def hasLocation(self):
return 'gps' in self.exif

This comment has been minimized.

@saimn

saimn Jul 11, 2014

Owner

You need to check if self.exif is not None.

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Jul 15, 2014

Sorry about my horrible python coding, I'm not that much into python yet ;) All comments addressed

<a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
{% endif %}
</td>
</table>

This comment has been minimized.

@saimn

saimn Jul 16, 2014

Owner

Using a table is not a good idea, as it limits the width of the breadcrumb even if there is no icon. You can simply wrap the icons in a div and push it to the right (float: right or a combination of position & inline-block):

          <div class="icons">
            <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
          </div>

This comment has been minimized.

@cbosdo

cbosdo Jul 17, 2014

Contributor

On Wed, 2014-07-16 at 13:41 -0700, Simon Conseil wrote:

In sigal/themes/galleria/templates/index.html:

       {%- for url, title in album.breadcrumb -%}
       <a href="{{ url }}">{{ title }}</a>
       {%- if not loop.last %} » {% endif -%}
       {% endfor -%}
  •        </td>
    
  •        <td style="width: 100%"></td>
    
  •        <td>
    
  •      {% if settings.show_map and album.show_map %}
    
  •        <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
    
  •      {% endif %}
    
  •        </td>
    
  •      </table>
    

Using a table is not a good idea, as it limits the width of the
breadcrumb even if there is no icon. You can simply wrap the icons in
a div and push it to the right (float: right or a combination of
position & inline-block):

      <div class="icons">
        <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
      </div>

That's what I tried first, but I never managed to come up with a proper
CSS combination of float and his friends to have a clean layout. I'm no
CSS expert and would appreciate if you have a better experience than me
on that topic.

@saimn

This comment has been minimized.

Owner

saimn commented Jul 16, 2014

No problem :). I just gave a try with your branch, so I have a few more comments:

  • please remove ie6-style*.css ;-)
  • the "google" css seems unused, as well as the mobile style. If I understand correctly Openlayers loads only style.css ?
  • when I navigate in the images with the keyboard, it breaks and I no more have control on the images (no left/right with the keyboard or mouse). I get this exception:
    Uncaught TypeError: Cannot read property 'layer' of undefined in OpenLayers.js:832
    which breaks in the block this.bind('image', function(e) ...
  • from the code I guess that clicking a point on the map should show the image, but it doesn't work.
@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Jul 17, 2014

On Wed, 2014-07-16 at 13:51 -0700, Simon Conseil wrote:

No problem :). I just gave a try with your branch, so I have a few
more comments:

  * please remove ie6-style*.css ;-)
  * the "google" css seems unused, as well as the mobile style. If
    I understand correctly Openlayers loads only style.css ?

Hum, indeed I need to check if some of the openlayers files can be
removed. These ones are just the unpacked tarball.

  * when I navigate in the images with the keyboard, it breaks and
    I no more have control on the images (no left/right with the
    keyboard or mouse). I get this exception: Uncaught TypeError:
    Cannot read property 'layer' of undefined in OpenLayers.js:832
    which breaks in the block this.bind('image', function(e) ...

Ouch! I'll try to reproduce.

  * from the code I guess that clicking a point on the map should
    show the image, but it doesn't work.

Not exactly, it calls the Galleria function to show the image to update
the Galleria active photo (seen in the thumbnails roll and when going
back to the picture view). To see the normal picture, you need to toggle
the view mode.

Cedric

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Oct 17, 2014

  • when I navigate in the images with the keyboard, it breaks and I no more have control on the images (no left/right with the keyboard or mouse). I get this exception: Uncaught TypeError: Cannot read property 'layer' of undefined in OpenLayers.js:832 which breaks in the block this.bind('image', function(e) ...

Could you tell me how you managed to do that? I couldn't reproduce that behavior here.

@saimn

This comment has been minimized.

Owner

saimn commented Oct 19, 2014

Hi @cbosdo
I don't remember, I will check if I can reproduce.

@cbosdo cbosdo force-pushed the cbosdo:master branch from 2205e65 to 3fed7da Feb 6, 2015

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Feb 6, 2015

@saimn I just fixed a race between galleria and map events and switched from openlayer 2 to leafletjs. It has the advantage to be much more scalable: I am loading 200+ pictures in the same folder without any trouble.

I also dropped the first commit as you introduced something similar in between. I hope it'll be OK for you this time ;)

@saimn

This comment has been minimized.

Owner

saimn commented Feb 16, 2015

Thanks for the update Cedric, it is great to see that you are still working on this ;).
I don't have a lot of time for sigal currently, but I will test this asap.

@muggenhor

This comment has been minimized.

Contributor

muggenhor commented Mar 2, 2015

As an example of how this can look like: http://sigal-test-parque.mortis.eu/

I did modify two things though, first of all the URL text in the image description for the location info. I.e. to make it look like Nxx.xxxxxx, Ey.yyyyyy instead.

Secondly I added a keybind for map toggling (keycode 0x4D, aka 'm') because as you can see with this the map/picture toggling icon/button is displayed above the album listing (if there are sub albums), which makes switching a great deal more difficult. For usability it's probably better to place it directly below or above the gallery display itself. Also the same toggling icon is not displayed for the gallery root if that has pictures. That should probably be fixed as well.

Other than that this is great 👍.

PS I'll probably remove the example gallery within a month or so.

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Mar 3, 2015

@muggenhor you probably have a patch to propose for this enhancement, right?

@muggenhor

This comment has been minimized.

Contributor

muggenhor commented Mar 3, 2015

@cbosdo only hacks in the template file, here's a diff below.

I did a few more things there though, one of which you probably don't want to include in this pull (i.e. the #fullscreen stuff). All of my changes:

  • Move map-toggling icon to be just above the gallery (and make sure it's also displayed if we're in the root gallery, which has no breadcrumb).
  • Don't put the toggling icon in a table, use a <div> instead
  • Use 'anchored' coordinate descriptions (anchored to N/S and E/W) and make it a clickable link to OSM
  • Use https links for OSM
  • Make fullscreen switching possible, this you probably don't want, so ignore the CSS addition of #fullscreen, the addition of the data-big attribute to images, addition of the fullscreen link, addition of the toggleFullscreen keybind and event handler for the fullscreen link.

If I have the time and think of it I can clean this up in the weekend to be a separate commit on top of your branch, I definitely don't have the time right now though.

diff --git a/sigal/themes/galleria/templates/index.html b/sigal/themes/galleria/templates/index.html
index 48e851c..80b9372 100644
--- a/sigal/themes/galleria/templates/index.html
+++ b/sigal/themes/galleria/templates/index.html
@@ -42,6 +42,17 @@
         .galleria-thumbnails .galleria-image {
            opacity: 0.6;
         }
+        .icons {
+          float: right;
+        }
+        #gallery {
+          clear: both;
+        }
+        {% if settings.show_map and album.show_map %}
+        #fullscreen {
+          cursor: pointer;
+        }
+        {% endif %}
     </style>
     {% endif %}
   </head>
@@ -62,20 +73,10 @@

         {% if album.breadcrumb %}
         <h2>
-          <table>
-            <td>
           {%- for url, title in album.breadcrumb -%}
           <a href="{{ url }}">{{ title }}</a>
           {%- if not loop.last %} » {% endif -%}
           {% endfor -%}
-            </td>
-            <td style="width: 100%"></td>
-            <td>
-          {% if settings.show_map and album.show_map %}
-            <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
-          {% endif %}
-            </td>
-          </table>
         </h2>
         <hr>
         {% endif %}
@@ -108,21 +109,29 @@
               {% if media.exif.exposure %}Exposure: {{ media.exif.exposure }}, {% endif %}
               {% if media.exif.fstop %}Fstop: {{ media.exif.fstop }}{% endif %}
               {% if media.exif.gps %}
-                <br/>Location: {{ media.exif.gps.lat }}, {{ media.exif.gps.lon }}
-                <a href='http://www.openstreetmap.org/?mlat={{ media.exif.gps.lat }}&mlon={{ media.exif.gps.lon }}&zoom=12&layers=M' target='_blank' class='map'/>
+                <br/>Location: <a href='https://www.openstreetmap.org/?mlat={{
+                media.exif.gps.lat }}&amp;mlon={{ media.exif.gps.lon}}&amp;zoom=12&amp;layers=M' target='_blank' class='map'
+                >{{ 'N{:.6f}'.format(media.exif.gps.lat) if media.exif.gps.lat > 0 else 'S{:.6f}'.format(-media.exif.gps.lat) }}
+                 {{ 'E{:.6f}'.format(media.exif.gps.lon) if media.exif.gps.lon > 0 else 'W{:.6f}'.format(-media.exif.gps.lon) }}</a>
               {% endif %}
               {% if media.exif.datetime %}
                 <br>Date: {{ media.exif.datetime }}
               {% endif %}
             {% endif %}
           {%- endmacro %}
+        {% if settings.show_map and album.show_map %}
+        <div class="icons">
+          <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"/></a>
+        </div>
+        {% endif %}
         <div id="gallery">
           {% for media in album.medias %}
             {% if media.type == "image" %}
               <a href="{{ media.filename }}">
                 <img src="{{ media.thumbnail }}" alt="{{ media.filename }}"
                     data-title="{{ media.title if media.title else media.filename }}"
-                    data-description="{{ img_description(media) }}"/>
+                    data-description="{{ img_description(media) }}"
+                   {% if media.big %} data-big="{{ media.big }}"{% endif %} />
               </a>
             {% endif %}
             {% if media.type == "video" %}
@@ -137,6 +146,7 @@
             {% endif %}
           {% endfor %}
         </div>
+        <a id="fullscreen">fullscreen</a>
         {% endif %}

         {% if album.zip %}
@@ -213,8 +223,8 @@
         if ($("#galleria-map").length == 0) {
             stage.after("<div id='galleria-map'></div>");
             map = L.map('galleria-map');
-            var attrib = '&copy; <a href="http://osm.org/copyright" title="OpenStreetMap" target="_blank">OpenStreetMap</a> contributors | Tiles Courtesy of <a href="http://www.mapquest.com/" title="MapQuest" target="_blank">MapQuest</a> <img src="http://developer.mapquest.com/content/osm/mq_logo.png" width="16" height="16">';
-            L.tileLayer('http://{s}.mqcdn.com/tiles/1.0.0/map/{z}/{x}/{y}.png', {
+            var attrib = '&copy; <a href="https://osm.org/copyright" title="OpenStreetMap" target="_blank">OpenStreetMap</a> contributors | Tiles Courtesy of <a href="https://www.mapquest.com/" title="MapQuest" target="_blank">MapQuest</a> <img src="https://developer.mapquest.com/content/osm/mq_logo.png" width="16" height="16">';
+            L.tileLayer('https://{s}-s.mqcdn.com/tiles/1.0.0/map/{z}/{x}/{y}.png', {
                         attribution: attrib,
                         subdomains: ['otile1','otile2','otile3','otile4']
                         }).addTo( map );
@@ -252,25 +262,8 @@
     });
     Galleria.run("#gallery");

-    Galleria.ready(function() {
-        this.attachKeyboard({
-            right: this.next,
-            left: this.prev
-         });
-
-        {% if settings.show_map and album.show_map %}
-        this.bind('loadfinish', function(e) {
-            init_map();
-            if (selected != e.index) {
-                selectMarker(features[e.index]);
-                selected = e.index;
-            }
-        });
-        {% endif %}
-     });
-
     {% if settings.show_map and album.show_map %}
-    $(".gallery-map-toggle").click(function() {
+    function galleryMapToggle() {
         /* Make sure the map is loaded */
         init_map();

@@ -293,8 +286,34 @@
             /* Change display of the Map link */
             $(".gallery-map-toggle img")[0].src = "{{ theme.url }}/img/photo.png";
         }
-    });
+    }
+    $(".gallery-map-toggle").click(galleryMapToggle);
     {% endif %}
+
+    Galleria.ready(function() {
+        this.attachKeyboard({
+            right: this.next,
+            left: this.prev,
+            0x46: this.toggleFullscreen,
+            {% if settings.show_map and album.show_map %}
+            0x4D: galleryMapToggle,
+            {% endif %}
+         });
+       var gallery = this;
+       $('#fullscreen').click(function() {
+          gallery.toggleFullscreen();
+        });
+
+        {% if settings.show_map and album.show_map %}
+        this.bind('loadfinish', function(e) {
+            init_map();
+            if (selected != e.index) {
+                selectMarker(features[e.index]);
+                selected = e.index;
+            }
+        });
+        {% endif %}
+     });
     </script>
     {% endif %}
     {% include 'analytics.html' %}
@muggenhor

This comment has been minimized.

Contributor

muggenhor commented Mar 3, 2015

The changes as a branch: muggenhor/sigal@map
Feel free to pull them into your branch (which'll update this pull request)

@saimn

This comment has been minimized.

Owner

saimn commented Mar 3, 2015

I made a little test, and it works fine, really nice !
If you can have a look at @muggenhor 's changes; I hope we can finally integrate this in sigal !
@muggenhor : it seem you have some patch to use galleria in full screen mode ? It would be great if you could share this (and make a PR) when you have some time.

@muggenhor

This comment has been minimized.

Contributor

muggenhor commented Mar 4, 2015

@saimn yes, I've got a patch for fullscreen mode. It was set up a bit hacky though, so I've cleaned it up and pushed it to #149.

@muggenhor

This comment has been minimized.

Contributor

muggenhor commented Apr 7, 2015

It seems there's a problem when you have a gallery with mixed photos. Specifically a few with and a few without GPS Exif data. If, for example you have 3 photos, 1 has GPS data, 2 doesn't and 3 does, then when selecting the second dot on the map the second photo is activated instead of the third. I.e. the mapping of the position-marking-dots to the photo-indices appears to depend on the assumption that there are just as much photos in the gallery and that they appear in the same order: this is a wrong assumption.

@saimn

This comment has been minimized.

Owner

saimn commented Aug 28, 2015

Hi @cbosdo and @muggenhor , any chance that one of you could help to get this merged for the mext release ? It would ne nice to finally have it.

@cbosdo cbosdo force-pushed the cbosdo:master branch from 3fed7da to 0cd5924 Sep 1, 2015

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Sep 1, 2015

Sorry for the delay, that project is kind of low priority to me and get less attention that it should have.
Got @muggenhor changes in + fixed the bug he mentioned in his last comment.

@cbosdo cbosdo force-pushed the cbosdo:master branch from 0cd5924 to e6d97f4 Sep 1, 2015

Map view for albums in galleria theme
This feature hacks galleria to show an openlayers map instead of the
picture. The pictures are represented by dots on the maps and clicking
them or the galleria thumbnails update the other (and the selected
picture).

By default, the map isn't shown: it needs to be enabled by the show_map
options.

@cbosdo cbosdo force-pushed the cbosdo:master branch from e6d97f4 to 8e4a08a Sep 1, 2015

@cbosdo

This comment has been minimized.

Contributor

cbosdo commented Sep 1, 2015

Fixed some rebase errors, now all green for merge

@saimn saimn merged commit 8e4a08a into saimn:master Sep 1, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@saimn

This comment has been minimized.

Owner

saimn commented Sep 1, 2015

Merged, finally ;), thanks Cédric !

@saimn saimn added this to the 0.10 milestone Sep 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment