-
-
Notifications
You must be signed in to change notification settings - Fork 184
docs: add REPORT.md documenting Unsplash preview contribution #351
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
base: main
Are you sure you want to change the base?
Conversation
|
@santarsiero is attempting to deploy a commit to the shravan20's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds REPORT.md documenting the Unsplash preview feature. Updates frontend Home and TemplateCard components to support bgSource=unsplash and unsplashQuery with debounced input, network image preview, memoized blob handling, and a loader. Updates .env.example with REACT_APP_SERVICE_URL and UNSPLASH_ACCESS_KEY notes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Home as Home (React)
participant TemplateCard as TemplateCard
participant API as Backend Service
participant Unsplash as Unsplash API
User->>Home: Select bgSource=unsplash / type unsplashQuery
Home-->>Home: Debounce input
Home->>TemplateCard: Pass computed quoteUrl (with bgSource, unsplashQuery)
TemplateCard->>API: Fetch preview image via quoteUrl
API->>Unsplash: Request image (bgSource=unsplash, query?)
Unsplash-->>API: Image URL/bytes or quota error
API-->>TemplateCard: Image/response
alt Success
TemplateCard-->>TemplateCard: Memoize blob / set loader=false
TemplateCard-->>User: Render live preview (network image)
else Quota/Invalid Key
TemplateCard-->>User: Show loader fallback/error state
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (7)
REPORT.md (7)
12-16: Clarify quota statement and add an official reference.Avoid hard-coding “~50 requests/hour” as a universal fact; quotas vary by plan and may change. Reword and add a docs link.
Apply this diff:
-During testing, we discovered a **new issue**: Unsplash previews stop working after ~50 requests/hour. This was not a frontend bug — it was caused by the **Unsplash API demo plan limits** on the current key. +During testing, we discovered a **new issue**: Unsplash previews stop working after the demo plan’s hourly rate limit is reached (we observed ≈50 req/h on our key). This was not a frontend bug — it was caused by **Unsplash API plan limits** on the current key. Add a link to the official rate‑limit docs here.
21-36: Tighten env guidance to prevent accidental key exposure.On CRA apps, REACT_APP_* is baked into the client bundle; recommend placing UNSPLASH_ACCESS_KEY on the server and using
.env.local(gitignored) for secrets.Apply this diff:
-- Configured `.env` with: +- Configure env vars (prefer `.env.local`, which is gitignored) with: @@ - SERVICE_URL=https://github-readme-quotes-bay.vercel.app/quote - REACT_APP_SERVICE_URL=https://github-readme-quotes-bay.vercel.app/quote - UNSPLASH_ACCESS_KEY=<your_key_here> + SERVICE_URL=https://github-readme-quotes-bay.vercel.app/quote + REACT_APP_SERVICE_URL=https://github-readme-quotes-bay.vercel.app/quote + # Server-side secret (do NOT prefix with REACT_APP_) + UNSPLASH_ACCESS_KEY=<your_server_key_here> + # Note: Any REACT_APP_* variable is exposed to the browser bundle. Keep keys server-side and proxy Unsplash calls via the backend.Also verify the repo’s actual script names (build/prod) match these commands.
43-56: Document error/fallback UX and a11y for the preview image.Add guidance for 429/5xx/NetworkError: show last-good image or neutral gradient; surface a brief message; set meaningful alt text.
Apply this diff:
2. **`frontend/src/components/organisms/TemplateCard/index.js`** @@ - Memoized blob creation to stop flickering loops. - - Added loader state for smoother UX. + - Added loader state for smoother UX. + - Error/fallback UX: on 429/5xx/network errors, render the last successful preview (if any) or a neutral gradient with a short inline message. Include alt text for accessibility.
73-85: Make the proof reproducible and safer to run.Use
curl -ito show headers and explicitly call outX-Ratelimit-Remaining. Keep placeholder keys.Apply this diff:
-$ curl "https://api.unsplash.com/photos/random?query=ocean&client_id=DEMO_KEY" +$ curl -i "https://api.unsplash.com/photos/random?query=ocean&client_id=DEMO_KEY" @@ X-Ratelimit-Limit: 50 X-Ratelimit-Remaining: 0
94-99: Broaden mitigation guidance beyond “upgrade key.”Add low-effort resiliency ideas the team can pick up later.
Apply this diff:
- **Upgrade API plan**: request a production Unsplash API key (higher quota). - **Rotate the key** in `.env` (`UNSPLASH_ACCESS_KEY`). - (Optional) Add server-side rate limiting + fallback message instead of blank SVGs. + - (Optional) Cache the last successful preview per query (short TTL) to reduce repeat calls. + - (Optional) Backoff on 429 with jitter; surface a non-blocking toast in the UI.
112-118: Neutralize tone and suggest a docs location.Slight tone tweak and consider moving the report under docs/.
Apply this diff:
-This contribution is complete, professional, and ready for maintainers. +Ready for maintainer review. If a standalone report isn’t desired at repo root, we can move this to `docs/unsplash-preview.md` or link it from the PR description.
43-59: Add cross-references for traceability.Link the tracked issue and the prior PR to help future readers.
Apply this diff:
### Files Edited & Why +Related: #340 (feature request), see prior work in #336 for integration guidance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
REPORT.md(1 hunks)
🔇 Additional comments (1)
REPORT.md (1)
3-11: Clear, succinct abstract — keep.Good overview of scope, user value, and delta vs. backend.
PR: Add Unsplash Preview Support to Dashboard
Summary
This PR implements support for Unsplash previews in the GitHub-Readme-Quotes dashboard.
Users can now select Unsplash as a background source.
Queries can be entered (e.g., “mountains”), or left blank for random images.
The preview updates live with the Unsplash background.
Files Edited
frontend/src/components/Pages/Home/index.js → added debounce on query input, confirmed state wiring.
frontend/src/components/organisms/TemplateCard/index.js → switched preview source to network URL when Unsplash is active, added memoization + loader.
.env.example → documented REACT_APP_SERVICE_URL and UNSPLASH_ACCESS_KEY.
REPORT.md → added to fork for documentation purposes (summarizes contribution, debugging, and Unsplash API findings).
Issue Reference
Closes #340
Testing
Verified Unsplash previews update correctly:
Off → default color/gradient.
On, empty query → random Unsplash image.
On, with query → keyword-based Unsplash image.
Added debounce and loader for smoother UX.
New Issue Discovered
During extended testing, Unsplash previews stop working after ~50 requests/hour.
Confirmed this is not a frontend bug.
Root cause: Unsplash API demo plan quota on current key.
Proof: curl tests returning 429 Too Many Requests with X-Ratelimit-Remaining: 0.
Solution Path
Upgrade Unsplash API plan (Production key).
Rotate new key in .env (UNSPLASH_ACCESS_KEY).
(Optional) Add server-side rate limiting/fallback for quota exhaustion.
End State
Feature implemented and fully functional when under quota.
Clear operational fix provided for maintainers.
Note: REPORT.md is included in this PR for assignment/portfolio documentation. Happy to remove it if it does not fit upstream contribution guidelines.
Summary by CodeRabbit