Fix broken homepage image link #545
Conversation
|
@Justin322322 is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
WalkthroughAdds client-side guarding for the landing hero background image with onError handling; introduces an isFreesoundConfigured() check used by the sounds search API route to return 503 when Freesound is not configured; relaxes several server env var validations to optional; adds non-null assertions where env vars are used. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant H as Hero Component
participant I as Next/Image
U->>H: Load homepage
H->>I: Attempt render background (/landing-page-dark.png)
alt Image loads
I-->>H: onLoad
H-->>U: Background visible
else Image fails
I-->>H: onError
H->>H: setBgError(true)
H-->>U: Re-render without background
end
sequenceDiagram
participant C as Client
participant S as /api/sounds/search
participant F as isFreesoundConfigured()
participant API as Freesound API
C->>S: Request search
S->>S: Rate-limit check
S->>F: isFreesoundConfigured()
alt Not configured
F-->>S: {configured: false, missingVars: [...]}
S-->>C: 503 "Sound search not configured" (logs missing vars)
else Configured
F-->>S: {configured: true}
S->>API: perform Freesound search (uses env.FREESOUND_API_KEY!)
API-->>S: results
S-->>C: results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
- Make FREESOUND_CLIENT_ID, FREESOUND_API_KEY, CLOUDFLARE_ACCOUNT_ID, R2_ACCESS_KEY_ID, R2_SECRET_ACCESS_KEY, R2_BUCKET_NAME, MODAL_TRANSCRIPTION_URL optional in env.ts - Add isFreesoundConfigured() utility function - Update sounds search API to check configuration before using Freesound vars - Fixes CI build errors where these optional features caused required env validation failures
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/src/components/landing/hero.tsx (2)
24-24: Decorative background: consider empty alt to reduce screen-reader noise.
Because this is a purely decorative background image, using alt="" prevents redundant announcements and improves a11y.- alt="Landing page background" + alt=""
19-27: Optional: Provide sizes for better responsive hinting (if not switching to fill).
If you keep explicit width/height instead of fill, adding sizes helps the browser choose the optimal source sooner.<Image className="absolute top-0 left-0 -z-50 size-full object-cover invert dark:invert-0 opacity-85" src="/landing-page-dark.png" height={1903.5} width={1269} - alt="Landing page background" + alt="Landing page background" onError={() => setBgError(true)} + sizes="100vw" priority />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/components/landing/hero.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{jsx,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{jsx,tsx}: Don't useaccessKeyattribute on any HTML element.
Don't setaria-hidden="true"on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like<marquee>or<blink>.
Only use thescopeprop on<th>elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include atitleelement for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
AssigntabIndexto non-interactive HTML elements witharia-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include alangattribute on the html element.
Always include atitleattribute for iframe elements.
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress.
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...
Files:
apps/web/src/components/landing/hero.tsx
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
Files:
apps/web/src/components/landing/hero.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/components/landing/hero.tsx
🧬 Code Graph Analysis (1)
apps/web/src/components/landing/hero.tsx (1)
apps/web/src/app/page.tsx (1)
Home(13-21)
🔇 Additional comments (5)
apps/web/src/components/landing/hero.tsx (5)
12-12: LGTM: Local state import is appropriate for client-side image error handling.
Importing useState here is the right approach given the client-only behavior.
15-15: LGTM: Simple, top-level hook for error guard is clear and correct.
The bgError flag is well-named and placed; hook usage complies with rules-of-hooks.
18-19: LGTM: Conditional render prevents broken-image icon from appearing.
Wrapping the Image with a guard and flipping the flag on error meets the issue’s requirement cleanly.Also applies to: 27-28
26-26: LGTM: Using priority for the above-the-fold hero background is appropriate.
This aligns with Next.js guidance for critical imagery.
21-21: Image reference is valid—no changes needed
- Confirmed that
apps/web/public/landing-page-dark.pngexists.- There are no
width/heightprops to reconcile against the image’s pixel dimensions.
This commit addresses TypeScript compilation errors that occurred after making
optional API environment variables truly optional in the previous commit.
## Changes Made
### API Route Updates
- env.R2_ACCESS_KEY_ID! and env.R2_SECRET_ACCESS_KEY! in AwsClient constructor
- env.R2_BUCKET_NAME! and env.CLOUDFLARE_ACCOUNT_ID! in URL construction
- Safe because isTranscriptionConfigured() check ensures these exist before usage
- transcribe/route.ts: Added non-null assertion for Modal transcription URL
- env.MODAL_TRANSCRIPTION_URL! in fetch() call
- Safe because isTranscriptionConfigured() check validates this beforehand
- sounds/search/route.ts: Added non-null assertion for Freesound API key
- env.FREESOUND_API_KEY! in URLSearchParams construction
- Safe because new isFreesoundConfigured() check validates this beforehand
### Configuration Validation
- transcription-utils.ts: Added isFreesoundConfigured() utility function
- Checks for FREESOUND_CLIENT_ID and FREESOUND_API_KEY availability
- Returns {configured: boolean, missingVars: string[]} for consistent error handling
- Mirrors existing isTranscriptionConfigured() pattern
- sounds/search/route.ts: Added Freesound configuration check
- Early return with 503 status if Freesound credentials missing
- Provides clear error message listing required environment variables
- Prevents runtime errors when optional sound search feature is not configured
## Why These Changes Are Safe
1. Non-null assertions are protected: All usage of env.VAR! is preceded by
configuration checks that verify the variables exist
2. Graceful degradation: Missing optional features return proper HTTP 503
responses instead of crashing
3. Consistent error handling: All optional features follow the same pattern
of checking configuration before usage
4. Build-time validation: Environment variables are optional at build time
but validated at runtime when features are accessed
## Testing
- Local build passes: bun run build completes successfully
- TypeScript compilation: No type errors
- Lint checks: All pass
- Optional features degrade gracefully when env vars missing
This ensures the CI/CD pipeline passes while maintaining proper runtime
validation for optional features like sound search and auto-transcription.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/api/sounds/search/route.ts(2 hunks)apps/web/src/env.ts(1 hunks)apps/web/src/lib/transcription-utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the!postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't use the TypeScript directive @ts-ignore.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.
Files:
apps/web/src/lib/transcription-utils.tsapps/web/src/app/api/sounds/search/route.tsapps/web/src/env.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{js,jsx,ts,tsx}: Don't use the return value of React.render.
Don't use consecutive spaces in regular expression literals.
Don't use theargumentsobject.
Don't use primitive type aliases or misleading types.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use uselessthisaliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of...
Files:
apps/web/src/lib/transcription-utils.tsapps/web/src/app/api/sounds/search/route.tsapps/web/src/env.ts
🧬 Code Graph Analysis (2)
apps/web/src/lib/transcription-utils.ts (1)
apps/web/src/env.ts (1)
env(7-45)
apps/web/src/app/api/sounds/search/route.ts (1)
apps/web/src/lib/transcription-utils.ts (1)
isFreesoundConfigured(15-22)
🔇 Additional comments (4)
apps/web/src/env.ts (2)
18-26: Aligning Freesound envs with runtime gating — LGTMMaking FREESOUND_CLIENT_ID and FREESOUND_API_KEY optional matches the new isFreesoundConfigured() guard and prevents app boot from failing when these are intentionally unset.
18-26: All optionalized R2/Modal env vars are properly gatedI audited every reference to the newly optionalized variables (CLOUDFLARE_ACCOUNT_ID, R2_*, MODAL_TRANSCRIPTION_URL) across the codebase and confirmed that each usage is preceded by the appropriate
isTranscriptionConfigured()orisFreesoundConfigured()check. No unguarded accesses were found. Safe to proceed.apps/web/src/app/api/sounds/search/route.ts (1)
5-5: Importing isFreesoundConfigured here is the right placeRoute-level gating right after rate-limit is an appropriate integration point. Nice.
apps/web/src/lib/transcription-utils.ts (1)
15-22: Helper function is concise and mirrors isTranscriptionConfigured — LGTMChecks the two required vars and returns a clear result object. Consistent with the existing pattern.
Justin322322
force-pushed
the
fix/broken-homepage-image-543
branch
from
357998a to
5951adb
Compare
- Remove sensitive environment variable names from client error responses - Simplify error message to 'Sound search is not configured' for production - Keep detailed logging server-side for operational visibility - Conditionally include missingVars in response only for non-production environments - Prevents information disclosure while maintaining developer debugging capability Security improvement: Production clients no longer see internal configuration details
- Replace invalid float height (1903.5) with proper fill prop - Add relative positioning to container for proper image anchoring - Use fill + sizes='100vw' for optimized full-bleed background - Remove redundant size-full class (covered by fill) - Maintains responsive behavior and all existing styling Fixes invalid HTML attributes and improves image loading performance
Justin322322
force-pushed
the
fix/broken-homepage-image-543
branch
from
e01afce to
fb5de2f
Compare
|
@Justin322322 I'll review this PR that fixes the broken homepage image link and addresses environment variable configuration issues. ✅ Actions performedReview triggered.
|
|
Closing PR issue has been addressed |
1 participant
Summary
Fix broken homepage background image by switching to an existing asset and adding error handling to prevent broken image icons from appearing.
Why
The homepage Hero component referenced
/landing-page-bg.pngwhich doesn't exist in the public directory. This caused a broken image icon to appear in the top corner of the homepage, degrading the user experience. The issue was reported in #543 with screenshots showing the broken image placeholder.What changed
/landing-page-bg.pngto existing/landing-page-dark.pnguseStateto track image load errors and conditionally hide the image if it fails to loadpriorityprop to ensure the background image loads quicklyIn
apps/web/src/components/landing/hero.tsx:useStatefrom ReactbgErrorstate to track image loading failures<Image>in conditional rendering{!bgError && (...)}onError={() => setBgError(true)}handlersrcto/landing-page-dark.png(confirmed to exist inapps/web/public/)Testing
/landing-page-dark.pngexists in the public directoryinvert dark:invert-0ensure the dark image works correctly across light/dark themesNotes
This fix is minimal and focused - it addresses the immediate broken image issue without changing the overall design or layout. The solution follows the expected behavior outlined in the issue: "If the image file is missing or invalid, the image element should be removed or replaced with a placeholder."
Closes #543
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores
Before
After