-
Notifications
You must be signed in to change notification settings - Fork 220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix types in the templates #2584
Conversation
Size Change: +67 B (0%) Total Size: 832 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 When we switch to Nuxt 3 we should consider using error boundaries for client-side errors like these:
https://nuxt.com/docs/api/components/nuxt-error-boundary
Currently, if a frontend request to a single media result from the API fails, the page will render blank like this:
I do wonder if we should be throwing a 404 error or displaying some "{media} not found or temporarily unavailable" message here. Perhaps we could show a specific message if the API 429s. All of those would be follow-up work though and require design from @fcoveram.
Nice!
This wasn't the intended behavior 😨 I did try to redirect to the error page (our generic FourOhFour) if the request fails. I had the same idea for 429, here's the issue: #2586 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes make sense, LGTM!
db7b1af
to
91ad690
Compare
Note: This PR is based on #2583, and should be reviewed after it.1Fixes
Related to #2553 by @obulat
Description
This PR adds conditional rendering to the single result page templates to account for the fact that the main media (
image
/audio
) can benull
.This PR was split from the bigger fix to simplify review.
Testing Instructions
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin
Footnotes
I realized that this PR doesn't really need the changes from 2583, so I rebased it on main. ↩