Show a post's image on its card, and show it full size
Some checks are pending
DevPlace CI / test (pull_request) Blocked by required conditions
Some checks are pending
DevPlace CI / test (pull_request) Blocked by required conditions
_attachment_display.html iterates a context variable named `attachments`, so every caller binds it before the include. _post_card.html was the one caller that did not: it guarded on item.attachments but included the partial with nothing bound, so the gallery looped over whatever `attachments` happened to be in the surrounding page context and rendered empty. Every post with an image looked image-less on the feed and on profiles, and on a project page - where project_detail.html sets `attachments` at template scope for the project's own files - a devlog card would have rendered the project's files as its own. With the image actually reaching the card, render a lone one properly: a gallery holding exactly one item gets a `single` class and takes the full content column (max-height 480px, object-fit contain, no hover scale), matching the original DevPlace. That branch serves the stored original rather than thumbnail_url, because a thumbnail is 200px on its longest side and stretching it to the column width is visibly blurry. Animated GIFs needed no change and now have a test proving it: they never had a thumbnail to flatten, so they already took the original-file path and simply render larger. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
08d370b020
commit
8856c38b4d
@ -29,6 +29,23 @@
|
||||
max-height: 200px;
|
||||
}
|
||||
|
||||
.attachment-gallery.single .attachment-gallery-item:has(.gallery-thumb) {
|
||||
width: 100%;
|
||||
max-width: 100%;
|
||||
}
|
||||
|
||||
.attachment-gallery.single .attachment-gallery-item:has(.gallery-thumb):hover {
|
||||
transform: none;
|
||||
border-color: var(--border-light);
|
||||
}
|
||||
|
||||
.attachment-gallery.single .gallery-thumb {
|
||||
width: 100%;
|
||||
max-width: 100%;
|
||||
max-height: 480px;
|
||||
object-fit: contain;
|
||||
}
|
||||
|
||||
.attachment-gallery-item:has(.non-image) {
|
||||
width: 120px;
|
||||
height: 120px;
|
||||
|
||||
@ -92,7 +92,11 @@ File validation: max 5MB, allowed extensions: `.png`, `.jpg`, `.jpeg`, `.gif`, `
|
||||
|
||||
**Ingesting a file from a URL.** `store_attachment_from_url(url, user_uid, filename=None)` (async, in `attachments.py`) is the remote counterpart to `store_attachment`: it downloads the URL on the server through `fetch_remote_file()` - SSRF-guarded (`_guard_public_url` resolves the host and refuses private/loopback/reserved/multicast addresses, mirroring the Devii fetch guard) and size-capped (streams, aborting once `_get_max_upload_bytes()` is exceeded) - resolves a filename from the URL path or the response `Content-Type` (`MIME_TO_EXT`), then calls `store_attachment()` so the bytes land in the **exact same** pipeline (validation, thumbnailing, DB row). It raises `RemoteFetchError(message, status)` which the route maps to an HTTP status. It is exposed at `POST /uploads/upload-url` (`UploadUrlForm{url, filename?}`, `require_user_api`) and as the Devii catalog action `attach_url` (handler `http`, `requires_auth=True`); both return the same record as `/uploads/upload`. The returned `uid` binds to a resource the same way as any upload - via `attachment_uids` at create/edit time - so attaching a remote image is just `attach_url` then `create_post`/`create_project`/etc. with that uid. Do not re-download remote files in a router; reuse this helper so the guard and size cap stay in one place.
|
||||
|
||||
`_row_to_attachment()` / `store_attachment()` expose `is_image` and `is_video` (derived from the mime prefix). The shared partial `templates/_attachment_display.html` branches image -> `<img>`, video -> `<video controls preload="metadata" class="gallery-video">`, else download link; rendering through this one partial is what makes video work across every feature at once. `AttachmentOut` (`schemas.py`) carries both flags - add new display keys there too or JSON drops them.
|
||||
`_row_to_attachment()` / `store_attachment()` expose `is_image` and `is_video` (derived from the mime prefix). The shared partial `templates/_attachment_display.html` branches image -> `<img>`, video -> `<video controls preload="metadata" class="gallery-video">`, else download link; rendering through this one partial is what makes video work across every feature at once.
|
||||
|
||||
**Every caller MUST bind `attachments` before including the partial** - `{% set attachments = item.get('attachments', []) %}` or `{% with attachments=... %}`. The partial iterates the bare name `attachments`, so a caller that only guards on `{% if item.attachments %}` and includes without binding renders the gallery from whatever `attachments` happens to be in the surrounding page context. This is not theoretical: `_post_card.html` did exactly that, so **feed and profile cards silently rendered an empty gallery for every post that had an image**, and on a project page (where `project_detail.html` sets `attachments` at template scope for the project's own files) a devlog card would have rendered the *project's* attachments as if they were the post's. Guarded by `tests/e2e/feed.py::test_feed_card_shows_the_post_image`.
|
||||
|
||||
**A lone attachment is a hero, not a chip.** When the gallery holds exactly one item the partial adds a `single` class, and `attachments.css` widens that item to the full content column (`max-height: 480px`, `object-fit: contain`, no hover scale) instead of the 240x200 chip a multi-item gallery uses. **The `single` branch must serve `att['url']`, never `thumbnail_url`** - a thumbnail is 200px on its longest side, so blowing it up to the column width renders visibly blurry. That is the whole reason the src is a conditional rather than "thumbnail when one exists". Because the partial is shared, this applies everywhere at once: post cards, post detail, comments, gists, projects and chat bubbles. Animated GIFs never had a thumbnail to begin with (`THUMBNAIL_EXTENSIONS` excludes `.gif`, so animation survives), which means they already took the original-file path and simply render larger now. `AttachmentOut` (`schemas.py`) carries both flags - add new display keys there too or JSON drops them.
|
||||
|
||||
Media is served **inline** (not forced-download) for known-safe types only. The set `INLINE_MEDIA_EXTENSIONS` in `main.py` (`UploadStaticFiles`) and the matching `map $uri $upload_disposition` in `nginx/nginx.conf.template` must stay in sync: images/video/audio -> `inline` (so `<video>` plays and seeks via Range), everything else -> `attachment`. SVG is deliberately excluded from both (stored-XSS defense). `ContentRenderer.js` embeds direct video URLs typed into content via `videoExtRe`, mirroring its image handling.
|
||||
|
||||
|
||||
@ -1,10 +1,9 @@
|
||||
<div class="attachment-gallery">
|
||||
{% set _single = attachments|length == 1 %}
|
||||
<div class="attachment-gallery{% if _single %} single{% endif %}">
|
||||
{% for att in attachments %}
|
||||
<div class="attachment-gallery-item">
|
||||
{% if att.get('is_image') and att.get('thumbnail_url') %}
|
||||
<img src="{{ att['thumbnail_url'] }}" alt="{{ att.get('original_filename', '') }}" loading="lazy" class="gallery-thumb" data-lightbox data-full="{{ att['url'] }}" data-mime="{{ att.get('mime_type', '') }}">
|
||||
{% elif att.get('is_image') %}
|
||||
<img src="{{ att['url'] }}" alt="{{ att.get('original_filename', '') }}" loading="lazy" class="gallery-thumb" data-lightbox data-full="{{ att['url'] }}" data-mime="{{ att.get('mime_type', '') }}">
|
||||
{% if att.get('is_image') %}
|
||||
<img src="{{ att['url'] if _single or not att.get('thumbnail_url') else att['thumbnail_url'] }}" alt="{{ att.get('original_filename', '') }}" loading="lazy" class="gallery-thumb" data-lightbox data-full="{{ att['url'] }}" data-mime="{{ att.get('mime_type', '') }}">
|
||||
{% elif att.get('is_video') %}
|
||||
<video src="{{ att['url'] }}" controls preload="metadata" class="gallery-video"></video>
|
||||
{% elif att.get('is_audio') %}
|
||||
|
||||
@ -25,7 +25,8 @@
|
||||
<a href="{{ item.project_link.url }}" class="project-link">Project: {{ item.project_link.name }}</a>
|
||||
{% endif %}
|
||||
|
||||
{% if item.attachments %}
|
||||
{% set attachments = item.get('attachments', []) %}
|
||||
{% if attachments %}
|
||||
{% include "_attachment_display.html" %}
|
||||
{% endif %}
|
||||
|
||||
|
||||
@ -344,6 +344,23 @@ def paste_image(page, selector, name="pasted.png"):
|
||||
)
|
||||
|
||||
|
||||
def create_post_with_files(page, content, files, expected_count=1):
|
||||
from playwright.sync_api import expect
|
||||
|
||||
page.goto(f"{BASE_URL}/feed", wait_until="domcontentloaded")
|
||||
page.locator(".feed-fab").first.wait_for(state="visible", timeout=10000)
|
||||
page.locator(".feed-fab").first.click()
|
||||
page.fill("#post-content", content)
|
||||
page.locator("#create-post-modal dp-upload .dp-upload-input").first.set_input_files(
|
||||
files
|
||||
)
|
||||
expect(
|
||||
page.locator("#create-post-modal dp-upload .dp-upload-count").first
|
||||
).to_have_text(f"({expected_count})", timeout=15000)
|
||||
page.locator("#create-post-modal button.btn-primary:has-text('Post')").click()
|
||||
page.wait_for_url(f"{BASE_URL}/posts/*", wait_until="domcontentloaded")
|
||||
|
||||
|
||||
def assert_share_copies(page, expected_fragment):
|
||||
from playwright.sync_api import expect
|
||||
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
# retoor <retoor@molodetz.nl>
|
||||
|
||||
from tests.conftest import BASE_URL, paste_image
|
||||
from tests.conftest import BASE_URL, create_post_with_files, paste_image
|
||||
import time
|
||||
import requests
|
||||
from playwright.sync_api import expect
|
||||
@ -720,6 +720,28 @@ def test_paste_image_attaches_in_post_composer(alice):
|
||||
).to_have_value(re.compile(r".+"))
|
||||
|
||||
|
||||
def test_feed_card_shows_the_post_image(alice):
|
||||
import io
|
||||
from PIL import Image
|
||||
|
||||
page, _ = alice
|
||||
buf = io.BytesIO()
|
||||
Image.new("RGB", (600, 400), (28, 120, 200)).save(buf, "PNG")
|
||||
create_post_with_files(
|
||||
page,
|
||||
"Feed card image rendering check",
|
||||
[{"name": "card.png", "mimeType": "image/png", "buffer": buf.getvalue()}],
|
||||
)
|
||||
page.goto(f"{BASE_URL}/feed", wait_until="domcontentloaded")
|
||||
card = page.locator(
|
||||
".post-card:has-text('Feed card image rendering check')"
|
||||
).first
|
||||
card.wait_for(state="visible", timeout=10000)
|
||||
image = card.locator(".attachment-gallery.single .gallery-thumb").first
|
||||
image.wait_for(state="visible", timeout=10000)
|
||||
assert "_thumb" not in image.get_attribute("src")
|
||||
|
||||
|
||||
def test_create_post_cancel_modal(alice):
|
||||
page, _ = alice
|
||||
page.goto(f"{BASE_URL}/feed", wait_until="domcontentloaded")
|
||||
|
||||
@ -2,7 +2,12 @@
|
||||
|
||||
import re
|
||||
from playwright.sync_api import expect
|
||||
from tests.conftest import BASE_URL, assert_share_copies, paste_image
|
||||
from tests.conftest import (
|
||||
BASE_URL,
|
||||
assert_share_copies,
|
||||
create_post_with_files,
|
||||
paste_image,
|
||||
)
|
||||
def create_post(page, topic="random", content="Test post content", title=None):
|
||||
page.goto(f"{BASE_URL}/feed", wait_until="domcontentloaded")
|
||||
page.locator(".feed-fab").first.wait_for(state="visible", timeout=10000)
|
||||
@ -13,6 +18,27 @@ def create_post(page, topic="random", content="Test post content", title=None):
|
||||
page.fill("#post-title", title)
|
||||
page.locator("#create-post-modal button.btn-primary:has-text('Post')").click()
|
||||
page.wait_for_url(f"{BASE_URL}/posts/*", wait_until="domcontentloaded")
|
||||
def _png_bytes(color=(0, 128, 255), size=(600, 400)):
|
||||
import io
|
||||
from PIL import Image
|
||||
|
||||
buf = io.BytesIO()
|
||||
Image.new("RGB", size, color).save(buf, "PNG")
|
||||
return buf.getvalue()
|
||||
|
||||
|
||||
def _gif_bytes():
|
||||
import io
|
||||
from PIL import Image
|
||||
|
||||
frames = [Image.new("RGB", (40, 40), c) for c in ((255, 0, 0), (0, 0, 255))]
|
||||
buf = io.BytesIO()
|
||||
frames[0].save(
|
||||
buf, "GIF", save_all=True, append_images=frames[1:], duration=120, loop=0
|
||||
)
|
||||
return buf.getvalue()
|
||||
|
||||
|
||||
def _profile_stars(page, username):
|
||||
page.goto(f"{BASE_URL}/profile/{username}", wait_until="domcontentloaded")
|
||||
value = page.locator(
|
||||
@ -390,6 +416,57 @@ def test_paste_image_attaches_in_comment_form(alice):
|
||||
).to_have_value(re.compile(r".+"))
|
||||
|
||||
|
||||
def test_single_image_post_shows_the_original_full_size(alice):
|
||||
page, _ = alice
|
||||
create_post_with_files(
|
||||
page,
|
||||
"Post carrying exactly one image",
|
||||
[{"name": "shot.png", "mimeType": "image/png", "buffer": _png_bytes()}],
|
||||
)
|
||||
gallery = page.locator(".attachment-gallery.single")
|
||||
gallery.wait_for(state="visible", timeout=10000)
|
||||
img = gallery.locator(".gallery-thumb").first
|
||||
src = img.get_attribute("src")
|
||||
assert "_thumb" not in src, f"hero image served the 200px thumbnail: {src}"
|
||||
assert src == img.get_attribute("data-full")
|
||||
|
||||
|
||||
def test_multiple_image_post_keeps_thumbnails(alice):
|
||||
page, _ = alice
|
||||
create_post_with_files(
|
||||
page,
|
||||
"Post carrying two images",
|
||||
[
|
||||
{"name": "one.png", "mimeType": "image/png", "buffer": _png_bytes()},
|
||||
{
|
||||
"name": "two.png",
|
||||
"mimeType": "image/png",
|
||||
"buffer": _png_bytes((200, 30, 90)),
|
||||
},
|
||||
],
|
||||
expected_count=2,
|
||||
)
|
||||
page.locator(".attachment-gallery").first.wait_for(state="visible", timeout=10000)
|
||||
assert page.locator(".attachment-gallery.single").count() == 0
|
||||
thumbs = page.locator(".attachment-gallery .gallery-thumb")
|
||||
assert thumbs.count() == 2
|
||||
for i in range(thumbs.count()):
|
||||
assert "_thumb" in thumbs.nth(i).get_attribute("src")
|
||||
|
||||
|
||||
def test_animated_gif_post_serves_the_original_file(alice):
|
||||
page, _ = alice
|
||||
create_post_with_files(
|
||||
page,
|
||||
"Post carrying an animated gif",
|
||||
[{"name": "loop.gif", "mimeType": "image/gif", "buffer": _gif_bytes()}],
|
||||
)
|
||||
img = page.locator(".attachment-gallery .gallery-thumb").first
|
||||
img.wait_for(state="visible", timeout=10000)
|
||||
src = img.get_attribute("src")
|
||||
assert src.endswith(".gif"), f"animation lost, served {src}"
|
||||
|
||||
|
||||
def test_attachment_upload_ui(alice):
|
||||
import io
|
||||
from PIL import Image
|
||||
|
||||
Loading…
Reference in New Issue
Block a user