Album cannot contain both subfolders and images

[thomasdn]

Hi,

When running sigal on a folder structure as the one below, sigal complains with this warning:

WARNING: Album X contains sub-albums and images. Please move images to their own sub-album. Images in album X will not be visible.

This results in all the images directly in the album not being visible. Only those in subfolders. I think this is unfortunate as it is quite useful to have subfolders within an album. Especially if it is a big album. For example, when I have pictures from a vacation, I might want to put all the videos in a separate subfolder called videos. Also, I sometimes take pictures of food. Most people are uninterested in those pictures. So I put all the food related pictures in a subfolder.

The folder structure that produces the above problem is:

pictures/
pictures/vacation-pictures/
pictures/vacation-pictures/img_001.jpg
pictures/vacation-pictures/img_002.jpg
pictures/vacation-pictures/img_003.jpg
pictures/vacation-pictures/food/
pictures/vacation-pictures/food/beautiful-sandwich.jpg
pictures/vacation-pictures/videos/
pictures/vacation-pictures/videos/video1.mp4

In this case, the following pictures will be omitted form the gallery:

pictures/vacation-pictures/img_001.jpg
pictures/vacation-pictures/img_002.jpg
pictures/vacation-pictures/img_003.jpg

[fidergo-stephane-gourichon]

Hi! Another user here. I also noticed this and it feels like it could be improved.

I had the opportunity to visit sigal source code in the past (I think the design is very well thought out) and remember two templates.

@saimn is this limitation "only" caused by having two templates, one for a page listing sub-albums, one for a page listing photos?

Could it be solved by having one template containing both parts, which would be activated or not depending on whether the current directory contains photos and/or contains subfolders?

Thanks!

[thomasdn]

I think the right way to present an album containing both images and sub albums is to present it with all the subalbums/folders first followed by all the images. Maybe this could be done by creating a parent template that includes a subtemplate for presenting subalbums followed by a subtemplate for presenting images.

Since the templates are based on jinja2, it should be possible to use template inheritance to solve this, right?

[saimn]

is this limitation "only" caused by having two templates, one for a page listing sub-albums, one for a page listing photos?

Yes, this choice was made (#343, #347, #348) to improve the maintenance of templates, and because this didn't seem to be a critical feature. But it's still possible to make a custom theme/template that would do this (we could remove the warning), and I would certainly welcome a pull request to add this possibility in sigal if it's done in a better way.

[thomasdn]

Yes, this choice was made (#343, #347, #348) to improve the maintenance of templates, and because this didn't seem to be a critical feature.

Admittedly, I only took a quick glance at the code/templates, but am I wrong in that it would not require a bigger effort in maintaining a single template that contains both images and subfolders using jinja2 template inheritance?

(we could remove the warning)

I do not think the warning should be removed if the files are actually not shown in the gallery. It was really confusing to me that some photos were not shown and I would probably just have given up finding out why and concluded the tool did not work if I had not seen the warning.

So I think the warning should stay until a combined template is added.

[thomasdn]

Here is a suggestion on such a combined template:

{% extends "base.html" %}

{% block extra_head %}
  <script src="{{ theme.url }}/photoswipe.min.js"></script>
  <script src="{{ theme.url }}/photoswipe-ui-default.min.js"></script>
  <script src="{{ theme.url }}/echo/echo.min.js"></script>
  <link rel="stylesheet" href="{{ theme.url }}/photoswipe.css">
  <link rel="stylesheet" href="{{ theme.url }}/default-skin/default-skin.css">
{% endblock extra_head %}

{% block content %}
  {% if album.zip %}
  <div class="additionnal-infos">
    <p><a href="{{ album.zip }}"
          title="Download a zip archive with all images">Download ZIP</a></p>
  </div>
  {% endif %}

  <div class="album-list">
  {% for alb in album.albums %}
    <div class="menu-img thumbnail">
      <a href="{{ alb.url }}">
        <img src="{{ alb.thumbnail }}" class="album_thumb"
             alt="{{ alb.title }}" title="{{ alb.title }}" /></a>
      <span class="album_title">{{ alb.title }}</span>
    </div>
  {% endfor %}
  </div>

  <div class="gallery" itemscope itemtype="http://schema.org/ImageGallery">
    {% for media in album.medias %}
      {% if media.type == "image" %}
        <figure class="gallery__img--{{ "main" if loop.first else "secondary" }} thumbnail"
                itemprop="associatedMedia" itemscope itemtype="http://schema.org/ImageObject">
          <a href="{{ media.url }}" itemprop="contentUrl" data-size="{{media.size.width}}x{{media.size.height}}">
            <img src="{{ theme.url }}/echo/blank.gif"
                 data-echo="{{ media.url if loop.first else media.thumbnail }}"
                 alt="{{ media.url }}" itemprop="thumbnail" title="{{ media.exif.datetime }}" />
          </a>
          <figcaption itemprop="caption description">{{ media.title }} - {{ media.exif.datetime }}</figcaption>
        </figure>
      {% endif %}
      {% if media.type == "video" %}
        <figure class="gallery__img--secondary thumbnail"
                itemprop="associatedMedia" itemscope itemtype="http://schema.org/ImageObject">
          <a href="{{ media.url }}" itemprop="contentUrl" data-type="video"
             data-video='<div class="video"><div class="video__container"><video controls><source src="{{ media.url }}" type="{{ media.mime }}" /></video></div></div>'>
          <img src="{{ theme.url }}/echo/blank.gif"
               data-echo="{{ media.thumbnail }}"
               alt="{{ media.thumbnail }}" itemprop="thumbnail" title="" />
          </a>
          <figcaption itemprop="caption description">{{ media_title }}</figcaption>
        </figure>
      {% endif %}
    {% endfor %}
  </div>

{% endblock %}

{% block extra_footer %}
  <!-- Root element of PhotoSwipe. Must have class pswp. -->
  <div class="pswp" tabindex="-1" role="dialog" aria-hidden="true">
    <!-- Background of PhotoSwipe.
      It's a separate element as animating opacity is faster than rgba(). -->
      <div class="pswp__bg"></div>

      <!-- Slides wrapper with overflow:hidden. -->
      <div class="pswp__scroll-wrap">
        <!-- Container that holds slides.
          PhotoSwipe keeps only 3 of them in the DOM to save memory.
          Don't modify these 3 pswp__item elements, data is added later on. -->
          <div class="pswp__container">
            <div class="pswp__item"></div>
            <div class="pswp__item"></div>
            <div class="pswp__item"></div>
          </div>

          <!-- Default (PhotoSwipeUI_Default) interface on top of sliding area. Can be changed. -->
          <div class="pswp__ui pswp__ui--hidden">
            <div class="pswp__top-bar">
              <!--  Controls are self-explanatory. Order can be changed. -->
              <div class="pswp__counter"></div>
              <button class="pswp__button pswp__button--close" title="Close (Esc)"></button>
              <button class="pswp__button pswp__button--share" title="Share"></button>
              <button class="pswp__button pswp__button--fs" title="Toggle fullscreen"></button>
              <button class="pswp__button pswp__button--zoom" title="Zoom in/out"></button>

              <!-- Preloader demo http://codepen.io/dimsemenov/pen/yyBWoR -->
              <!-- element will get class pswp__preloader-active when preloader is running -->
              <div class="pswp__preloader">
                <div class="pswp__preloader__icn">
                  <div class="pswp__preloader__cut">
                    <div class="pswp__preloader__donut"></div>
                  </div>
                </div>
              </div>
            </div>

            <div class="pswp__share-modal pswp__share-modal--hidden pswp__single-tap">
              <div class="pswp__share-tooltip"></div>
            </div>
            <button class="pswp__button pswp__button--arrow--left" title="Previous (arrow left)">
            </button>
            <button class="pswp__button pswp__button--arrow--right" title="Next (arrow right)">
            </button>
            <div class="pswp__caption">
              <div class="pswp__caption__center"></div>
            </div>
          </div>
      </div>
  </div>
  <script src="{{ theme.url }}/app.js"></script>

{% endblock %}

This currently does not work as it is because gallery.py needs to be adjusted so that this template is actually fed both album_list and album elements. When there are no immediate pictures inside the album_list, then "album" could just contain an empty list.

I did not attempt making the changes to gallery.py myself.

[thomasdn]

OK. So I looked a bit into the code for this.

As I understand it, this is the code that produces the errors and also the code that decides to use a different template for albums that contains albums:

From gallery.py:

    if self.settings['write_html']:                                                                                                 
        album_writer = AlbumPageWriter(self.settings,                                                                               
                                       index_title=self.title)                                                                      
        album_list_writer = AlbumListPageWriter(self.settings,                                                                      
                                                index_title=self.title)                                                             
        with progressbar(self.albums.values(),                                                                                      
                         label="%16s" % "Writing files",                                                                            
                         item_show_func=log_func, show_eta=False,                                                                   
                         file=self.progressbar_target) as albums:                                                                   
            for album in albums:                                                                                                    
                if album.albums:                                                                                                    
                    if album.medias:                                                                                                
                        self.logger.warning(                                                                                        
                            "Album %s contains sub-albums and images. "                                                             
                            "Please move images to their own sub-album. "                                                           
                            "Images in album %s will not be visible.",                                                              
                            album.title, album.title                                                                                
                        )                                                                                                           
                    album_list_writer.write(album)                                                                                  
                else:                                                                                                               
                    album_writer.write(album)                                                                                       

album_list_writer and album_writer are these:

class AlbumListPageWriter(AbstractWriter):                                                                                              
    """Generate an html page for a directory of albums"""                                                                               
    template_file = "album_list.html"                                                                                                   

class AlbumPageWriter(AbstractWriter):                                                                                                  
    """Generate html pages for a directory of images."""                                                                                
    template_file = "album.html"                      

These are completely identical except for the use of two different templates. The AbstractWriter.writer function looks like this:

def write(self, album):                                                                                                             
    """Generate the HTML page and save it."""                                                                                       
    context = self.generate_context(album)                                                                                          
    signals.before_render.send(context)                                                                                             
    page = self.template.render(**context)                                                                                          
    output_file = os.path.join(album.dst_path, album.output_file)                                                                   

    with open(output_file, 'w', encoding='utf-8') as f:                                                                             
        f.write(page)                                                                                                               

I see two ways to implement support for albums with both photos and subalbums:

1) Change AbstractWriter.writer so that it allows being passed both album and album.medias (as an optional second argument) and then if this argument is passed, both the elements of album.albums and album.medias are passed along to the template.

2) Change AbstractWriter.writer so that it checks if album has a non-empty album.medias and if it does, then it passes along album.medias to the template. And check if album has a non-empty album.albums, and if it does, passes this along to the template as well.

Of course, those of you that are familiar with the codebase probably have a much better approach.

[thomasdn]

Do you agree with the above two ways of implementing this? Or is there a better way?

Is it something you want to prioritize implementing? I would be happy to help implementing this. But I would like to know if the proposed way of implementing it would be accepted or if it needs to be solved in a different way before doing it. Maybe there is a third way that is even better than what I am proposing?

Of course, the best would be if this could be solved entirely in the templates - without changing the actual code. But I do not believe this is possible as it is now. Do you agree?

[saimn]

Thanks for exploring the solutions, it seems you're on a good track.

Change AbstractWriter.writer so that it allows being passed both album and album.medias (as an optional second argument) and then if this argument is passed, both the elements of album.albums and album.medias are passed along to the template.

album.medias is already passed to the template, since that's an attribute of album. So there is not a lot to do on the Python side, which is not the real problem. The question is how to organize the templates to avoid a huge blob of HTML/JS. I guess one way to proceed would be to put the HTML code to generate the list of albums in a new file that could be included both in "album_list.html" and "album.html".

[thomasdn]

Thanks for exploring the solutions, it seems you're on a good track.

Change AbstractWriter.writer so that it allows being passed both album and album.medias (as an optional second argument) and then if this argument is passed, both the elements of album.albums and album.medias are passed along to the template.

album.medias is already passed to the template, since that's an attribute of album. So there is not a lot to do on the Python side, which is not the real problem. The question is how to organize the templates to avoid a huge blob of HTML/JS. I guess one way to proceed would be to put the HTML code to generate the list of albums in a new file that could be included both in "album_list.html" and "album.html".

I have not had success in implementing something that works. Would someone else be wiling to give it a try?

[DuendeInexistente]

Maybe a directory tree on the side and images in the main view like a typical file browser. Or two separate views, with the folders listed between the breadcrumbs and the image list

[thomasdn]

I believe I have a working fix to this problem now. I have changed the album_list.html template in the gallaria.

This works in the combination of theme='galleria' and galleria_theme='folio'.

Here is the full diff:

--- bak/album_list.html 2021-07-07 11:25:49.929585457 +0200
+++ album_list.html 2021-07-07 11:28:43.553966015 +0200
@@ -24,6 +24,23 @@
             {% endfor %}
           </ul>
         </div>
+        <div class="icons">
+          {% if settings.show_map and album.show_map %}
+          <a class="gallery-map-toggle"><img src="{{ theme.url }}/img/map.png"
+            title="Show/Hide map" alt="Show/Hide Map (m)" /></a>
+          {% endif %}
+          <a id="fullscreen"><img src="{{ theme.url }}/img/fullscreen.png"
+            title="Fullscreen" alt="Fullscreen (f)" /></a>
+        </div>
+        <div id="gallery"></div>
+
+        {% if album.zip %}
+        <div id="additionnal-infos" class="row">
+          <p><a href="{{ album.zip }}"
+                title="Download a zip archive with all images">Download ZIP</a>
+          </p>
+        </div>
+        {% endif %}

         {% if album.description %}
         <div id="description">
@@ -31,3 +48,211 @@
         </div>
         {% endif %}
 {% endblock %}
+
+{% block late_js %}
+    {% macro img_description(media) -%}
+      {%- if media.big -%}<a href='{{ media.big_url }}'>Full size</a>{%- endif -%}
+       {# clean up tags and whitespace, including newlines, in the description #}
+      {%- if media.description -%}<br>{{ media.description | striptags }}{%- endif -%}
+      {%- if media.exif -%}
+        <div class='film-meta'>
+        {%- if media.exif.iso -%}<abbr title='film speed'>{{ media.exif.iso }}</abbr> {%- endif -%}
+        {%- if media.exif.exposure -%}<abbr title='exposure'>{{ media.exif.exposure }}</abbr> {%- endif -%}
+        {%- if media.exif.fstop -%}<abbr title='aperture'>{{ media.exif.fstop }}</abbr> {%- endif -%}
+        {%- if media.exif.focal -%}<abbr title='focal length'>{{ media.exif.focal }}</abbr> {%- endif -%}
+        </div>
+        {%- if media.exif.gps -%}
+          <a title='location' 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.Make or media.exif.Model -%}
+          <abbr title='camera make and model'>{{ media.exif.Make }} {{ media.exif.Model }}</abbr>
+        {%- endif -%}
+        {%- if media.exif.datetime -%}
+          <abbr title='date'>{{ media.exif.datetime }}</abbr>
+        {%- endif -%}
+      {% endif %}
+    {%- endmacro %}
+
+    <script src="{{ theme.url }}/jquery-3.3.1.min.js"></script>
+    {% if settings.show_map and album.show_map %}
+    <script src="{{ theme.url }}/leaflet/leaflet.js"></script>
+    <script src="{{ theme.url }}/leaflet/leaflet-providers.js"></script>
+    {% endif %}
+    <script src="{{ theme.url }}/galleria.min.js"></script>
+    <script src="{{ theme.url }}/themes/{{ settings.galleria_theme }}/galleria.{{ settings.galleria_theme }}.min.js"></script>
+    <script src="{{ theme.url }}/plugins/history/galleria.history.min.js"></script>
+    <script>
+    var data = [
+      {% for media in album.medias -%}
+      {
+        title: "{{ media.title }}",
+        description: "{{ img_description(media) | e }}",
+        thumb: "{{ media.thumbnail }}",
+        {% if media.big %}
+        big: "{{ media.big_url }}",
+        {% endif %}
+        {% if media.type == "image" %}
+        image: "{{ media.url }}"
+        {% endif %}
+        {% if media.type == "video" %}
+        image: "{{ theme.url }}/img/empty.png",
+        layer: "<video controls><source src='{{ media.url }}' type='{{ media.mime }}' /></video>"
+        {% endif %}
+      },
+      {% endfor %}
+    ]
+
+    {% if settings.show_map and album.show_map %}
+    var selected =-1;
+    /* Init markers */
+    var features = new Array();
+
+    function selectMarker(marker) {
+        if (selected >= 0 && features[selected] != null) {
+            features[selected].setStyle({
+                radius: 7,
+                fillColor: "#888a85",
+                color: "#2e3436",
+                weight: 1,
+                fillOpacity: 1,
+                clickable: true
+            });
+            features[selected].redraw();
+        }
+
+        if (marker != null) {
+          marker.setStyle({
+              radius: 7,
+              fillColor: "#cc0000",
+              color: "#ef2929",
+              weight: 1,
+              fillOpacity: 1,
+              clickable: true
+          });
+          marker.bringToFront();
+          map.panTo(marker.getLatLng());
+        }
+    }
+
+    function onFeatureClick(e) {
+      selectMarker(e.target);
+      var id = features.indexOf(e.target);
+      var galleria = $("#gallery").data("galleria");
+      if (selected != id) {
+          selected = id;
+          galleria.show(id);
+      }
+    }
+
+    function init_map() {
+        /* Make sure we have the map div */
+        if ($("#galleria-map").length == 0) {
+            $(".galleria-stage").after("<div id='galleria-map'></div>");
+            map = L.map('galleria-map');
+            L.tileLayer.provider('{{ settings.leaflet_provider }}').addTo(map);
+            map.setView([51.505, -0.09], 12);
+
+            {% for media in album.medias %}
+              {% if media.exif and media.exif.gps %}
+            var lat = {{ media.exif.gps.lat }};
+            var lon = {{ media.exif.gps.lon }};
+            var marker = L.circleMarker([lat, lon], {
+                radius: 7,
+                fillColor: "#888a85",
+                color: "#2e3436",
+                weight: 1,
+                fillOpacity: 1,
+                clickable: true
+            });
+            marker.on('click', onFeatureClick);
+            features.push(marker);
+            marker.addTo(map);
+              {% else %}
+            features.push(null);
+              {% endif %}
+            {% endfor %}
+
+            selectMarker(features[selected]);
+        }
+    }
+
+    function galleryMapToggle() {
+        /* Make sure the map is loaded */
+        init_map();
+
+        var stage = $(".galleria-stage");
+        var mapdiv = $("#galleria-map");
+        var icons = $(".icons");
+
+        if (stage.attr.hidden == true) {
+            stage.attr.hidden = false;
+            $(".galleria-info-link").show();
+            mapdiv.hide();
+            stage.css("opacity", "1");
+            icons.css("background-color", "inherit");
+            /* Change display of the Map link */
+            $(".gallery-map-toggle img")[0].src = "{{ theme.url }}/img/map.png";
+        } else {
+            stage.attr.hidden = true;
+            $(".galleria-info-link").hide();
+            mapdiv.show();
+            /* Play with the stage opacity otherwise galleria will go mad */
+            stage.css("opacity", "0");
+            icons.css("background-color", "#333");
+            /* Change display of the Map link */
+            $(".gallery-map-toggle img")[0].src = "{{ theme.url }}/img/photo.png";
+        }
+    }
+    {% endif %}
+
+    Galleria.ready(function() {
+        /*
+        load thumbnails after gallery setup, and download has started
+        for the main images, but before the loading of map tiles starts
+        in the background. This ensures visible elements are prioritised
+        over non-visible elements
+
+        The choice to lazy load thumbnails in batches of 50 was not made
+        scientifically. Other numbers may yield better performance.
+        */
+        this.lazyLoadChunks( 50 );
+
+        this.attachKeyboard({
+            right: this.next,
+            left: this.prev,
+            /* Toggle fullscreen display on press of 'f' key */
+            0x46: this.toggleFullscreen,
+            {% if settings.show_map and album.show_map %}
+            /* Toggle map display on press of 'm' key */
+            0x4D: galleryMapToggle,
+            {% endif %}
+        });
+
+        var gallery = this;
+        $('#fullscreen').click(function() {
+          gallery.toggleFullscreen();
+        });
+
+        $('.icons').appendTo(this.$('container'));
+
+        {% if settings.show_map and album.show_map %}
+        $(".gallery-map-toggle").click(galleryMapToggle);
+        this.bind('loadfinish', function(e) {
+            /*  Triggers every time Galleria has finished loading an image.*/
+            if (selected != e.index) {
+                selectMarker(features[e.index]);
+                selected = e.index;
+            }
+        });
+        {% endif %}
+    });
+
+    Galleria.configure({
+        imageCrop: false,
+        transition: "fade",
+        thumbnails: "lazy"
+    });
+    Galleria.run("#gallery", {dataSource: data});
+
+    </script>
+{% endblock %}

Could this be accepted as a change? Maybe the modified template could be called galleria-with-subalbums or something like that?

[saimn]

This duplicates a lot of code which is not ideal for maintenance. So as mentioned above I think a solution would be to use a new template that could be included both in "album_list.html" and "album.html".

[thomasdn]

Please find this pull request with a new subtemplate 'album_items.html' that can be included in album_list.html and album.html.

https://github.com/saimn/sigal/pull/438

[thomasdn]

@saimn you left a few comments on the pull request https://github.com/saimn/sigal/pull/438 to which I replied.

This is the first time I do a pull request on github at all. So I am not too familiar with how this works. So I have a few questions:

1: Did you receive my replies to your comments?

2: What is the next step from here? Will you merge this pull request into the mainline codebase?

3: Is there anything else I need to do in order to move this one forward?

[saimn]

Hi @thomasdn, Sorry for not replying sooner, sigal as a toy project comes after almost everything else ;). If you can just address the change about checking album.medias in the late_js block, would be great, and on my side I will try to test your PR locally soon.

[thomasdn]

Hi @thomasdn, Sorry for not replying sooner, sigal as a toy project comes after almost everything else ;). If you can just address the change about checking album.medias in the late_js block, would be great, and on my side I will try to test your PR locally soon.

Hello,

Here is a patch to album_items.html that will solve the issue with late_js by checking album.medias:

--- album_items.html    2021-07-20 09:49:46.404005093 +0200
+++ venv/lib/python3.7/site-packages/sigal/themes/galleria/templates/album_items.html   2021-08-19 17:23:48.598681719 +0200
@@ -23,6 +23,7 @@
         {% endif %}

 {% block late_js %}
+  {% if album.medias %}
     {% macro img_description(media) -%}
       {%- if media.big -%}<a href='{{ media.big_url }}'>Full size</a>{%- endif -%}
            {# clean up tags and whitespace, including newlines, in the description #}
@@ -228,4 +229,5 @@
     Galleria.run("#gallery", {dataSource: data});

     </script>
+  {% endif %}
 {% endblock %}

I just tested this in my local copy and it seems to work.

[saimn]

Hi @thomasdn, could you add this as a new commit to #438 ?

[thomasdn]

Hi @thomasdn, could you add this as a new commit to #438 ?

I'll try :-)

[thomasdn]

Not sure if this worked?

Did you receive the updated pull request?

https://github.com/saimn/sigal/pull/438

[Lucas-C]

Hi! I'd love to see this feature released :) What's its current status ?

[saimn]

It was done for the galleria theme in #438.