Skip to content

Commit

Permalink
fix: Reduce number of thumbnails created for admin, avoid admin thumb…
Browse files Browse the repository at this point in the history
…nails for svg files (#1490)

* Fix #1377

* Simplify admin thumbnails sizes to 40, 80, 160. No thumbnails for svg in the admin.

* Update settings

* Fix undefined variable add_attrs

* Dynamic css for new setting `FILER_THUMBNAIL_ICON_SIZE`

* Replace SVG by icon if larger than 1MB (can be changed by setting)

* Dynamic folder icon size

* Default sizes 40, and 120px

* Update folderadmin.py for better naming
  • Loading branch information
fsbraun authored Aug 22, 2024
1 parent 01702d8 commit f919cbf
Show file tree
Hide file tree
Showing 16 changed files with 89 additions and 54 deletions.
15 changes: 6 additions & 9 deletions filer/admin/clipboardadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
from django.core.exceptions import ValidationError
from django.forms.models import modelform_factory
from django.http import JsonResponse
from django.urls import path
from django.urls import path, reverse
from django.utils.translation import gettext_lazy as _
from django.views.decorators.csrf import csrf_exempt

from .. import settings as filer_settings
from ..models import Clipboard, ClipboardItem, Folder
from ..settings import FILER_THUMBNAIL_ICON_SIZE
from ..utils.files import handle_request_files_upload, handle_upload
from ..utils.loader import load_model
from ..validation import validate_upload
Expand Down Expand Up @@ -141,14 +142,10 @@ def ajax_upload(request, folder_id=None):
}
# prepare preview thumbnail
if isinstance(file_obj, Image):
thumbnail_180_options = {
'size': (180, 180),
'crop': True,
'upscale': True,
}
thumbnail_180 = file_obj.file.get_thumbnail(
thumbnail_180_options)
data['thumbnail_180'] = thumbnail_180.url
data['thumbnail_180'] = reverse(
f"admin:filer_{file_obj._meta.model_name}_fileicon",
args=(file_obj.pk, FILER_THUMBNAIL_ICON_SIZE),
)
data['original_image'] = file_obj.url
return JsonResponse(data)
except Exception as error:
Expand Down
15 changes: 10 additions & 5 deletions filer/admin/folderadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
from .. import settings
from ..cache import clear_folder_permission_cache
from ..models import File, Folder, FolderPermission, FolderRoot, ImagesWithMissingData, UnsortedImages, tools
from ..settings import FILER_IMAGE_MODEL, FILER_PAGINATE_BY, TABLE_LIST_TYPE
from ..settings import (
FILER_IMAGE_MODEL, FILER_PAGINATE_BY, FILER_TABLE_ICON_SIZE, FILER_THUMBNAIL_ICON_SIZE, TABLE_LIST_TYPE,
)
from ..thumbnail_processors import normalize_subject_location
from ..utils.compatibility import get_delete_permission
from ..utils.filer_easy_thumbnails import FilerActionThumbnailer
Expand Down Expand Up @@ -271,11 +273,13 @@ def directory_listing(self, request, folder_id=None, viewtype=None):

list_type = get_directory_listing_type(request) or settings.FILER_FOLDER_ADMIN_DEFAULT_LIST_TYPE
if list_type == TABLE_LIST_TYPE:
size = "40x40" # Prefetch thumbnails for listing
size_x2 = "80x80"
# Prefetch thumbnails for table view
size = f"{FILER_TABLE_ICON_SIZE}x{FILER_TABLE_ICON_SIZE}"
size_x2 = f"{2 * FILER_TABLE_ICON_SIZE}x{2 * FILER_TABLE_ICON_SIZE}"
else:
size = "160x160" # Prefetch thumbnails for thumbnail view
size_x2 = "320x320"
# Prefetch thumbnails for thumbnail view
size = f"{FILER_THUMBNAIL_ICON_SIZE}x{FILER_THUMBNAIL_ICON_SIZE}"
size_x2 = f"{2 * FILER_THUMBNAIL_ICON_SIZE}x{2 * FILER_THUMBNAIL_ICON_SIZE}"

# Check actions to see if any are available on this changelist
actions = self.get_actions(request)
Expand Down Expand Up @@ -467,6 +471,7 @@ def directory_listing(self, request, folder_id=None, viewtype=None):
'show_result_count': show_result_count,
'folder_children': folder_qs,
'folder_files': file_qs,
'thumbnail_size': FILER_TABLE_ICON_SIZE if list_type == TABLE_LIST_TYPE else FILER_THUMBNAIL_ICON_SIZE,
'limit_search_to_folder': limit_search_to_folder,
'is_popup': popup_status(request),
'filer_admin_context': AdminContext(request),
Expand Down
3 changes: 1 addition & 2 deletions filer/cache.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import typing

from django.core.cache import cache

from django.contrib.auth import get_user_model
from django.core.cache import cache


User = get_user_model()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# Generated by Django 3.2.25 on 2024-08-19 14:49

import django.db.models.deletion
from django.conf import settings
from django.db import migrations, models
import django.db.models.deletion

import filer.fields.multistorage_file
import filer.models.filemodels
import filer.models.mixins
Expand Down
4 changes: 2 additions & 2 deletions filer/models/abstract.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import logging

from django.conf import settings
from django.core.checks import Warning, register as register_check
from django.core.checks import Warning
from django.core.checks import register as register_check
from django.core.exceptions import ValidationError
from django.db import models
from django.utils.functional import cached_property
from django.utils.translation import gettext_lazy as _


import easy_thumbnails.utils
from easy_thumbnails.VIL import Image as VILImage
from PIL.Image import MAX_IMAGE_PIXELS
Expand Down
2 changes: 1 addition & 1 deletion filer/models/foldermodels.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
from django.utils.translation import gettext_lazy as _

from .. import settings as filer_settings
from . import mixins
from ..cache import get_folder_permission_cache, update_folder_permission_cache
from . import mixins


class FolderPermissionManager(models.Manager):
Expand Down
16 changes: 6 additions & 10 deletions filer/private/sass/components/_navigator.scss
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ body {
// removes padding to make sure that column has correct height #664
padding-top: 0 !important;
padding-bottom: 0 !important;
img {
width: 40px;
height: auto;
}
}
.column-action {
text-align: center;
Expand Down Expand Up @@ -732,17 +728,17 @@ body {
float: left;
display: inline-block;
padding: 10px;
width: 125px;
height: 125px;
width: calc(var(--thumbnail-size, 120px) + 5px);
height: calc(var(--thumbnail-size, 120px) + 5px);
border: 1px solid $gray-lighter;
margin: 16px 12px;
background-color: $white;
position: relative;
overflow: hidden;
.thumbnail-file-item-box {
padding: 10px;
width: 125px;
height: 125px;
width: calc(var(--thumbnail-size, 120px) + 5px);
height: calc(var(--thumbnail-size, 120px) + 5px);
border: 1px solid $gray-lighter;
margin: 16px 12px;
background-color: $white;
Expand Down Expand Up @@ -791,8 +787,8 @@ body {
}
.thumbnail-file-item {
float: none;
width: 147px;
height: 200px;
width: calc(var(--thumbnail-size, 120px) + 27px);
height: calc(var(--thumbnail-size, 120px) + 80px);
border: 0;
padding: 0;
background-color: transparent;
Expand Down
28 changes: 17 additions & 11 deletions filer/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import os

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.utils.module_loading import import_string as get_storage_class
from django.utils.translation import gettext_lazy as _

Expand Down Expand Up @@ -38,20 +37,24 @@

FILER_PAGINATE_BY = getattr(settings, 'FILER_PAGINATE_BY', 100)

if hasattr(settings, "FILER_ADMIN_ICON_SIZES"):
logger.warning("FILER_ADMIN_ICON_SIZES is deprecated and will be removed in the future.")

_ICON_SIZES = getattr(settings, 'FILER_ADMIN_ICON_SIZES', ('16', '32', '48', '64'))
if not _ICON_SIZES:
raise ImproperlyConfigured('Please, configure FILER_ADMIN_ICON_SIZES')
# Reliably sort by integer value, but keep icon size as string.
# (There is some code in the wild that depends on this being strings.)
FILER_ADMIN_ICON_SIZES = [str(i) for i in sorted([int(s) for s in _ICON_SIZES])]

# Filer admin templates have specific icon sizes hardcoded: 32 and 48.
_ESSENTIAL_ICON_SIZES = ('32', '48')
if not all(x in FILER_ADMIN_ICON_SIZES for x in _ESSENTIAL_ICON_SIZES):
logger.warn(
"FILER_ADMIN_ICON_SIZES has not all of the essential icon sizes "
"listed: {}. Some icons might be missing in admin templates.".format(
_ESSENTIAL_ICON_SIZES))
# Currently, these two icon sizes are hard-coded into the admin and admin templates
FILER_TABLE_ICON_SIZE = getattr(settings, "FILER_TABLE_ICON_SIZE", 40)
FILER_THUMBNAIL_ICON_SIZE = getattr(settings, "FILER_THUMBNAIL_ICON_SIZE", 120)
DEFERRED_THUMBNAIL_SIZES = (
FILER_TABLE_ICON_SIZE,
2 * FILER_TABLE_ICON_SIZE,
FILER_THUMBNAIL_ICON_SIZE,
2 * FILER_THUMBNAIL_ICON_SIZE,
)


# This is an ordered iterable that describes a list of
# classes that I should check for when adding files
Expand Down Expand Up @@ -283,7 +286,6 @@ def update_server_settings(settings, defaults, s, t):
},
}

DEFERRED_THUMBNAIL_SIZES = (40, 80, 160)
IMAGE_EXTENSIONS = ['.jpg', '.jpeg', '.png', '.gif', '.webp']
IMAGE_MIME_TYPES = ['gif', 'jpeg', 'png', 'x-png', 'svg+xml', 'webp']

Expand Down Expand Up @@ -322,3 +324,7 @@ def update_server_settings(settings, defaults, s, t):
except (ModuleNotFoundError, ImportError):
# Import error? No django CMS used: stay with own icons
pass


# SVG are their own thumbnails if their size is below this limit
FILER_MAX_SVG_THUMBNAIL_SIZE = getattr(settings, "FILER_MAX_SVG_THUMBNAIL_SIZE", 1024 * 1024) # 1MB default
2 changes: 1 addition & 1 deletion filer/static/filer/css/admin_filer.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion filer/static/filer/css/maps/admin_filer.css.map

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
<td class="column-icon">
<a href="{{ subfolder.get_admin_directory_listing_url_path }}{% filer_admin_context_url_params %}"
title="{% blocktrans with subfolder.name as item_label %}Change '{{ item_label }}' folder details{% endblocktrans %}">
<img src="{% static 'filer/icons/folder.svg' %}" alt="{% translate 'Folder Icon' %}" width="28" height="28">
<img src="{% static 'filer/icons/folder.svg' %}" alt="{% translate 'Folder Icon' %}" width="{% widthratio 7 10 thumbnail_size %}" height="{% widthratio 7 10 thumbnail_size %}" style="height: {{ thumbnail_size }}px;">
</a>
</td>
<td class="column-name">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% load i18n l10n admin_list filer_tags filer_admin_tags static thumbnail %}
<div class="drag-hover-border thumbnail-drag-hover-border"></div>
<section class="navigator{% if is_popup %} navigator-popup{% endif %} navigator-thumbnail-list">
<section class="navigator{% if is_popup %} navigator-popup{% endif %} navigator-thumbnail-list" style="--thumbnail-size: {{ thumbnail_size }}px;">

<div class="js-filer-dropzone js-filer-dropzone-base navigator-list" id="result_list" data-url="{% if folder.id %}{% url 'admin:filer-ajax_upload' folder_id=folder.id %}{% else %}{% url 'admin:filer-ajax_upload' %}{% endif %}" data-folder-name="{% if folder.is_root %}{% translate 'Unsorted Uploads' %}{% else %}{{ folder.name }}{% endif %}" data-max-uploader-connections="{{ uploader_connections }}"
data-max-files="{{ max_files|safe }}"
Expand Down Expand Up @@ -106,7 +106,7 @@
<a href="{{ file.get_admin_change_url }}{% filer_admin_context_url_params %}"
title="{% blocktrans with file.label as item_label %}Change '{{ item_label }}' details{% endblocktrans %}">
{% endif %}
{% file_icon file size="160x160" %}
{% file_icon file detail="thumbnail" %}
{% if has_change_permission or is_popup and filer_admin_context.pick_file %}
</a>
{% endif %}
Expand Down
15 changes: 13 additions & 2 deletions filer/templates/admin/filer/templatetags/file_icon.html
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,21 @@
{% translate 'Your browser does not support video.' %}
</video>
{% else %}
<img src="{{ icon_url }}" loading="lazy" width="{{ width }}" height="{{ height }}"{% if alt_text %} alt="{{ alt_text }}"{% endif %}{% if highres_url %} srcset="{{ icon_url }} 1x, {{ highres_url }} 2x"{% endif %}{% if sidebar_image_ratio %} data-ratio="{{ sidebar_image_ratio }}" class="js-focal-point-image"{% endif %} />
<img src="{{ icon_url }}"
loading="lazy" width="{{ width }}" height="{{ height }}"
{% if alt_text %} alt="{{ alt_text }}"{% endif %}
{% if highres_url %} srcset="{{ icon_url }} 1x, {{ highres_url }} 2x"{% endif %}
{% if sidebar_image_ratio %} data-ratio="{{ sidebar_image_ratio }}"
class="js-focal-point-image"{% endif %}
{{ add_styles|safe }}
/>
{% endif %}
{% else %}
<img src="{{ icon_url }}" loading="lazy" width="{{ width }}" height="{{ height }}"{% if alt_text %} alt="{{ alt_text }}"{% endif %}{% if highres_url %} srcset="{{ icon_url }} 1x, {{ highres_url }} 2x"{% endif %} class="thumbnail_img" />
<img src="{{ icon_url }}" loading="lazy" width="{{ width }}" height="{{ height }}"
{% if alt_text %} alt="{{ alt_text }}"{% endif %}
{% if highres_url %} srcset="{{ icon_url }} 1x, {{ highres_url }} 2x"{% endif %} class="thumbnail_img"
{{ add_styles|safe }}
/>
{% endif %}
{% if sidebar_image_ratio %}
<div class="image-preview-field">
Expand Down
2 changes: 1 addition & 1 deletion filer/templates/admin/filer/widgets/admin_file.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<span class="filerFile js-file-selector">
{% if object %}
{% if object.file.exists %}
<a href="{{ object.url }}" target="_blank">{% file_icon object size='80x80' %}</a>
<a href="{{ object.url }}" target="_blank">{% file_icon object detail=True %}</a>
&nbsp;<span class="description_text">{{ object.label }}</span>
{% else %}
{% file_icon object %}
Expand Down
28 changes: 24 additions & 4 deletions filer/templatetags/filer_admin_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
from filer import settings
from filer.admin.tools import admin_url_params, admin_url_params_encoded
from filer.models.imagemodels import BaseImage
from filer.settings import DEFERRED_THUMBNAIL_SIZES
from filer.settings import (
DEFERRED_THUMBNAIL_SIZES, FILER_MAX_SVG_THUMBNAIL_SIZE, FILER_TABLE_ICON_SIZE, FILER_THUMBNAIL_ICON_SIZE,
)


register = Library()
Expand Down Expand Up @@ -113,6 +115,7 @@ def file_icon_context(file, detail, width, height):
if file.file and isinstance(file.file.source_storage, FileSystemStorage) and not file.file.exists():
return not_available_context

add_styles = {}
if isinstance(file, BaseImage):
thumbnailer = get_thumbnailer(file)

Expand All @@ -135,6 +138,16 @@ def file_icon_context(file, detail, width, height):
icon_url = file.file.thumbnail_storage.url(configured_name)
if mime_subtype != 'svg+xml' and file.thumbnailx2_name:
context['highres_url'] = file.file.thumbnail_storage.url(file.thumbnailx2_name)
elif mime_subtype == 'svg+xml':
if file.size < FILER_MAX_SVG_THUMBNAIL_SIZE:
icon_url = file.url
else:
# Only display a generic image icon
# Better solution: Render a bitmap thumbnail from SVG
icon_url = staticfiles_storage.url('filer/icons/file-picture.svg')
add_styles = {
"object-fit": "cover", # Mimic the behavior of the thumbnail
}
else: # Probably does not exist, defer creation
icon_url = reverse("admin:filer_file_fileicon", args=(file.pk, width))
context['alt_text'] = file.default_alt_text
Expand All @@ -161,6 +174,9 @@ def file_icon_context(file, detail, width, height):
icon_url = staticfiles_storage.url('filer/icons/file-unknown.svg')
height = width # icon is a square
context.update(width=width, height=height, icon_url=icon_url)
if add_styles:
styles = "; ".join(f"{k}: {v}" for k, v in add_styles.items())
context.update(add_styles=f'style="{styles}"')
return context


Expand All @@ -187,16 +203,20 @@ def file_icon(file, detail=False, size=None):
"""
if size:
width, height = (int(s) for s in size.split('x'))
elif detail == "thumbnail":
width, height = FILER_THUMBNAIL_ICON_SIZE, FILER_THUMBNAIL_ICON_SIZE
elif detail is True:
width, height = 2 * FILER_TABLE_ICON_SIZE, 2 * FILER_TABLE_ICON_SIZE
else:
width, height = (75, 75) if detail else (40, 40)
return file_icon_context(file, detail, width, height)
width, height = FILER_TABLE_ICON_SIZE, FILER_TABLE_ICON_SIZE
return file_icon_context(file, detail is True, width, height)


@register.simple_tag
def file_icon_url(file):
# Cache since it is called repeatedly by templates
if not hasattr(file, "_file_icon_url_cache"):
context = file_icon_context(file, False, 80, 80)
context = file_icon_context(file, False, 2 * FILER_TABLE_ICON_SIZE, 2 * FILER_TABLE_ICON_SIZE)
file._file_icon_url_cache = escapejs(context.get('highres_url', context['icon_url']))
return file._file_icon_url_cache

Expand Down
2 changes: 1 addition & 1 deletion tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ def tearDown(self):
def test_icon_view_sizes(self):
"""Redirects are issued for accepted thumbnail sizes and 404 otherwise"""
test_set = tuple((size, 302) for size in DEFERRED_THUMBNAIL_SIZES)
test_set += (50, 404), (90, 404), (320, 404)
test_set += (50, 404), (90, 404), (640, 404)
for size, expected_status in test_set:
url = reverse('admin:filer_file_fileicon', kwargs={
'file_id': self.file_object.pk,
Expand Down

0 comments on commit f919cbf

Please sign in to comment.