Skip to content

Handle broken plugin detail posters#3943

Closed
YUHAO-corn wants to merge 1 commit into
nexu-io:mainfrom
YUHAO-corn:yuhao/fix-plugin-detail-poster-fallback-3894
Closed

Handle broken plugin detail posters#3943
YUHAO-corn wants to merge 1 commit into
nexu-io:mainfrom
YUHAO-corn:yuhao/fix-plugin-detail-poster-fallback-3894

Conversation

@YUHAO-corn

@YUHAO-corn YUHAO-corn commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • hide plugin detail poster images after the browser reports a load error
  • fall back to the existing unavailable/glyph states instead of leaving a broken image visible
  • add regression coverage for image posters, recovered poster URLs, and audio poster fallback

Fixes #3894

Surface area

  • UI
  • Daemon/runtime
  • Docs
  • Tests only

Validation

  • git diff --check
  • lightweight static assertions for the poster error/fallback paths

Note: the focused Vitest command could not run locally in this fresh worktree because node_modules is not installed; relying on CI for the full test run.

@lefarcen lefarcen requested review from lefarcen and nettee June 8, 2026 19:09
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/medium Medium risk: regular code changes type/bugfix Bug fix labels Jun 8, 2026

@lefarcen lefarcen 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.

Hi @YUHAO-corn! 👋 The posterLoadFailed state + reset effect approach is clean — catching the error at the img level and reusing the existing unavailable/glyph fallback path is exactly right. Pairing that with regression tests for image posters, recovered URLs, and audio poster fallback is a solid coverage shape.

Two quick template items before pool review picks this up:

  • Surface area: could you add a ## Surface area section and tick UI (this touches the plugin detail stage visually)? The checklist helps reviewers scope the change quickly.
  • Validation: a brief ## Validation section covering what you ran locally would help — e.g. the vitest command you used, and whether you verified the fallback in browser with a broken URL.

Once those are in, the PR should be in great shape. ✨

💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …):
Take over nexu-io/open-design#3943 until it merges — read https://raw.githubusercontent.com/nexu-io/looper/main/skills/pr-takeover/SKILL.md and follow it.

@nettee nettee 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.

@YUHAO-corn I reviewed the changed ranges in PluginMediaDetail and the new regression coverage. The poster error state resets correctly when the record or poster URL changes, and the fallback behavior for broken image and audio posters matches the broken-preview symptom described in #3894 without regressing the rest of the detail view. Nice fix, and the focused regression cases make this path much easier to trust.

🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Visual regression review

Head: df7200d · Base: 75f1472

3 changed · 14 unchanged · 0 missing baseline · 0 failed

Changed cases

Case Main PR Diff
visual-design-systems main pr diff
visual-home-plugin-filter main pr diff
visual-new-project-modal main pr diff
Unchanged cases
Case Main PR Diff
visual-avatar-menu main pr diff
visual-home main pr diff
visual-home-catalog main pr diff
visual-home-context-picker main pr diff
visual-integrations main pr diff
visual-integrations-use-everywhere main pr diff
visual-plugin-details main pr diff
visual-plugins main pr diff
visual-projects main pr diff
visual-projects-kanban main pr diff
visual-settings-byok main pr diff
visual-settings-execution main pr diff
visual-tasks main pr diff
visual-topbar-execution-switcher main pr diff

Visual diff is advisory only and does not block merging.

@kokisanai

Copy link
Copy Markdown
Contributor

Hi @YUHAO-corn!

This PR looks quiet for a bit, so I'm checking in to make sure it doesn't get stuck.

If you are waiting on review, feedback, or a specific unblock from the team, feel free to say so here. If faster coordination helps, you can also drop into our contributors channel in Discord:
https://discord.gg/3C6EWXbdQQ

We want to help move this forward instead of letting it stall.

@lefarcen

Copy link
Copy Markdown
Contributor

Quick status note for @YUHAO-corn: this PR isn't stale — nettee reviewed and approved it, and all CI checks are green. It's sitting clean and waiting for a maintainer to merge. Nothing is stuck on your end. 🙌

@lefarcen lefarcen added needs-validation Runtime change detected; needs human or /explore agent validation. and removed needs-validation Runtime change detected; needs human or /explore agent validation. labels Jun 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

@YUHAO-corn friendly reminder: this PR has been waiting on an author response for more than 3 days after reviewer or maintainer feedback.

When you have a chance, please reply here or push an update. To keep the queue manageable, PRs with no author activity for more than 5 days after feedback may be closed automatically, but they can be reopened when work resumes.

@lefarcen lefarcen added the needs-validation Runtime change detected; needs human or /explore agent validation. label Jun 12, 2026
@lefarcen

Copy link
Copy Markdown
Contributor

🧪 This PR has changes that need manual validation, so it needs a QA pass by @AmyShang-alt before merge — please hold off self-merging; we’ve queued design + QA review now and will update here once that pass is done.

@lefarcen

Copy link
Copy Markdown
Contributor

Hey @YUHAO-corn — thanks for the fix. The code path and tests make the fallback logic clear, but since this PR changes a visible plugin detail UI state, could you please add screenshots to the PR body?

In particular, it would help to see:

  • A plugin detail image preview after the poster fails and the unavailable fallback is shown
  • An audio preview after the poster fails, showing the audio glyph fallback while the audio player remains available

Without screenshots, the logic is reviewable from the diff/tests, but the final UI layout and fallback appearance are hard to validate. Thanks! 🙌

@github-actions

Copy link
Copy Markdown
Contributor

Closing this PR for now because it has been waiting on an author response for more than 5 days after reviewer or maintainer feedback.

This is only a queue-management step, not a rejection of the work. If you would like to continue, please leave a comment or push an update and reopen the PR when ready.

@github-actions github-actions Bot closed this Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-validation Runtime change detected; needs human or /explore agent validation. risk/medium Medium risk: regular code changes size/M PR changes 100-300 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugin detail previews can show broken images in the Plugins tab

4 participants