Browse Source

bug-fixes

gitcitadel
Silberengel 2 weeks ago
parent
commit
d81dbed032
  1. 76
      assets/controllers/article_comments_controller.js
  2. 46
      assets/styles/app.css
  3. 7
      assets/styles/article.css
  4. 30
      src/Controller/ArticleController.php
  5. 8
      src/Service/MagazineContentService.php
  6. 7
      src/Service/NostrClient.php
  7. 10
      src/Service/NostrRelayFanoutTransport.php
  8. 6
      src/Service/NostrRelayRequestFactory.php
  9. 4
      src/Twig/ArticleCardCoverExtension.php
  10. 3
      templates/components/Molecules/Card.html.twig
  11. 3
      templates/components/Organisms/Comments.html.twig
  12. 3
      templates/components/Organisms/FeaturedList.html.twig
  13. 3
      templates/components/Organisms/FeaturedWall.html.twig
  14. 3
      templates/components/Organisms/HomeMagazineArticleStrip.html.twig

76
assets/controllers/article_comments_controller.js

@ -1,7 +1,16 @@
import { Controller } from '@hotwired/stimulus'; import { Controller } from '@hotwired/stimulus';
/** /**
* Fetches the comment thread HTML after the article shell has rendered (no relay I/O on first paint). * Two-phase comment loading:
*
* Phase 1 fires ?cached=1 immediately, shows whatever is in the server-side cache
* without touching any Nostr relays (< 100 ms on a warm cache).
*
* Phase 2 fires the full URL in parallel, which does relay I/O. When it resolves
* it replaces the Phase-1 content. If Phase 2 finishes first (e.g. the
* cached response was held up by DNS), Phase 1 is silently discarded.
*
* Result: readers always see something quickly; fresh relay data appears when ready.
*/ */
export default class extends Controller { export default class extends Controller {
static values = { static values = {
@ -13,8 +22,7 @@ export default class extends Controller {
connect() { connect() {
this.partialReloads = 0; this.partialReloads = 0;
// Stable reference across reconnects: rebinding each connect() would strand old listeners // Stable reference across reconnects: rebinding each connect() would strand old listeners.
// because removeEventListener must use the same function reference that was passed to add.
this.boundOnAuth ??= this.onAuthChanged.bind(this); this.boundOnAuth ??= this.onAuthChanged.bind(this);
window.removeEventListener('unfold:auth-changed', this.boundOnAuth); window.removeEventListener('unfold:auth-changed', this.boundOnAuth);
window.addEventListener('unfold:auth-changed', this.boundOnAuth); window.addEventListener('unfold:auth-changed', this.boundOnAuth);
@ -22,9 +30,8 @@ export default class extends Controller {
return; return;
} }
if (this.preloadedValue) { if (this.preloadedValue) {
// Article SSR already included comments. Do not re-fetch: a slow or dropped // Article SSR already included comments (cache hit at render time). Do not re-fetch;
// request would replace working HTML with a generic error. Re-fetch on auth // a slow relay request would only replace working HTML. Auth changes may still reload.
// only (reply UI may need fresh permission state).
return; return;
} }
void this.load(); void this.load();
@ -43,13 +50,23 @@ export default class extends Controller {
void this.load(); void this.load();
} }
buildFetchUrl() { /** Append ?cb=<timestamp> (and optional extras) to bust HTTP caches. */
buildFetchUrl(extra = '') {
const u = this.urlValue; const u = this.urlValue;
const bust = `cb=${Date.now()}`; const parts = [`cb=${Date.now()}`, extra].filter(Boolean);
return u.includes('?') ? `${u}&${bust}` : `${u}?${bust}`; const qs = parts.join('&');
return u.includes('?') ? `${u}&${qs}` : `${u}?${qs}`;
} }
async load() { async load() {
// Track whether Phase 2 has already written to the DOM so Phase 1 never clobbers it.
this._fullFetchDone = false;
this.partialReloads = 0;
// Phase 1: fire a cache-only request in the background — completes in < 100 ms.
void this._showCachedVersion();
// Phase 2: full relay fetch — replaces Phase 1 output when it resolves.
const t0 = performance.now(); const t0 = performance.now();
const perAttemptMs = 45_000; const perAttemptMs = 45_000;
const maxAttempts = 3; const maxAttempts = 3;
@ -57,7 +74,6 @@ export default class extends Controller {
const controller = new AbortController(); const controller = new AbortController();
const timer = window.setTimeout(() => controller.abort(), perAttemptMs); const timer = window.setTimeout(() => controller.abort(), perAttemptMs);
try { try {
// Avoid a stale 60s-cached "guest" fragment right after login (see comments fragment headers).
const res = await fetch(this.buildFetchUrl(), { const res = await fetch(this.buildFetchUrl(), {
signal: controller.signal, signal: controller.signal,
cache: 'no-store', cache: 'no-store',
@ -68,10 +84,11 @@ export default class extends Controller {
throw new Error(`HTTP ${res.status}`); throw new Error(`HTTP ${res.status}`);
} }
const html = await res.text(); const html = await res.text();
if (!this.hasContainerTarget) {
window.clearTimeout(timer); window.clearTimeout(timer);
if (!this.hasContainerTarget) {
return; return;
} }
this._fullFetchDone = true;
this.containerTarget.innerHTML = html; this.containerTarget.innerHTML = html;
const isPartial = /data-comments-partial="1"/.test(html); const isPartial = /data-comments-partial="1"/.test(html);
if (isPartial && this.partialReloads < 2) { if (isPartial && this.partialReloads < 2) {
@ -83,12 +100,10 @@ export default class extends Controller {
}, 1200); }, 1200);
} }
const ms = Math.round(performance.now() - t0); const ms = Math.round(performance.now() - t0);
if (attempt > 1) { console.debug(
console.debug(`[article-comments] fragment OK in ${ms}ms (after ${attempt} attempts)`, this.urlValue); `[article-comments] relay fetch OK in ${ms}ms${attempt > 1 ? ` (attempt ${attempt})` : ''}`,
} else { this.urlValue,
console.debug(`[article-comments] fragment OK in ${ms}ms`, this.urlValue); );
}
window.clearTimeout(timer);
return; return;
} catch (err) { } catch (err) {
window.clearTimeout(timer); window.clearTimeout(timer);
@ -101,12 +116,35 @@ export default class extends Controller {
continue; continue;
} }
const ms = Math.round(performance.now() - t0); const ms = Math.round(performance.now() - t0);
console.warn(`[article-comments] fragment failed after ${ms}ms`, this.urlValue, err); console.warn(`[article-comments] relay fetch failed after ${ms}ms`, this.urlValue, err);
if (this.hasContainerTarget) { // Only show the error if Phase 1 hasn't already displayed something useful.
if (this.hasContainerTarget && !this._fullFetchDone) {
this.containerTarget.innerHTML = this.containerTarget.innerHTML =
'<p class="text-subtle">Comments could not be loaded.</p>'; '<p class="text-subtle">Comments could not be loaded.</p>';
} }
} }
} }
} }
/** Phase 1: return the server's cached copy immediately, without doing any relay I/O. */
async _showCachedVersion() {
try {
const res = await fetch(this.buildFetchUrl('cached=1'), {
cache: 'no-store',
credentials: 'same-origin',
headers: { Accept: 'text/html', 'X-Requested-With': 'XMLHttpRequest' },
});
if (!res.ok || !this.hasContainerTarget || this._fullFetchDone) {
return;
}
const html = await res.text();
// Re-check: Phase 2 may have landed while we were awaiting the body.
if (!this.hasContainerTarget || this._fullFetchDone) {
return;
}
this.containerTarget.innerHTML = html;
} catch {
// Ignore; Phase 2 will fill the container regardless.
}
}
} }

46
assets/styles/app.css

@ -451,17 +451,17 @@ svg.icon {
transform: scale(1.06); transform: scale(1.06);
} }
.featured-list--picture-grid .featured-tile__picture-img[src*="favicon-96x96"] { /* Suppress hover zoom when using the branded fallback — the faded portrait shouldn't animate */
object-fit: contain; .featured-list--picture-grid .featured-tile--picture-block:has(.card-header--no-cover) .featured-tile__picture-img {
padding: 2rem;
box-sizing: border-box;
transform: none; transform: none;
transition: none;
} }
.featured-list--picture-grid .featured-tile__picture-scrim { .featured-list--picture-grid .featured-tile__picture-scrim {
position: absolute; position: absolute;
inset: 0; inset: 0;
pointer-events: none; pointer-events: none;
z-index: 2;
background: linear-gradient( background: linear-gradient(
to top, to top,
color-mix(in srgb, #0a0a0a 88%, transparent) 0%, color-mix(in srgb, #0a0a0a 88%, transparent) 0%,
@ -476,7 +476,7 @@ svg.icon {
right: 0; right: 0;
bottom: 0; bottom: 0;
padding: 0.65rem 0.85rem 0.75rem; padding: 0.65rem 0.85rem 0.75rem;
z-index: 1; z-index: 3;
display: flex; display: flex;
flex-direction: column; flex-direction: column;
gap: 0.28rem; gap: 0.28rem;
@ -736,12 +736,36 @@ svg.icon {
text-underline-offset: 2px; text-underline-offset: 2px;
} }
/* List cards: same site-logo treatment when the hero is the default mark */ /* Fallback hero: portrait painting at low opacity with a diagonal stripe overlay.
.article-list .card-header img[src*="favicon-96x96"] { Applied to any card container (.card-header, .featured-tile__media, etc.) when
object-fit: contain; the article has no cover image. The painting is cropped to the face same
padding: 1.25rem; object-position as .header__logo-banner. */
box-sizing: border-box; .card-header--no-cover {
background: var(--color-bg-light); position: relative;
overflow: hidden;
background-color: var(--color-bg-light);
}
.card-header--no-cover img {
opacity: 0.18;
object-fit: cover;
object-position: 50% 14%;
}
.card-header--no-cover::after {
content: '';
position: absolute;
inset: 0;
background:
repeating-linear-gradient(
-45deg,
transparent 0,
transparent 5px,
rgba(0, 0, 0, 0.028) 5px,
rgba(0, 0, 0, 0.028) 6px
);
pointer-events: none;
z-index: 1;
} }
/* Optional category label above cover (see Molecules/Card) */ /* Optional category label above cover (see Molecules/Card) */

7
assets/styles/article.css

@ -268,6 +268,13 @@
gap: 0.55rem; gap: 0.55rem;
} }
/* Empty-state label when the relay fetch returned zero comments */
.comments__empty {
font-size: 0.9rem;
color: var(--color-text-mid);
margin: 0.5rem 0;
}
.comments .card.comment, .comments .card.comment,
.comments-quotes__list .card.comment { .comments-quotes__list .card.comment {
margin-left: 0; margin-left: 0;

30
src/Controller/ArticleController.php

@ -61,16 +61,36 @@ class ArticleController extends AbstractController
$articleTitle = substr($articleTitle, 0, 200); $articleTitle = substr($articleTitle, 0, 200);
} }
$headers = [
'Content-Type' => 'text/html; charset=UTF-8',
'Cache-Control' => 'private, no-store',
];
// Phase-1 fast path: return whatever is in the filesystem cache without touching relays.
// The JS fires this in parallel with the full relay request so readers see cached comments
// immediately (< 100 ms) while the relay fetch continues in the background.
if ($request->query->getBoolean('cached')) {
$cached = $loader->tryLoadFromCacheOnly($coordinate, $articleEventId);
if ($cached === null) {
// Cache miss — return an empty shell; the full relay fetch is already in flight.
// The article template already shows "Loading comments…" as the initial DOM state,
// so there is no need to repeat it here.
return new Response('<div class="comments" data-comments-partial="1"></div>', Response::HTTP_OK, $headers);
}
try {
$data = $this->enrichCommentDataWithReplyContext($cached, $coordinate, $articleEventId, $articleTitle);
return $this->render('components/Organisms/Comments.html.twig', $data, new Response('', Response::HTTP_OK, $headers));
} catch (\Throwable) {
return new Response('<div class="comments"></div>', Response::HTTP_OK, $headers);
}
}
$logger->info('http.fragment.comments_start', [ $logger->info('http.fragment.comments_start', [
'coordinate' => $coordinate, 'coordinate' => $coordinate,
'article_event_hex' => $articleEventId, 'article_event_hex' => $articleEventId,
]); ]);
$headers = [
'Content-Type' => 'text/html; charset=UTF-8',
'Cache-Control' => 'private, max-age=60',
];
try { try {
$data = $loader->load($coordinate, $articleEventId); $data = $loader->load($coordinate, $articleEventId);
$data = $this->enrichCommentDataWithReplyContext( $data = $this->enrichCommentDataWithReplyContext(

8
src/Service/MagazineContentService.php

@ -200,7 +200,11 @@ final class MagazineContentService
continue; continue;
} }
$parts = explode(':', (string) $seq[1], 3); $parts = explode(':', (string) $seq[1], 3);
if (\count($parts) < 2) { if (\count($parts) < 3) {
continue;
}
// Only longform article authors are featured authors; skip sub-index (30040) references.
if (!\in_array((int) $parts[0], KindsEnum::longformKindValues(), true)) {
continue; continue;
} }
$pk = strtolower((string) $parts[1]); $pk = strtolower((string) $parts[1]);
@ -370,7 +374,7 @@ final class MagazineContentService
* missing_total: int, * missing_total: int,
* entries: list<array{ * entries: list<array{
* coordinate: string, * coordinate: string,
* status: 'resolved'|'missing', * status: 'resolved'|'missing'|'skipped',
* reason: string, * reason: string,
* article_title?: string, * article_title?: string,
* article_slug?: string * article_slug?: string

7
src/Service/NostrClient.php

@ -833,9 +833,14 @@ class NostrClient
$this->logger->warning('nostr.article_discussion.sequential_fallback', [ $this->logger->warning('nostr.article_discussion.sequential_fallback', [
'relays' => $forSeq, 'relays' => $forSeq,
]); ]);
// Use a shorter per-relay timeout for the web sequential fallback so one slow
// relay does not hold up the HTTP response for 3 × 12 s = 36 s.
// CLI prewarm still uses the full configured timeout via the normal path.
$seqTimeoutSec = min(6, $this->relayFanout->getRelayRequestTimeoutSec());
$response = $this->relayFanout->sendSequential( $response = $this->relayFanout->sendSequential(
$this->relayListFactory->relaySetFromDistinctUrlList($forSeq), $this->relayListFactory->relaySetFromDistinctUrlList($forSeq),
$requestMessage $requestMessage,
$seqTimeoutSec
); );
} }
} }

10
src/Service/NostrRelayFanoutTransport.php

@ -35,6 +35,11 @@ final readonly class NostrRelayFanoutTransport
) { ) {
} }
public function getRelayRequestTimeoutSec(): int
{
return $this->relayRequestFactory->getRelayRequestTimeoutSec();
}
/** /**
* @param list<string> $relayUrls * @param list<string> $relayUrls
* *
@ -58,9 +63,9 @@ final readonly class NostrRelayFanoutTransport
* *
* @return array<string, mixed> Same shape as {@see Request::send()} * @return array<string, mixed> Same shape as {@see Request::send()}
*/ */
public function sendSequential(RelaySet $relaySet, RequestMessage $requestMessage): array public function sendSequential(RelaySet $relaySet, RequestMessage $requestMessage, ?int $overrideTimeoutSec = null): array
{ {
$request = $this->relayRequestFactory->createTimedRequest($relaySet, $requestMessage); $request = $this->relayRequestFactory->createTimedRequest($relaySet, $requestMessage, $overrideTimeoutSec);
return $request->send(); return $request->send();
} }
@ -167,6 +172,7 @@ final readonly class NostrRelayFanoutTransport
* One line per relay after {@see Request::send()}: errors vs message-type counts (EVENT, EOSE, …). * One line per relay after {@see Request::send()}: errors vs message-type counts (EVENT, EOSE, …).
* *
* @param array<string, mixed> $response * @param array<string, mixed> $response
* @param int|null $overrideTimeoutSec when set, overrides the configured per-relay WebSocket timeout
*/ */
public function logWireResponseSummary(string $context, array $response): void public function logWireResponseSummary(string $context, array $response): void
{ {

6
src/Service/NostrRelayRequestFactory.php

@ -26,12 +26,14 @@ final readonly class NostrRelayRequestFactory
/** /**
* {@see Request::setTimeout()} drives per-relay WebSocket I/O for {@see Request::send()}. * {@see Request::setTimeout()} drives per-relay WebSocket I/O for {@see Request::send()}.
*
* @param int|null $overrideTimeoutSec when set, uses this instead of the configured default
*/ */
public function createTimedRequest(RelaySet $relaySet, RequestMessage $requestMessage): Request public function createTimedRequest(RelaySet $relaySet, RequestMessage $requestMessage, ?int $overrideTimeoutSec = null): Request
{ {
$request = new Request($relaySet, $requestMessage); $request = new Request($relaySet, $requestMessage);
return $request->setTimeout($this->relayRequestTimeoutSec); return $request->setTimeout($overrideTimeoutSec ?? $this->relayRequestTimeoutSec);
} }
/** /**

4
src/Twig/ArticleCardCoverExtension.php

@ -19,9 +19,9 @@ final class ArticleCardCoverExtension extends AbstractExtension
{ {
/** /**
* Used when the article has no image and the author has no (or no usable) NIP-01 {@see picture} URL. * Used when the article has no image and the author has no (or no usable) NIP-01 {@see picture} URL.
* Same asset as the header mark so empty hero slots read as the site, not a blank gray field. * The portrait painting is shown at low opacity with a CSS pattern overlay (see `.card-header--no-cover`).
*/ */
private const DEFAULT_PACKAGE_IMAGE = 'icons/favicon-96x96.png'; private const DEFAULT_PACKAGE_IMAGE = 'laeserin_logo.png';
private const OG_FALLBACK_PACKAGE_IMAGE = 'og-image.jpg'; private const OG_FALLBACK_PACKAGE_IMAGE = 'og-image.jpg';

3
templates/components/Molecules/Card.html.twig

@ -12,7 +12,8 @@
{% endif %} {% endif %}
</div> </div>
<a href="{{ (article.pubkey and npub_from_hex(article.pubkey) != '') ? path('article', { npub: npub_from_hex(article.pubkey), slug: article.slug }) : path('article-legacy-redirect', { slug: article.slug }) }}"> <a href="{{ (article.pubkey and npub_from_hex(article.pubkey) != '') ? path('article', { npub: npub_from_hex(article.pubkey), slug: article.slug }) : path('article-legacy-redirect', { slug: article.slug }) }}">
<div class="card-header"> {% set _no_cover = article.image|default('')|trim == '' %}
<div class="card-header{{ _no_cover ? ' card-header--no-cover' : '' }}">
{% if category %}<small class="text-uppercase">{{ category }}</small>{% endif %} {% if category %}<small class="text-uppercase">{{ category }}</small>{% endif %}
<img <img
src="{{ article_card_cover(article.image, article.pubkey) }}" src="{{ article_card_cover(article.image, article.pubkey) }}"

3
templates/components/Organisms/Comments.html.twig

@ -48,6 +48,9 @@
{% endif %} {% endif %}
<div class="comments" data-comments-partial="{{ comments_partial|default(false) ? '1' : '0' }}"> <div class="comments" data-comments-partial="{{ comments_partial|default(false) ? '1' : '0' }}">
{% if list|default([])|length == 0 and not comments_partial|default(false) %}
<p class="text-subtle comments__empty">No comments yet.</p>
{% endif %}
{% for item in list %} {% for item in list %}
{% set cid = item.id|default('')|lower %} {% set cid = item.id|default('')|lower %}
{% set cpk = item.pubkey|default('') %} {% set cpk = item.pubkey|default('') %}

3
templates/components/Organisms/FeaturedList.html.twig

@ -16,7 +16,8 @@
<div class="featured-tile__head"> <div class="featured-tile__head">
<span class="featured-tile__cat">{{ title }}</span> <span class="featured-tile__cat">{{ title }}</span>
</div> </div>
<div class="featured-tile__media featured-tile__media--ar{{ loop.index0 % 4 }}"> {% set _no_cover = item.image|default('')|trim == '' %}
<div class="featured-tile__media featured-tile__media--ar{{ loop.index0 % 4 }}{{ _no_cover ? ' card-header--no-cover' : '' }}">
<img <img
src="{{ article_card_cover(item.image, item.pubkey) }}" src="{{ article_card_cover(item.image, item.pubkey) }}"
alt="{{ ('Illustration for ' ~ item.title)|e('html_attr') }}" alt="{{ ('Illustration for ' ~ item.title)|e('html_attr') }}"

3
templates/components/Organisms/FeaturedWall.html.twig

@ -18,7 +18,8 @@
href="{{ article_href }}" href="{{ article_href }}"
aria-label="{{ (item.title ~ ' — ' ~ tile.categoryTitle)|e('html_attr') }}" aria-label="{{ (item.title ~ ' — ' ~ tile.categoryTitle)|e('html_attr') }}"
> >
<div class="featured-tile__picture"> {% set _no_cover = item.image|default('')|trim == '' %}
<div class="featured-tile__picture{{ _no_cover ? ' card-header--no-cover' : '' }}">
<img <img
class="featured-tile__picture-img" class="featured-tile__picture-img"
src="{{ article_card_cover(item.image, item.pubkey) }}" src="{{ article_card_cover(item.image, item.pubkey) }}"

3
templates/components/Organisms/HomeMagazineArticleStrip.html.twig

@ -14,7 +14,8 @@
{% set article_href = (item.pubkey and npub_from_hex(item.pubkey) != '') ? path('article', { npub: npub_from_hex(item.pubkey), slug: item.slug }) : path('article-legacy-redirect', { slug: item.slug }) %} {% set article_href = (item.pubkey and npub_from_hex(item.pubkey) != '') ? path('article', { npub: npub_from_hex(item.pubkey), slug: item.slug }) : path('article-legacy-redirect', { slug: item.slug }) %}
<article class="curation-article-display"> <article class="curation-article-display">
<div class="curation-article-display__pane"> <div class="curation-article-display__pane">
<div class="curation-article-display__media"> {% set _no_cover = item.image|default('')|trim == '' %}
<div class="curation-article-display__media{{ _no_cover ? ' card-header--no-cover' : '' }}">
<a href="{{ article_href }}" tabindex="-1" aria-hidden="true"> <a href="{{ article_href }}" tabindex="-1" aria-hidden="true">
<img <img
src="{{ article_card_cover(item.image, item.pubkey) }}" src="{{ article_card_cover(item.image, item.pubkey) }}"

Loading…
Cancel
Save