Fix repository and discovery UI inconsistencies#100
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR adjusts UI styling and layout across discovery, cards, search filters, and settings: icon/text color updates, Tailwind class fixes, unified filter classes, sidebar wrapper changed from fixed to lg:sticky, and some modals now mounted via portal. No public API changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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 `@src/components/ReleaseCard.tsx`:
- Around line 97-112: The metadata block in ReleaseCard.tsx is hidden on small
screens because it uses "hidden md:flex", causing mobile to lose publish time
and asset-count; make the metadata visible on mobile by removing the "hidden"
class or by adding a compact mobile-only fallback (e.g., a small "flex"
container shown on sm screens and the existing md layout preserved). Update the
div wrapping the calendar/download info (which reads release.published_at and
uses downloadLinks, selectedFilters and matchesActiveFilters) so it renders a
compact responsive view on mobile (smaller text/icons or collapsed count) while
retaining the current md:flex layout for larger screens.
In `@src/components/RepositoryCard.tsx`:
- Line 2: The JSX uses the lucide-react icon Star (used around the
RepositoryCard component at the usage near line 969) but the named import list
at the top (the import statement that currently lists GripVertical, StarOff,
ExternalLink, ...) no longer includes Star; restore Star into that import list
from 'lucide-react' so the JSX symbol resolves (ensure it is added as a named
import alongside the other icons and there are no duplicate imports).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a2a4644-d42d-4a24-9df9-b4ff0516ec3a
📒 Files selected for processing (6)
src/components/DiscoveryView.tsxsrc/components/ReleaseCard.tsxsrc/components/RepositoryCard.tsxsrc/components/SearchBar.tsxsrc/components/SubscriptionRepoCard.tsxsrc/components/settings/DataManagementPanel.tsx
| <div className="hidden md:flex min-w-[140px] flex-col justify-center gap-2 text-xs text-gray-500 dark:text-text-tertiary"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <Calendar className="w-3.5 h-3.5" /> | ||
| <span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span> | ||
| </div> | ||
| {downloadLinks.length > 0 && ( | ||
| <div className="flex items-center gap-1.5"> | ||
| <Download className="w-3.5 h-3.5" /> | ||
| <span> | ||
| {selectedFilters.length > 0 | ||
| ? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}` | ||
| : downloadLinks.length} | ||
| </span> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Mobile loses release metadata after this change.
At Line 97, the metadata block is hidden md:flex, so publish time and asset-count context are not shown on small screens anymore. There is no mobile replacement in this section.
📱 Suggested fix (keep compact metadata on mobile)
<div className="hidden md:flex min-w-[140px] flex-col justify-center gap-2 text-xs text-gray-500 dark:text-text-tertiary">
<div className="flex items-center gap-1.5">
<Calendar className="w-3.5 h-3.5" />
<span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span>
</div>
{downloadLinks.length > 0 && (
<div className="flex items-center gap-1.5">
<Download className="w-3.5 h-3.5" />
<span>
{selectedFilters.length > 0
? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}`
: downloadLinks.length}
</span>
</div>
)}
</div>
+ <div className="flex md:hidden items-center gap-3 text-xs text-gray-500 dark:text-text-tertiary">
+ <div className="flex items-center gap-1">
+ <Calendar className="w-3.5 h-3.5" />
+ <span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span>
+ </div>
+ {downloadLinks.length > 0 && (
+ <div className="flex items-center gap-1">
+ <Download className="w-3.5 h-3.5" />
+ <span>
+ {selectedFilters.length > 0
+ ? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}`
+ : downloadLinks.length}
+ </span>
+ </div>
+ )}
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="hidden md:flex min-w-[140px] flex-col justify-center gap-2 text-xs text-gray-500 dark:text-text-tertiary"> | |
| <div className="flex items-center gap-1.5"> | |
| <Calendar className="w-3.5 h-3.5" /> | |
| <span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span> | |
| </div> | |
| {downloadLinks.length > 0 && ( | |
| <div className="flex items-center gap-1.5"> | |
| <Download className="w-3.5 h-3.5" /> | |
| <span> | |
| {selectedFilters.length > 0 | |
| ? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}` | |
| : downloadLinks.length} | |
| </span> | |
| </div> | |
| )} | |
| </div> | |
| <div className="hidden md:flex min-w-[140px] flex-col justify-center gap-2 text-xs text-gray-500 dark:text-text-tertiary"> | |
| <div className="flex items-center gap-1.5"> | |
| <Calendar className="w-3.5 h-3.5" /> | |
| <span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span> | |
| </div> | |
| {downloadLinks.length > 0 && ( | |
| <div className="flex items-center gap-1.5"> | |
| <Download className="w-3.5 h-3.5" /> | |
| <span> | |
| {selectedFilters.length > 0 | |
| ? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}` | |
| : downloadLinks.length} | |
| </span> | |
| </div> | |
| )} | |
| </div> | |
| <div className="flex md:hidden items-center gap-3 text-xs text-gray-500 dark:text-text-tertiary"> | |
| <div className="flex items-center gap-1"> | |
| <Calendar className="w-3.5 h-3.5" /> | |
| <span>{formatDistanceToNow(new Date(release.published_at), { addSuffix: true })}</span> | |
| </div> | |
| {downloadLinks.length > 0 && ( | |
| <div className="flex items-center gap-1"> | |
| <Download className="w-3.5 h-3.5" /> | |
| <span> | |
| {selectedFilters.length > 0 | |
| ? `${downloadLinks.filter(link => matchesActiveFilters(link.name)).length}/${downloadLinks.length}` | |
| : downloadLinks.length} | |
| </span> | |
| </div> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ReleaseCard.tsx` around lines 97 - 112, The metadata block in
ReleaseCard.tsx is hidden on small screens because it uses "hidden md:flex",
causing mobile to lose publish time and asset-count; make the metadata visible
on mobile by removing the "hidden" class or by adding a compact mobile-only
fallback (e.g., a small "flex" container shown on sm screens and the existing md
layout preserved). Update the div wrapping the calendar/download info (which
reads release.published_at and uses downloadLinks, selectedFilters and
matchesActiveFilters) so it renders a compact responsive view on mobile (smaller
text/icons or collapsed count) while retaining the current md:flex layout for
larger screens.
…issues 1. RepositoryCard: Use React portal to render edit/readme modals at document body level, preventing them from being trapped inside card container's overflow context 2. DiscoveryView: Remove self-start from sidebar sticky container to fix vertical alignment with main content 3. DiscoveryView/DiscoverySidebar: Add explicit text color classes to channel icons for proper contrast against gradient backgrounds Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/DiscoveryView.tsx (1)
43-49: Channel icon colors now diverge betweenDiscoveryViewandDiscoverySidebar.
src/components/DiscoverySidebar.tsx(lines 5–11) maps each channel to its own color (text-blue-600,text-orange-600,text-amber-600,text-emerald-600,text-violet-600withdark:variants), while this map collapses everything to white/slate-900. Visually, the same channel ends up looking different in the desktop sidebar vs. the mobile tab vs. the toolbar pill. If the design intent is the colored treatment fromDiscoverySidebar, consider centralizing one neutral/colored map and reusing it across both components (with the gradient pill being the only place that overrides to white/slate).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 43 - 49, The discoveryChannelIconMap in DiscoveryView currently uses white/slate icons causing inconsistent colors vs. DiscoverySidebar; extract a shared mapping (e.g., discoveryChannelColorMap) that maps DiscoveryChannelIcon keys to the same color class strings used in DiscoverySidebar (text-blue-600, text-orange-600, text-amber-600, text-emerald-600, text-violet-600 with dark: variants), export it from a shared module and import it into both DiscoveryView and DiscoverySidebar, then update discoveryChannelIconMap (and the Sidebar's icon map if needed) to apply those shared classes to the icon elements, leaving the gradient pill rendering to explicitly override to white/slate-900 where required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 43-49: The icons in discoveryChannelIconMap use hard-coded text
colors that are suitable for the gradient pill but not for the mobile tab nav;
split the mapping into two clear maps (e.g., discoveryChannelPillIconMap with
the existing gradient-tuned classes and discoveryChannelNeutralIconMap or reuse
the per-channel colored variants from DiscoverySidebar) and then update usages
so the toolbar pill (where currentChannelIcon is rendered around Line 918) uses
discoveryChannelPillIconMap[currentChannelIcon] while mobileChannels /
MobileTabNav (around Line 855) uses the neutral/per-channel-colored map so icons
inherit appropriate accessible colors in both light and dark themes.
---
Nitpick comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 43-49: The discoveryChannelIconMap in DiscoveryView currently uses
white/slate icons causing inconsistent colors vs. DiscoverySidebar; extract a
shared mapping (e.g., discoveryChannelColorMap) that maps DiscoveryChannelIcon
keys to the same color class strings used in DiscoverySidebar (text-blue-600,
text-orange-600, text-amber-600, text-emerald-600, text-violet-600 with dark:
variants), export it from a shared module and import it into both DiscoveryView
and DiscoverySidebar, then update discoveryChannelIconMap (and the Sidebar's
icon map if needed) to apply those shared classes to the icon elements, leaving
the gradient pill rendering to explicitly override to white/slate-900 where
required.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f930d276-2e8a-4433-8f48-27a0640b3bd3
📒 Files selected for processing (3)
src/components/DiscoverySidebar.tsxsrc/components/DiscoveryView.tsxsrc/components/RepositoryCard.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/DiscoverySidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/RepositoryCard.tsx
| const discoveryChannelIconMap: Record<DiscoveryChannelIcon, React.ReactNode> = { | ||
| trending: <TrendingUp className="w-4 h-4" />, | ||
| rocket: <Rocket className="w-4 h-4" />, | ||
| star: <Crown className="w-4 h-4" />, | ||
| tag: <Tag className="w-4 h-4" />, | ||
| search: <Search className="w-4 h-4" />, | ||
| trending: <TrendingUp className="w-4 h-4 text-white" />, | ||
| rocket: <Rocket className="w-4 h-4 text-white" />, | ||
| star: <Crown className="w-4 h-4 text-slate-900" />, | ||
| tag: <Tag className="w-4 h-4 text-slate-900" />, | ||
| search: <Search className="w-4 h-4 text-white" />, | ||
| }; |
There was a problem hiding this comment.
Hard-coded text-white / text-slate-900 regresses contrast in the mobile tab nav.
These icon nodes are designed for the gradient pill at Line 918 (where text-white over the blue/orange/violet gradients and text-slate-900 over amber/emerald gradients makes sense). However, the same discoveryChannelIconMap is reused for mobileChannels at Line 855, and MobileTabNav renders those icons over bg-light-bg95 dark:bg-panel-dark/95 — no gradient:
- Light mode:
text-whiteicons (trending / rocket / search) become nearly invisible againstbg-light-bg95. - Dark mode:
text-slate-900icons (star / tag) become nearly invisible againstbg-panel-dark/95.
Consider either (a) splitting the map into a "pill" variant (gradient-tuned, current colors) and a "neutral" variant that inherits the parent text color for mobile tabs, or (b) reusing the per-channel colored variants already defined in src/components/DiscoverySidebar.tsx (text-blue-600 dark:text-blue-400, etc.) for the mobile tabs so they remain readable in both themes.
♻️ Sketch of one possible split
-const discoveryChannelIconMap: Record<DiscoveryChannelIcon, React.ReactNode> = {
- trending: <TrendingUp className="w-4 h-4 text-white" />,
- rocket: <Rocket className="w-4 h-4 text-white" />,
- star: <Crown className="w-4 h-4 text-slate-900" />,
- tag: <Tag className="w-4 h-4 text-slate-900" />,
- search: <Search className="w-4 h-4 text-white" />,
-};
+// Used inside the gradient pill in the toolbar header only.
+const discoveryChannelPillIconMap: Record<DiscoveryChannelIcon, React.ReactNode> = {
+ trending: <TrendingUp className="w-4 h-4 text-white" />,
+ rocket: <Rocket className="w-4 h-4 text-white" />,
+ star: <Crown className="w-4 h-4 text-slate-900" />,
+ tag: <Tag className="w-4 h-4 text-slate-900" />,
+ search: <Search className="w-4 h-4 text-white" />,
+};
+
+// Used in mobile tabs / neutral surfaces — inherits theme text color.
+const discoveryChannelIconMap: Record<DiscoveryChannelIcon, React.ReactNode> = {
+ trending: <TrendingUp className="w-4 h-4" />,
+ rocket: <Rocket className="w-4 h-4" />,
+ star: <Crown className="w-4 h-4" />,
+ tag: <Tag className="w-4 h-4" />,
+ search: <Search className="w-4 h-4" />,
+};Then update the toolbar pill at Line 918–920 to use discoveryChannelPillIconMap[currentChannelIcon] while mobileChannels (Line 855) keeps using the neutral discoveryChannelIconMap.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DiscoveryView.tsx` around lines 43 - 49, The icons in
discoveryChannelIconMap use hard-coded text colors that are suitable for the
gradient pill but not for the mobile tab nav; split the mapping into two clear
maps (e.g., discoveryChannelPillIconMap with the existing gradient-tuned classes
and discoveryChannelNeutralIconMap or reuse the per-channel colored variants
from DiscoverySidebar) and then update usages so the toolbar pill (where
currentChannelIcon is rendered around Line 918) uses
discoveryChannelPillIconMap[currentChannelIcon] while mobileChannels /
MobileTabNav (around Line 855) uses the neutral/per-channel-colored map so icons
inherit appropriate accessible colors in both light and dark themes.
Use bg-brand-indigo text-white instead of bg-gray-100 text-white for better visibility in both light and dark modes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Verification
npx tsc --noEmitnpm run build(fails in current workspace because @tailwindcss/typography is missing from the environment)Summary by CodeRabbit