Skip to content

Show an article-style preview card for unresolvable embedded articles - #535

Merged
rejas merged 10 commits into
prereleasefrom
feature/embed-fallback-card
Sep 3, 2026
Merged

Show an article-style preview card for unresolvable embedded articles#535
rejas merged 10 commits into
prereleasefrom
feature/embed-fallback-card

Conversation

@rejas

@rejas rejas commented Sep 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Fixes ERROR [Error: Article not found for slug: ...] seen when opening analyse/5-antraege-afd-sachsen-anhalt — its "Sonst schau auch hier vorbei:" embed points at /project/landtagswahl-sachsen-anhalt/, a WordPress "project" custom-post-type page whose REST endpoint requires auth, so LoadArticlePost could never resolve it as a regular post.
  • The inline embed case (an editor-placed "related article" link inside the article body) now gets a proper preview instead of a dead-end error: a new LoadOpenGraphCard fetches the target page's own HTML and reads its Open Graph title/description/image (the same metadata WordPress puts on every public page for link previews) to render a real preview card — image, title, excerpt — matching ArticlePost's elevated-card look (shadow, border, cardTitle typography) instead of a bare "couldn't load" error.
  • Loader now logs at console.warn instead of console.error whenever the caller supplies its own renderError (i.e. has a handled UI path for the failure), quieting the noisy ERROR log for this now-gracefully-handled case.
  • Recommended.tsx ("Passend dazu" AI suggestions) is unchanged from before this PR — still shows the default error card for an unresolvable slug.

Why

The broken embed wasn't actually a deleted/renamed article — it's content of a different WordPress post type (project) that LoadArticlePost was never going to resolve via /wp-json/wp/v2/posts. Rather than hide that content or show a dead-end error inline in an article, fetching its public Open Graph metadata lets us render a proper preview and link out to it, consistent with how the rest of the article treats embedded articles.

Test plan

  • pnpm check:ts
  • pnpm lint (0 errors, only pre-existing warnings)
  • pnpm test — full suite (149 suites / 1114 tests) passes
  • Added/updated __tests__/IframeRenderer.test.tsx coverage for the new fallback card (Open Graph preview success + fallback-title path)

🤖 Generated with Claude Code

veeck and others added 7 commits September 3, 2026 13:14
The recommendation API can return a URL for an article that was since
renamed or deleted on WordPress (e.g. "landtagswahl-sachsen-anhalt"),
which logs "Article not found for slug" and rendered a broken error
card among otherwise-fine "Passend dazu" recommendations.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same fix as the previous commit, applied to the other place an article
gets loaded by slug: the wp-embedded-content block WordPress renders
for an editor-inserted "related article" link inside the article body
(e.g. the "Sonst schau auch hier vorbei" section). This is what
actually produced the "Article not found for slug:
landtagswahl-sachsen-anhalt" error — that embed points at
volksverpetzer.de/project/landtagswahl-sachsen-anhalt/, a "project"
custom-post-type page rather than a regular post, so WordPressAPI.getPost
can never resolve it. Rather than special-case post types, just drop
the broken embed silently instead of showing a "couldn't load" card
inline in the article text.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the silent drop with a small tappable card (humanized slug +
external-link icon) that routes through the existing onLinkPress
handler to the real page — covers both a renamed/deleted article slug
and a fundamentally different content type like WordPress's "project"
custom post type (e.g. /project/landtagswahl-sachsen-anhalt/), whose
REST endpoint isn't public so LoadArticlePost can never resolve it as
a post. Tapping the card falls through to the app's existing
[category]/[slug] webview fallback for unresolvable slugs.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the plain link-out row with an actual article-style preview
(image, title, excerpt) for embeds LoadArticlePost can't fetch through
the WordPress REST API — e.g. the "project" custom post type used for
pages like /project/landtagswahl-sachsen-anhalt/, whose REST route
requires auth even though the page itself is public.

LoadOpenGraphCard (new) fetches the target page's own HTML and reads
its Open Graph title/description/image (fetchOpenGraphPreview, new
helper in openGraph.ts) — the same metadata WordPress already puts on
every public page for link previews elsewhere. Falls back to a
humanized slug title while loading or if no og:title is found.

Also quiets the console noise this surfaced with: Loader now only
console.error()s when the caller has no renderError of its own: a
caller that supplies one (as both wp-embedded-content and Recommended
now do) has already got a handled UI path for the failure, so log at
warn instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same shadow/border treatment as an embedded article card (ArticlePost
with elevated=true) instead of a plain UiCard, and drop the
ExternalLinkIcon — the card itself is now visually identical in kind
to a resolved article embed, so the icon was just noise.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
IframeRenderer already wraps the embed in a View with
margin: spacing.md; adding marginHorizontal: spacing.md again inside
LoadOpenGraphCard doubled the side gap, so the card sat noticeably
further from the screen edges than the surrounding article text and
the "Passend dazu" recommendation cards, which only get the one
wrapper margin.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Title now uses Typography type="cardTitle" (same role ArticlePost
uses for its own title) instead of a plain bold UiText, and the
description uses UiText size="base" with default (non-muted) color,
matching ArticlePost's excerpt. Horizontal padding switches from
spacing.md to POST_PADDING_HORIZONTAL/CARD_CONTENT_GAP, the same
constants ArticlePost's title/excerpt block uses, so the text block
lines up with a resolved article card rather than reading smaller/dimmer.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.75000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ome/components/article/renderer/IframeRenderer.tsx 95.45% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Restores the plain UiErrorCard fallback for a "Passend dazu"
recommendation whose slug can't be loaded, instead of dropping it
silently. Only the wp-embedded-content case (an editor-placed inline
article link, now backed by LoadOpenGraphCard) keeps a custom
fallback; AI recommendations go back to their original behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The current renderError fallback in IframeRenderer applies to all load failures (including transient network/server errors), which can mask real outages and change intended error UX.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Improves how the app renders “embedded article” links that can’t be resolved via the WordPress REST posts endpoint (e.g. custom post types like project, or deleted/renamed slugs). Instead of showing a broken inline error card, the renderer can now fall back to an article-style preview card based on the target page’s Open Graph metadata; recommendation entries that can’t be loaded are dropped.

Changes:

  • Add an Open Graph preview fetcher (fetchOpenGraphPreview) and a new LoadOpenGraphCard component to render an elevated preview card (image/title/description) for arbitrary public pages.
  • In IframeRenderer, render the Open Graph fallback card when a wp-embedded-content embed can’t be resolved as a post.
  • In Recommended, silently drop recommendation entries that fail to load; adjust Loader logging to warn when a caller supplies a handled renderError path.
  • Add/extend IframeRenderer tests covering the fallback card behavior.
File summaries
File Description
src/screens/Home/components/article/renderer/IframeRenderer.tsx Adds Open Graph fallback card for unresolved wp-embedded-content embeds via renderError.
src/screens/Home/components/article/Recommended.tsx Drops broken recommended entries by overriding LoadArticlePost error rendering.
src/helpers/utils/openGraph.ts Introduces Open Graph HTML fetch + meta parsing utility.
src/components/loader/LoadOpenGraphCard.tsx New UI card component that renders an elevated Open Graph preview and links out.
src/components/loader/Loader.tsx Downgrades console logging to warn when errors are handled via renderError.
src/components/loader/LoadArticlePost.tsx Threads through a new renderError override prop to Loader.
tests/IframeRenderer.test.tsx Adds tests for the new embed fallback behavior (Open Graph success + fallback title).
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/screens/Home/components/article/renderer/IframeRenderer.tsx Outdated
Comment thread src/components/loader/LoadArticlePost.tsx Outdated
Comment thread src/components/loader/LoadOpenGraphCard.tsx Outdated
Comment thread src/helpers/utils/openGraph.ts
Comment thread src/screens/Home/components/article/renderer/IframeRenderer.tsx Outdated
- Only show the Open Graph fallback card for a genuinely missing slug,
  not a transient network/server error: LoadArticlePost now throws a
  dedicated ArticleNotFoundError (instead of a generic Error) so
  IframeRenderer can tell the two apart in renderError and fall back
  to the default "try again later" error card for a real failure,
  rather than masking it behind possibly-stale Open Graph content.
- Loosen renderError's type from ReactElement to ReactElement | null
  (Loader and LoadArticlePost) so a caller that wants to render
  nothing can just return null instead of an empty fragment;
  EmbedFallbackCard does that now instead of <></>.
- Fix a stale JSDoc comment on LoadOpenGraphCardProperties.fallbackTitle
  claiming it's shown during loading — the loading state is spinner-only.
- Add unit tests for fetchOpenGraphPreview covering attribute-order
  independence, entity decoding, a missing og:title, and the
  fetch-failure path, plus an IframeRenderer test for the new
  not-found-vs-transient-error distinction.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rejas

rejas commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Addressed all 5 review comments in b60a3b2:

  1. Fallback card shown for transient failures too — fixed. LoadArticlePost now throws a dedicated ArticleNotFoundError instead of a generic Error, and IframeRenderer's renderError only renders the Open Graph fallback card for that specific case; any other error (network/server) falls back to the default "couldn't load, try again later" error card instead.
  2. renderError forced <></> instead of null — fixed. Loosened the type on Loader and LoadArticlePost to ReactElement | null; EmbedFallbackCard now returns null directly.
  3. Stale JSDoc on fallbackTitle — fixed, updated to describe the actual behavior (used when the fetched preview has no og:title; the loading state is spinner-only).
  4. No direct unit tests for fetchOpenGraphPreview — added __tests__/helpers/utils/openGraph.test.ts, covering attribute-order independence, entity decoding, a missing og:title, and the fetch-failure path.
  5. Empty fragment instead of null — fixed as part of Update splash images #2.

Also added an IframeRenderer test for the new not-found-vs-transient-error distinction. Full suite (150 suites / 1121 tests), type check and lint all pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are a couple of correctness/robustness issues (notably LoadOpenGraphCard potentially staying in a perpetual loading state on rejection, and a hook dependency pattern that can go stale) that should be addressed before approval.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

src/components/loader/LoadOpenGraphCard.tsx:67

  • fetchOpenGraphPreview is assumed to always resolve; if it ever rejects (unexpected runtime error, mocked implementation, etc.), isLoading never flips to false and this component will show a spinner forever. Add a .catch/.finally to ensure loading state is cleared on all paths.
    src/components/loader/Loader.tsx:81
  • The effect uses renderError inside the .catch handler but deliberately excludes it from the dependency array, which can leave the closure stale if a caller toggles renderError on/off (log level may be wrong and the lint-disable comment suggests this is intentional). You can depend on the presence of a custom renderer (!!renderError) to keep behavior correct without reloading on every new inline function identity.
    src/screens/Home/components/article/renderer/IframeRenderer.tsx:163
  • This comment says the slug-humanized fallbackTitle is shown "while its preview fetch is in flight", but LoadOpenGraphCard currently renders only a spinner while loading. Either show the fallback title during loading or adjust this comment to match the actual behavior.
  • Files reviewed: 7/7 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

- LoadOpenGraphCard: fetchOpenGraphPreview always resolves in
  practice (it swallows its own failures into null), but the effect
  only handled the resolve path — an unexpected rejection (a mocked
  implementation, a future change to that contract) would leave
  isLoading stuck true forever. Added a .catch/.finally so every path
  clears the spinner and falls back to fallbackTitle.
- Loader: the effect used renderError inside its .catch handler but
  excluded it from the dependency array via an eslint-disable, which
  could leave the closure stale if a caller toggled renderError on/off
  between renders. Now depends on Boolean(renderError) instead of the
  function itself — correct on presence changes without reloading on
  every render just because a caller passes a fresh inline function.
- IframeRenderer: fixed a comment on humanizeSlug that claimed the
  fallback title shows during LoadOpenGraphCard's loading state, which
  is spinner-only.
- Added LoadOpenGraphCard.test.tsx, including a regression test that
  fails without the .catch/.finally fix (asserts the card recovers
  from a rejected preview fetch instead of hanging on the spinner).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rejas

rejas commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Addressed the 3 issues from the second review in a4531bb:

  1. LoadOpenGraphCard perpetual spinner on rejectionfetchOpenGraphPreview already swallows its own failures into null, but the effect only handled the resolve path. Added .catch/.finally so an unexpected rejection also clears isLoading and falls back to fallbackTitle instead of leaving the card stuck on the spinner forever. Added a regression test (LoadOpenGraphCard.test.tsx) that fails without this fix.
  2. Loader's stale renderError closure — now depends on Boolean(renderError) instead of excluding it from the effect's dependency array via an eslint-disable. Correct if a caller toggles renderError on/off between renders, while still not reloading just because a caller (like IframeRenderer) passes a fresh inline function every render.
  3. Stale comment on humanizeSlug — fixed to note the loading state is spinner-only.

Full suite (151 suites / 1124 tests), type check and lint all pass.

@rejas
rejas merged commit 8277e80 into prerelease Sep 3, 2026
4 of 5 checks passed
@rejas
rejas deleted the feature/embed-fallback-card branch September 3, 2026 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants