feat: 趋势页增强 - RSS数据源、描述字段、README预览、时间范围筛选#96
Merged
AmintaCCCP merged 3 commits intomainfrom Apr 23, 2026
Merged
Conversation
1. 趋势频道名称从'热门仓库(Trending)'改为'趋势(Trending)' 2. 趋势页数据源改为 GitHubTrendingRSS，支持今日/本周/本月筛选 - 今日: https://mshibanami.github.io/GitHubTrendingRSS/daily/all.xml - 本周: https://mshibanami.github.io/GitHubTrendingRSS/weekly/all.xml - 本月: https://mshibanami.github.io/GitHubTrendingRSS/monthly/all.xml 3. RSS描述字段解析：剥离XML/HTML标签，仅显示纯文本描述 4. 通过GitHub API补充RSS缺失字段（id, language, description, topics等） 5. 仓库卡片已支持描述字段显示（原始描述+AI分析摘要） 6. 点击仓库卡片可预览README.md，复用仓库页的Markdown渲染组件 7. 新增TrendingTimeRange类型和store状态管理
Contributor
📝 WalkthroughWalkthroughAdds a selectable trending time range (daily/weekly/monthly) to state and UI, threads it into the trending data fetch path, switches trending fetch to GitHubTrendingRSS with RSS parsing and supplemental /repos lookups, and triggers a non-appended reload when the time range changes. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as DiscoveryView (UI)
participant Store as useAppStore
participant API as GitHubApiService
participant RSS as GitHubTrendingRSS
User->>UI: Select time range (daily/weekly/monthly)
UI->>Store: setTrendingTimeRange(range)
Store->>Store: Update trendingTimeRange
Store-->>UI: State updated
UI->>UI: Effect detects trendingTimeRange change
UI->>API: getTrendingRepositories(platform='trending', page=1, perPage=20, timeRange)
API->>RSS: Fetch RSS feed for selected timeRange
RSS-->>API: Return feed items
API->>API: Parse items -> construct DiscoveryRepo list
API->>API: Throttled calls to /repos/{owner}/{repo} for missing fields
API-->>UI: Return paginated trending results
UI->>User: Render updated trending repositories
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/DiscoveryView.tsx (1)
2-39:⚠️ Potential issue | 🔴 CriticalAdd missing
CalendarandTrendingTimeRangeimports.Both symbols are used in the new time-range dropdown (lines 1068, 1071) but not imported, causing TypeScript compilation to fail.
Import fix
Globe, - X + X, + Calendar } from 'lucide-react'; @@ SortBy, SortOrder, - TopicCategory + TopicCategory, + TrendingTimeRange } from '../types';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 2 - 39, The build fails because the DiscoveryView component uses the Calendar icon and the TrendingTimeRange type in the time-range dropdown but they were not imported; add Calendar to the lucide-react import list (alongside RefreshCw, TrendingUp, etc.) and import TrendingTimeRange from the types module (alongside DiscoveryChannelId, DiscoveryRepo, SortBy, etc.) so both symbols (Calendar and TrendingTimeRange) are available to the dropdown logic in DiscoveryView.src/services/githubApi.ts (1)
1-16:⚠️ Potential issue | 🔴 CriticalImport
TrendingTimeRangeandDiscoveryRepofrom../types.These types are used on lines 497-499 and 518 but are not imported, causing TypeScript compilation to fail.
Fix
import { Repository, Release, GitHubUser, DiscoveryPlatform, + DiscoveryRepo, ProgrammingLanguage, SortBy, SortOrder, PaginatedDiscoveryRepositories, DiscoveryChannelId, + TrendingTimeRange, TopicCategory, SubscriptionRepo, SubscriptionDev, GitHubSearchUserResponse, GitHubUserDetail } from '../types';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 1 - 16, The file's top import block in src/services/githubApi.ts is missing two types used later (TrendingTimeRange and DiscoveryRepo); update the import list that currently includes Repository, Release, GitHubUser, etc. to also import TrendingTimeRange and DiscoveryRepo from '../types' so references to TrendingTimeRange (used around the trending-related functions) and DiscoveryRepo (used where discovery repositories are typed) resolve and TypeScript compiles.
🤖 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 760-765: The useEffect that reacts to trendingTimeRange changes
calls refreshChannel('trending', 1, false) but does not reset the pagination
state, so currentPage remains at the previous value; update the effect to also
reset the pagination state (e.g., call setCurrentPage(1) or the component's
pagination state setter) before/when invoking refreshChannel so the UI uses page
1 results for the new trendingTimeRange; locate the useEffect that references
trendingTimeRange, selectedDiscoveryChannel and refreshChannel and add the
currentPage reset there (or modify refreshChannel to accept a flag that resets
currentPage internally).
In `@src/services/githubApi.ts`:
- Around line 548-609: The supplemental-update block leaves r.id as 0 on API
failure and spawns all requests at once; change
Promise.all(reposNeedUpdate.map(...)) to a sequential or concurrency-limited
loop (e.g., for...of with await or use a p-limit) so the await setTimeout(...)
actually throttles requests, and inside the catch ensure you assign a stable,
unique fallback id instead of 0 (e.g., set r.id to a generated negative or
composite id using r.rank, channel or a monotonic counter) so React keys/store
updates won't break; keep the rest of the makeRequest(...) handling and the
existing field fallbacks but move the 80ms delay into the sequential flow so it
limits bursts.
- Around line 527-535: The current RSS description decoding uses a temporary DOM
node and tempDiv.innerHTML which risks executing untrusted markup; replace that
logic by feeding the raw description string (from descriptionEl?.textContent)
into a standalone DOMParser (parseFromString(..., 'text/html')) and extract the
safe text via the parsedDocument.body.textContent (or textContent fallback) to
decode entities and strip tags without touching document.innerHTML; update the
block around descriptionEl, tempDiv and description to use DOMParser and remove
creation/appending of any DOM nodes.
---
Outside diff comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 2-39: The build fails because the DiscoveryView component uses the
Calendar icon and the TrendingTimeRange type in the time-range dropdown but they
were not imported; add Calendar to the lucide-react import list (alongside
RefreshCw, TrendingUp, etc.) and import TrendingTimeRange from the types module
(alongside DiscoveryChannelId, DiscoveryRepo, SortBy, etc.) so both symbols
(Calendar and TrendingTimeRange) are available to the dropdown logic in
DiscoveryView.
In `@src/services/githubApi.ts`:
- Around line 1-16: The file's top import block in src/services/githubApi.ts is
missing two types used later (TrendingTimeRange and DiscoveryRepo); update the
import list that currently includes Repository, Release, GitHubUser, etc. to
also import TrendingTimeRange and DiscoveryRepo from '../types' so references to
TrendingTimeRange (used around the trending-related functions) and DiscoveryRepo
(used where discovery repositories are typed) resolve and TypeScript compiles.
🪄 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: f8d00e10-8227-426b-9337-676727b37cf1
📒 Files selected for processing (4)
src/components/DiscoveryView.tsxsrc/services/githubApi.tssrc/store/useAppStore.tssrc/types/index.ts
Comment on lines
+760
to
+765
| // 趋势时间范围改变时刷新数据 | ||
| useEffect(() => { | ||
| if (selectedDiscoveryChannel === 'trending' && trendingTimeRange) { | ||
| refreshChannel('trending', 1, false); | ||
| } | ||
| }, [trendingTimeRange, selectedDiscoveryChannel, refreshChannel]); |
Contributor
There was a problem hiding this comment.
Reset pagination when the trending range changes.
This fetches page 1 for the new range but leaves currentPage unchanged. If the user switches ranges while on page 2+, the UI slices the new page-1 results from the old page offset and can render an empty page.
🐛 Proposed pagination fix
// 趋势时间范围改变时刷新数据
useEffect(() => {
if (selectedDiscoveryChannel === 'trending' && trendingTimeRange) {
+ setCurrentPage(1);
refreshChannel('trending', 1, false);
}
}, [trendingTimeRange, selectedDiscoveryChannel, refreshChannel]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DiscoveryView.tsx` around lines 760 - 765, The useEffect that
reacts to trendingTimeRange changes calls refreshChannel('trending', 1, false)
but does not reset the pagination state, so currentPage remains at the previous
value; update the effect to also reset the pagination state (e.g., call
setCurrentPage(1) or the component's pagination state setter) before/when
invoking refreshChannel so the UI uses page 1 results for the new
trendingTimeRange; locate the useEffect that references trendingTimeRange,
selectedDiscoveryChannel and refreshChannel and add the currentPage reset there
(or modify refreshChannel to accept a flag that resets currentPage internally).
Comment on lines
+527
to
+535
| // Parse description - strip XML/HTML tags | ||
| const descriptionEl = item.querySelector('description'); | ||
| let description = descriptionEl?.textContent || ''; | ||
| // Decode HTML entities and strip HTML tags | ||
| const tempDiv = document.createElement('div'); | ||
| tempDiv.innerHTML = description; | ||
| description = tempDiv.textContent || tempDiv.innerText || ''; | ||
| // Clean up extra whitespace | ||
| description = description.replace(/\s+/g, ' ').trim(); |
Contributor
There was a problem hiding this comment.
Avoid innerHTML for external RSS content decoding.
The RSS description is untrusted external content. Using innerHTML creates a DOM node that could execute scripts if the feed is compromised. Use DOMParser with a sandboxed context to safely decode HTML entities and extract text content instead.
🛡️ Safer decode approach
- // Decode HTML entities and strip HTML tags
- const tempDiv = document.createElement('div');
- tempDiv.innerHTML = description;
- description = tempDiv.textContent || tempDiv.innerText || '';
+ // Decode HTML entities and strip HTML tags without assigning untrusted markup to the live DOM
+ const decodedDoc = new DOMParser().parseFromString(description, 'text/html');
+ description = decodedDoc.body.textContent || '';📝 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.
Suggested change
| // Parse description - strip XML/HTML tags | |
| const descriptionEl = item.querySelector('description'); | |
| let description = descriptionEl?.textContent || ''; | |
| // Decode HTML entities and strip HTML tags | |
| const tempDiv = document.createElement('div'); | |
| tempDiv.innerHTML = description; | |
| description = tempDiv.textContent || tempDiv.innerText || ''; | |
| // Clean up extra whitespace | |
| description = description.replace(/\s+/g, ' ').trim(); | |
| // Parse description - strip XML/HTML tags | |
| const descriptionEl = item.querySelector('description'); | |
| let description = descriptionEl?.textContent || ''; | |
| // Decode HTML entities and strip HTML tags without assigning untrusted markup to the live DOM | |
| const decodedDoc = new DOMParser().parseFromString(description, 'text/html'); | |
| description = decodedDoc.body.textContent || ''; | |
| // Clean up extra whitespace | |
| description = description.replace(/\s+/g, ' ').trim(); |
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 531-531: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: tempDiv.innerHTML = description
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 531-531: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: tempDiv.innerHTML = description
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/githubApi.ts` around lines 527 - 535, The current RSS
description decoding uses a temporary DOM node and tempDiv.innerHTML which risks
executing untrusted markup; replace that logic by feeding the raw description
string (from descriptionEl?.textContent) into a standalone DOMParser
(parseFromString(..., 'text/html')) and extract the safe text via the
parsedDocument.body.textContent (or textContent fallback) to decode entities and
strip tags without touching document.innerHTML; update the block around
descriptionEl, tempDiv and description to use DOMParser and remove
creation/appending of any DOM nodes.
Comment on lines
+548
to
+609
| repos.push({ | ||
| id: 0, // will be filled by GitHub API | ||
| name: repoName, | ||
| full_name: `${owner}/${repoName}`, | ||
| description: description, | ||
| html_url: link, | ||
| stargazers_count: stars, | ||
| forks_count: forks, | ||
| forks: forks, | ||
| language: null, | ||
| created_at: new Date().toISOString(), | ||
| updated_at: new Date().toISOString(), | ||
| pushed_at: new Date().toISOString(), | ||
| owner: { | ||
| login: owner, | ||
| avatar_url: `https://github.com/${owner}.png`, | ||
| }, | ||
| topics: [], | ||
| rank: i + 1, | ||
| channel: 'trending' as DiscoveryChannelId, | ||
| platform, | ||
| }); | ||
| } | ||
|
|
||
| // Supplement missing fields via GitHub API | ||
| const reposNeedUpdate = repos.filter(r => r.id === 0 || r.stargazers_count === 0 || r.forks_count === 0 || !r.language); | ||
| if (reposNeedUpdate.length > 0) { | ||
| await Promise.all(reposNeedUpdate.map(async (r) => { | ||
| try { | ||
| const [owner, repo] = r.full_name.split('/'); | ||
| if (!owner || !repo) return; | ||
| const data = await this.makeRequest<{ | ||
| id: number; | ||
| stargazers_count: number; | ||
| forks_count: number; | ||
| forks: number; | ||
| language: string | null; | ||
| description: string | null; | ||
| topics: string[]; | ||
| created_at: string; | ||
| updated_at: string; | ||
| pushed_at: string; | ||
| }>(`/repos/${owner}/${repo}`); | ||
| r.id = data.id; | ||
| r.stargazers_count = data.stargazers_count ?? r.stargazers_count; | ||
| r.forks_count = data.forks_count ?? r.forks_count; | ||
| r.forks = data.forks ?? r.forks; | ||
| r.language = data.language ?? r.language; | ||
| r.topics = data.topics ?? r.topics; | ||
| r.created_at = data.created_at ?? r.created_at; | ||
| r.updated_at = data.updated_at ?? r.updated_at; | ||
| r.pushed_at = data.pushed_at ?? r.pushed_at; | ||
| // Use GitHub API description as fallback (RSS description may contain emoji markers) | ||
| if (data.description) { | ||
| r.description = data.description; | ||
| } | ||
| } catch (e) { | ||
| console.warn(`Failed to fetch repo details for ${r.full_name}:`, e); | ||
| } | ||
| // Avoid GitHub API rate limiting | ||
| await new Promise(resolve => setTimeout(resolve, 80)); | ||
| })); |
Contributor
There was a problem hiding this comment.
Keep fallback IDs unique and actually throttle supplemental GitHub calls.
If /repos/{owner}/{repo} fails, every repo keeps id: 0, which breaks React keys and store updates by id. Also, Promise.all starts all requests immediately, so the delay at Line 608 does not throttle request bursts.
🐛 Proposed reliability fix
repos.push({
- id: 0, // will be filled by GitHub API
+ id: -(i + 1), // temporary unique id; replaced by GitHub API when enrichment succeeds
name: repoName,
full_name: `${owner}/${repoName}`,
@@
- const reposNeedUpdate = repos.filter(r => r.id === 0 || r.stargazers_count === 0 || r.forks_count === 0 || !r.language);
+ const reposNeedUpdate = repos.filter(r => r.id < 0 || r.stargazers_count === 0 || r.forks_count === 0 || !r.language);
if (reposNeedUpdate.length > 0) {
- await Promise.all(reposNeedUpdate.map(async (r) => {
+ for (const r of reposNeedUpdate) {
try {
const [owner, repo] = r.full_name.split('/');
- if (!owner || !repo) return;
+ if (!owner || !repo) continue;
const data = await this.makeRequest<{
id: number;
@@
} catch (e) {
console.warn(`Failed to fetch repo details for ${r.full_name}:`, e);
}
// Avoid GitHub API rate limiting
await new Promise(resolve => setTimeout(resolve, 80));
- }));
+ }
}📝 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.
Suggested change
| repos.push({ | |
| id: 0, // will be filled by GitHub API | |
| name: repoName, | |
| full_name: `${owner}/${repoName}`, | |
| description: description, | |
| html_url: link, | |
| stargazers_count: stars, | |
| forks_count: forks, | |
| forks: forks, | |
| language: null, | |
| created_at: new Date().toISOString(), | |
| updated_at: new Date().toISOString(), | |
| pushed_at: new Date().toISOString(), | |
| owner: { | |
| login: owner, | |
| avatar_url: `https://github.com/${owner}.png`, | |
| }, | |
| topics: [], | |
| rank: i + 1, | |
| channel: 'trending' as DiscoveryChannelId, | |
| platform, | |
| }); | |
| } | |
| // Supplement missing fields via GitHub API | |
| const reposNeedUpdate = repos.filter(r => r.id === 0 || r.stargazers_count === 0 || r.forks_count === 0 || !r.language); | |
| if (reposNeedUpdate.length > 0) { | |
| await Promise.all(reposNeedUpdate.map(async (r) => { | |
| try { | |
| const [owner, repo] = r.full_name.split('/'); | |
| if (!owner || !repo) return; | |
| const data = await this.makeRequest<{ | |
| id: number; | |
| stargazers_count: number; | |
| forks_count: number; | |
| forks: number; | |
| language: string | null; | |
| description: string | null; | |
| topics: string[]; | |
| created_at: string; | |
| updated_at: string; | |
| pushed_at: string; | |
| }>(`/repos/${owner}/${repo}`); | |
| r.id = data.id; | |
| r.stargazers_count = data.stargazers_count ?? r.stargazers_count; | |
| r.forks_count = data.forks_count ?? r.forks_count; | |
| r.forks = data.forks ?? r.forks; | |
| r.language = data.language ?? r.language; | |
| r.topics = data.topics ?? r.topics; | |
| r.created_at = data.created_at ?? r.created_at; | |
| r.updated_at = data.updated_at ?? r.updated_at; | |
| r.pushed_at = data.pushed_at ?? r.pushed_at; | |
| // Use GitHub API description as fallback (RSS description may contain emoji markers) | |
| if (data.description) { | |
| r.description = data.description; | |
| } | |
| } catch (e) { | |
| console.warn(`Failed to fetch repo details for ${r.full_name}:`, e); | |
| } | |
| // Avoid GitHub API rate limiting | |
| await new Promise(resolve => setTimeout(resolve, 80)); | |
| })); | |
| repos.push({ | |
| id: -(i + 1), // temporary unique id; replaced by GitHub API when enrichment succeeds | |
| name: repoName, | |
| full_name: `${owner}/${repoName}`, | |
| description: description, | |
| html_url: link, | |
| stargazers_count: stars, | |
| forks_count: forks, | |
| forks: forks, | |
| language: null, | |
| created_at: new Date().toISOString(), | |
| updated_at: new Date().toISOString(), | |
| pushed_at: new Date().toISOString(), | |
| owner: { | |
| login: owner, | |
| avatar_url: `https://github.com/${owner}.png`, | |
| }, | |
| topics: [], | |
| rank: i + 1, | |
| channel: 'trending' as DiscoveryChannelId, | |
| platform, | |
| }); | |
| } | |
| // Supplement missing fields via GitHub API | |
| const reposNeedUpdate = repos.filter(r => r.id < 0 || r.stargazers_count === 0 || r.forks_count === 0 || !r.language); | |
| if (reposNeedUpdate.length > 0) { | |
| for (const r of reposNeedUpdate) { | |
| try { | |
| const [owner, repo] = r.full_name.split('/'); | |
| if (!owner || !repo) continue; | |
| const data = await this.makeRequest<{ | |
| id: number; | |
| stargazers_count: number; | |
| forks_count: number; | |
| forks: number; | |
| language: string | null; | |
| description: string | null; | |
| topics: string[]; | |
| created_at: string; | |
| updated_at: string; | |
| pushed_at: string; | |
| }>(`/repos/${owner}/${repo}`); | |
| r.id = data.id; | |
| r.stargazers_count = data.stargazers_count ?? r.stargazers_count; | |
| r.forks_count = data.forks_count ?? r.forks_count; | |
| r.forks = data.forks ?? r.forks; | |
| r.language = data.language ?? r.language; | |
| r.topics = data.topics ?? r.topics; | |
| r.created_at = data.created_at ?? r.created_at; | |
| r.updated_at = data.updated_at ?? r.updated_at; | |
| r.pushed_at = data.pushed_at ?? r.pushed_at; | |
| // Use GitHub API description as fallback (RSS description may contain emoji markers) | |
| if (data.description) { | |
| r.description = data.description; | |
| } | |
| } catch (e) { | |
| console.warn(`Failed to fetch repo details for ${r.full_name}:`, e); | |
| } | |
| // Avoid GitHub API rate limiting | |
| await new Promise(resolve => setTimeout(resolve, 80)); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/githubApi.ts` around lines 548 - 609, The supplemental-update
block leaves r.id as 0 on API failure and spawns all requests at once; change
Promise.all(reposNeedUpdate.map(...)) to a sequential or concurrency-limited
loop (e.g., for...of with await or use a p-limit) so the await setTimeout(...)
actually throttles requests, and inside the catch ensure you assign a stable,
unique fallback id instead of 0 (e.g., set r.id to a generated negative or
composite id using r.rank, channel or a monotonic counter) so React keys/store
updates won't break; keep the rest of the makeRequest(...) handling and the
existing field fallbacks but move the 80ms delay into the sequential flow so it
limits bursts.
added 2 commits
April 23, 2026 20:07
- 补充 DiscoveryView 中遗漏的 Calendar lucide-react 导入（修复运行时 Calendar is not defined 报错） - 趋势频道中文名从'趋势(Trending)'修正为'趋势'，英文名保持'Trending'
1. 修复发现频道迁移逻辑，强制使用默认name/nameEn/description - 只保留用户enabled状态，其他字段从defaultChannel获取 - 解决旧数据'name: 热门仓库'覆盖新name问题 2. 修复SubscriptionRepoCard在desktopSafeMode下的功能 - 移除描述/AI摘要/标签的desktopSafeMode限制 - 卡片点击打开README不再受desktopSafeMode限制 - ReadmeModal在desktop模式下也可正常渲染
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/SubscriptionRepoCard.tsx (1)
282-293: Desktop mode: clickable card without visual affordance.Now that
handleCardClickalways opens the README modal regardless ofdesktopSafeMode, the card is clickable in desktop mode but still renders withoutcursor-pointeror hover styles (lines 285–288). Consider also applyingcursor-pointerin thedesktopSafeModebranch so users get a visual hint that the card is interactive.💡 Proposed tweak
className={`bg-white dark:bg-gray-800 border border-gray-200 dark:border-gray-700 p-5 transition-shadow duration-200 ${ desktopSafeMode - ? 'rounded-lg' + ? 'rounded-lg cursor-pointer' : 'rounded-xl hover:shadow-lg hover:border-blue-300 dark:hover:border-blue-600 cursor-pointer' }`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/SubscriptionRepoCard.tsx` around lines 282 - 293, The card is clickable because handleCardClick always opens the README modal, but when desktopSafeMode is true the class branch omits cursor-pointer/hover affordances; update the className logic for the container (where desktopSafeMode is checked) to include visual affordances (at minimum add cursor-pointer and a subtle hover style) in the desktopSafeMode branch so users know the card is interactive; ensure you modify the same JSX element that sets className (the div using desktopSafeMode) so both branches present consistent click affordance when handleCardClick is enabled.src/store/useAppStore.ts (1)
203-246:trendingTimeRangeresets to'weekly'on every reload.
trendingTimeRangeis absent from bothPersistedAppState(Pick at lines 203–246) andpartialize(lines 1230–1302), so it is never written to storage. In addition,normalizePersistedStateunconditionally sets it to'weekly'at line 410 (placed after...safePersisted), so even if a value were ever present in storage it would be discarded on merge. Net effect: a user's daily/monthly selection is lost on every page reload / app restart, while other discovery filters (discoveryPlatform,discoveryLanguage,discoverySortBy,discoverySortOrder, …) are persisted. Confirm whether session-only behavior is intentional; if not, add it to the persisted keys and stop hard-overriding it in normalize.♻️ Proposed fix (if persistence is desired)
type PersistedAppState = Partial< Pick< AppState, ... | 'discoverySortBy' | 'discoverySortOrder' + | 'trendingTimeRange' | 'subscriptionRepos' ... > > & {- trendingTimeRange: 'weekly' as TrendingTimeRange, + trendingTimeRange: (['daily', 'weekly', 'monthly'] as const).includes( + safePersisted.trendingTimeRange as TrendingTimeRange + ) + ? (safePersisted.trendingTimeRange as TrendingTimeRange) + : 'weekly',discoverySortOrder: state.discoverySortOrder, discoverySelectedTopic: state.discoverySelectedTopic, + trendingTimeRange: state.trendingTimeRange, }),Also applies to: 410-410, 664-664, 1230-1302
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/useAppStore.ts` around lines 203 - 246, trendingTimeRange is missing from the persisted keys and is being force-reset in normalizePersistedState, so user selection is never stored; to fix, add 'trendingTimeRange' to the PersistedAppState Pick and include it in the partialize persistence whitelist (the partialize function used for storage), and remove the unconditional assignment that sets trendingTimeRange = 'weekly' inside normalizePersistedState (or only set that default when no value exists) so stored values are merged correctly; update references to trendingTimeRange in normalizePersistedState, partialize, and the PersistedAppState type to ensure it persists across reloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/SubscriptionRepoCard.tsx`:
- Around line 282-293: The card is clickable because handleCardClick always
opens the README modal, but when desktopSafeMode is true the class branch omits
cursor-pointer/hover affordances; update the className logic for the container
(where desktopSafeMode is checked) to include visual affordances (at minimum add
cursor-pointer and a subtle hover style) in the desktopSafeMode branch so users
know the card is interactive; ensure you modify the same JSX element that sets
className (the div using desktopSafeMode) so both branches present consistent
click affordance when handleCardClick is enabled.
In `@src/store/useAppStore.ts`:
- Around line 203-246: trendingTimeRange is missing from the persisted keys and
is being force-reset in normalizePersistedState, so user selection is never
stored; to fix, add 'trendingTimeRange' to the PersistedAppState Pick and
include it in the partialize persistence whitelist (the partialize function used
for storage), and remove the unconditional assignment that sets
trendingTimeRange = 'weekly' inside normalizePersistedState (or only set that
default when no value exists) so stored values are merged correctly; update
references to trendingTimeRange in normalizePersistedState, partialize, and the
PersistedAppState type to ensure it persists across reloads.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2421bb49-34bd-4e04-ba68-87f1235b91e0
📒 Files selected for processing (2)
src/components/SubscriptionRepoCard.tsxsrc/store/useAppStore.ts
This was referenced Apr 23, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更内容
1. 趋势频道重命名
2. RSS 数据源替换
daily/all.xmlweekly/all.xml（默认）monthly/all.xml3. RSS 描述解析
<description>中的 XML/HTML 标签DOMParser+innerHTML解码 HTML 实体并剥离标签4. GitHub API 字段补充
/repos/{owner}/{repo}补充缺失字段：5. 仓库卡片描述字段
repo.description）repo.ai_summary），带紫色 Bot 图标区分6. README 预览
ReadmeModal组件，支持 Markdown 渲染、目录导航、字体大小切换技术细节
TrendingTimeRange类型（daily | weekly | monthly）trendingTimeRange和 actionsetTrendingTimeRangeSummary by CodeRabbit