Limit releases to only show most recent ones #4570
Limit releases to only show most recent ones #4570devnchill wants to merge 7 commits intoOWASP:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 12 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughAdds a client-side toggle to limit visible Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
On contribution guidelines, it is mentioned that if pr implements new feature, will have to also write tests for it, so ig I don't have to be assigned again for tests and can instead write tests directly in this same pr? |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/community/snapshots/`[id]/page.tsx:
- Around line 36-40: The effect that currently only sets showAllReleases to true
must reset the toggle when the incoming snapshot changes: update the useEffect
that watches snapshot so it sets setShowAllReleases(snapshot?.newReleases &&
snapshot.newReleases.length <= MAX_RELEASES_TO_SHOW) (or explicitly set true
when length <= MAX_RELEASES_TO_SHOW and false otherwise) so the
collapsed/expanded state is recomputed on each new snapshot; reference the
existing useEffect, snapshot, newReleases, MAX_RELEASES_TO_SHOW, and
setShowAllReleases symbols when making the change.
- Around line 107-110: The classname in the showButton object contains a typo
'darK:hover:text-white' that prevents Tailwind's dark variant from applying;
update the classname string in the showButton definition to use the correct
lowercase variant 'dark:hover:text-white' so the dark-mode hover style is
applied (locate the showButton constant and fix the classname property).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 918a8d62-d85b-4af0-ab90-fe466fbfb6c8
📒 Files selected for processing (1)
frontend/src/app/community/snapshots/[id]/page.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 4/5
- This PR is likely safe to merge, with minimal risk: the reported issue is a localized styling typo rather than a functional or data-impacting bug.
- In
frontend/src/app/community/snapshots/[id]/page.tsx,darK:hover:text-whiteshould bedark:hover:text-white; because Tailwind variants are case-sensitive, the dark-mode hover text color class is silently ignored. - User impact appears limited to dark-mode hover visuals on that page, so this is a minor UX regression rather than a merge-blocking defect.
- Pay close attention to
frontend/src/app/community/snapshots/[id]/page.tsx- fix thedark:variant casing to restore intended dark-mode hover styling.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/app/community/snapshots/[id]/page.tsx">
<violation number="1" location="frontend/src/app/community/snapshots/[id]/page.tsx:110">
P2: `darK:hover:text-white` has an uppercase `K` — Tailwind's `dark:` variant is case-sensitive and must be all-lowercase. This typo silently drops the dark-mode hover style.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
devnchill
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
8df6804
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/components/SnapshotReleaseSection.tsx (2)
24-31: Remove the redundant cast and key fallback.
releaseis alreadyReleaseType, andRelease.idis required infrontend/src/types/release.ts, so theunknowncast and fallback key only hide contract regressions.Proposed cleanup
<Release - key={ - release.id || `${release.tagName}-${release.repositoryName ?? 'unknown'}-${index}` - } - release={release as unknown as ReleaseType} + key={release.id} + release={release} showAvatar={true} index={index} />Please verify with the existing TypeScript check after removing the cast.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SnapshotReleaseSection.tsx` around lines 24 - 31, Remove the redundant type cast and fallback key: pass release directly (no "as unknown as ReleaseType") and use release.id as the key (remove the `${release.tagName}...` fallback), since ReleaseType and Release.id are required; update the JSX in the Release component invocation (the Release element, props `release` and `key`) and run the TypeScript check to verify there are no type errors.
12-15: Make the toggle state clearer for assistive tech.Use release-specific button text and expose the expanded state.
Proposed accessibility refinement
const showButton = { - label: showAll ? 'show less' : 'show all', + label: showAll ? 'Show less releases' : 'Show all releases', classname: 'dark:hover:text-white rounded-md border-1 font-light border-blue-400 p-2 text-blue-400 hover:bg-blue-500 hover:text-white', }- <button className={showButton.classname} type="button" onClick={onToggle}> + <button + aria-expanded={showAll} + className={showButton.classname} + type="button" + onClick={onToggle} + > {showButton.label} </button>Also applies to: 35-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/SnapshotReleaseSection.tsx` around lines 12 - 15, The toggle button in SnapshotReleaseSection currently uses a generic label and doesn't expose expanded state; update the showButton logic (and the similar button block around lines 35-39) to produce release-specific labels (e.g., include the release name or version via the component prop/state used in SnapshotReleaseSection) and ensure the rendered button exposes its expanded state for assistive tech by adding aria-expanded={showAll} and an aria-controls that points to the collapsible content's id; adjust classname generation only as needed and keep the same button element (refer to showButton and showAll) so screen readers read distinct, contextual labels and know when the panel is expanded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/components/SnapshotReleaseSection.tsx`:
- Around line 24-31: Remove the redundant type cast and fallback key: pass
release directly (no "as unknown as ReleaseType") and use release.id as the key
(remove the `${release.tagName}...` fallback), since ReleaseType and Release.id
are required; update the JSX in the Release component invocation (the Release
element, props `release` and `key`) and run the TypeScript check to verify there
are no type errors.
- Around line 12-15: The toggle button in SnapshotReleaseSection currently uses
a generic label and doesn't expose expanded state; update the showButton logic
(and the similar button block around lines 35-39) to produce release-specific
labels (e.g., include the release name or version via the component prop/state
used in SnapshotReleaseSection) and ensure the rendered button exposes its
expanded state for assistive tech by adding aria-expanded={showAll} and an
aria-controls that points to the collapsible content's id; adjust classname
generation only as needed and keep the same button element (refer to showButton
and showAll) so screen readers read distinct, contextual labels and know when
the panel is expanded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6d748d3-9647-4744-85bf-f369199df8a2
📒 Files selected for processing (2)
frontend/src/app/community/snapshots/[id]/page.tsxfrontend/src/components/SnapshotReleaseSection.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/community/snapshots/[id]/page.tsx
devnchill
dismissed stale reviews from cubic-dev-ai[bot] and coderabbitai[bot]
via
73e74da
This makes it easy to test this component
devnchill
force-pushed
the
feat/limit-release
branch
from
73e74da to
102607c
Compare
|
1 participant
Resolves #4560
Earlier the snapshots page would display all release at once making the ui look overwhelming.
This pr:
9most recent releasesshow allbutton which on clicking would show all the releasesshow lesswhich would again collapse it to show 9 most recent ones.Checklist
make check-testlocally: all warnings addressed, tests passed