refactor:重构密钥卡片样式，移除未使用的代码，移除悬停效果和过渡 #20
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds statusCode propagation in worker updates; introduces reusable UI components (Card, EmptyState, FormField, IconButton, StatCard, StatusBadge); refactors Modal; reworks Results and Logs UIs; adds detailed-logs toggle with state persistence; simplifies virtualization to viewport-based heights with tests; broad CSS consolidation and removals; multiple feature UIs updated to new components. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant AdvancedSettings as AdvancedSettings
participant LogDisplay as LogDisplayControl
participant AppState as AppStateContext
participant Storage as localStorage
participant UI as Logs UI (LogsPreview/KeyLogModal)
User->>AdvancedSettings: Open Advanced Settings
AdvancedSettings->>LogDisplay: Render toggle
User->>LogDisplay: Toggle "Show detailed logs"
LogDisplay->>AppState: dispatch(SET_SHOW_DETAILED_LOGS, value)
AppState->>Storage: Persist showDetailedLogs
alt value = true
AppState->>UI: Enable log collector/render
else value = false
AppState->>UI: CLEAR_LOGS and hide gated views
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/features/Results/StatsCards.jsx (1)
52-63: Fix misaligned testing stat label translation
t('testing')resolves to the sentence “Testing API keys…”, so the new StatCard now renders a long progress message where a short status label is expected. Use the existingtestingLabeltranslation (and fall back tostat.keyotherwise) so we keep the concise label in all locales.const testingStats = [ { key: 'testing', + labelKey: 'testingLabel', value: testingCount, className: 'testing' }, { key: 'retrying', value: retryingCount, className: 'retrying' } ]; … - label={t(stat.key)} + label={t(stat.labelKey || stat.key)}Also applies to: 86-91
src/components/features/LogsPreview/LogsPreviewPanel.jsx (2)
41-47: Prevent runtime errors in search filtering when fields are missing.log.keyId or log.apiType can be undefined, causing toLowerCase to throw.
- filtered = filtered.filter(log => - log.keyId.toLowerCase().includes(searchTerm.toLowerCase()) || - log.apiType.toLowerCase().includes(searchTerm.toLowerCase()) || - (log.error?.message || '').toLowerCase().includes(searchTerm.toLowerCase()) - ); + const term = searchTerm.toLowerCase(); + filtered = filtered.filter(log => + (log.keyId || '').toLowerCase().includes(term) || + (log.apiType || '').toLowerCase().includes(term) || + (log.error?.message || '').toLowerCase().includes(term) + );
26-29: Mask API keys in UI to avoid leaking secrets.maskApiKey currently returns the full key. This risks exposing sensitive keys in logs.
- const maskApiKey = (key) => { - if (!key || typeof key !== 'string') return 'Unknown'; - return key; - }; + const maskApiKey = (key) => { + if (!key || typeof key !== 'string') return 'Unknown'; + if (key.length <= 8) return key; + return `${key.slice(0, 4)}...${key.slice(-4)}`; + };src/components/features/Results/VirtualizedList.jsx (1)
265-279: Fix per-row state leakage with react-window by setting itemKey.KeyItem holds local state (isKeyVisible). Without itemKey, react-window reuses rows and state can “jump” between items.
<List ref={listRef} height={listHeight} itemCount={filteredKeys.length} itemSize={getItemSize} itemData={filteredKeys} + itemKey={(index, data) => (data[index] && (data[index].key || data[index].id || index))} overscanCount={3} width="100%" className="virtualized-list" >
🧹 Nitpick comments (11)
public/worker.js (1)
501-519: LGTM! Clear status code extraction logic.The dual-format detection (parentheses and "HTTP " prefix) correctly handles various error message formats. The regex
/\((\d{3})\)$/properly matches trailing status codes like "认证失败 (401)".Consider making the comment on line 504 more explicit about the format it matches:
- // 匹配括号中的3位数字状态码,如 "认证失败 (401)" 或 "权限不足 (403)" + // 匹配字符串末尾括号中的3位数字状态码,如 "认证失败 (401)" 或 "权限不足 (403)"src/components/features/LogDisplayControl/index.jsx (1)
16-30: Consider adding explicit label association for better accessibility.The outer label at line 16 displays the translated text but isn't explicitly associated with the checkbox input. Consider wrapping the entire switch control (lines 21-30) with a label element or adding an
htmlForattribute pointing to a uniqueidon the checkbox input.Apply this diff to improve accessibility:
- <label className="text-sm font-medium text-primary"> + <label htmlFor="detailed-logs-toggle" className="text-sm font-medium text-primary"> {t('showDetailedLogs')} </label> <div className="space-y-sm"> <div className="flex items-center gap-sm"> <label className="switch-base"> <input + id="detailed-logs-toggle" type="checkbox" checked={state.showDetailedLogs} onChange={handleToggleDetailedLogs} disabled={state.isTesting} className="switch-input" /> <span className="switch-slider"></span> </label>src/components/features/ConcurrencyControl/index.jsx (2)
31-48: Consider UX inconsistency between slider and numeric input ranges.The slider is capped at 50 (line 35), but the numeric input allows values up to 100 (line 45). When a user enters a value greater than 50 via the numeric input, the slider will be maxed out at 50, which could be confusing. Consider either:
- Aligning both to the same max value (e.g., both 100)
- Updating the slider's max dynamically based on the current value
- Adding a clear indicator that values above 50 require manual entry
13-19: Direct DOM mutation in event handler is unusual.Lines 14-18 directly mutate
e.target.valueafter validation. While this works, it's not idiomatic in React. The controlled input pattern typically lets React manage the input value through state. The current approach could cause confusion during debugging.Consider this alternative pattern:
const handleInputChange = (e) => { let value = parseInt(e.target.value); if (isNaN(value) || value < 1) value = 1; if (value > 100) value = 100; - e.target.value = value; updateConcurrency(value); };The input will reflect the correct value through the controlled
valueprop at line 43.src/components/common/EmptyState/index.jsx (1)
21-25: Redundant conditional expression.Line 23 has a no-op ternary:
typeof icon === 'string' ? icon : iconalways returnsiconregardless of the type check. This was likely intended to handle different icon types differently, but currently serves no purpose.If the intent is to simply render the icon (whether string emoji or JSX), simplify to:
{icon && ( <div className="empty-icon"> - {typeof icon === 'string' ? icon : icon} + {icon} </div> )}If you intended to wrap JSX icons differently from string emojis, clarify the intended behavior.
src/components/features/ConcurrencyControl/ConcurrencyControl.module.css (1)
71-71: Consider completing the CSS variable migration.While many colors have been migrated to CSS variables, some hardcoded colors remain:
- Lines 71, 81: Slider thumb uses
#495057- Lines 179, 184: Dark theme slider thumb uses
#ffffffFor full theming consistency, consider replacing these with CSS variables like
var(--slider-thumb-color)or similar.Also applies to: 81-81, 179-179, 184-184
src/components/features/AdvancedSettings/index.jsx (1)
45-61: Add dialog semantics for accessibility (role and aria-modal).The modal root lacks dialog attributes. Add role="dialog" and aria-modal="true" to improve a11y and announce the modal to assistive tech.
- <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center" style={{zIndex: 9999}} onClick={handleOverlayClick}> + <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center" + style={{ zIndex: 9999 }} + role="dialog" + aria-modal="true" + onClick={handleOverlayClick}>src/components/common/FormField/index.jsx (1)
41-77: Unused prop and a11y hints in Input subcomponent.
- The variant prop isn’t used (only textarea class toggles). Either implement variants or remove the prop to avoid confusion.
- Consider mirroring aria-invalid/aria-describedby handling at the Input level when used standalone (outside FormField).
src/components/features/LogsPreview/LogsPreviewPanel.jsx (1)
116-119: PR objective alignment: remove transition class.The container still uses transition-all while the PR aims to remove transitions.
- <div className={`card-base mb-md transition-all overflow-hidden ${ + <div className={`card-base mb-md overflow-hidden ${ isExpanded ? 'max-h-96' : 'max-h-32' }`}>src/components/common/StatusBadge/index.jsx (1)
10-19: Align default status class with rest of code.VirtualizedList maps unknown statuses to 'status-unknown'. Use the same here for consistency.
- default: return 'status-testing'; + default: return 'status-unknown';src/components/features/LogsPreview/KeyLogModal.jsx (1)
194-194: Misleading use of EmptyState for loading.
EmptyStateis semantically intended for "no data" scenarios, not loading states. A loading spinner or dedicated loading component would communicate intent more clearly.Consider introducing a
Loadingcomponent or using a spinner:- <EmptyState message={t('loading') || '加载中...'} /> + <div className="flex items-center justify-center p-lg"> + <div className="loading-spinner" /> + <span className="ml-2 text-secondary">{t('loading') || '加载中...'}</span> + </div>Or reuse the existing
Loadingcomponent if it supports inline rendering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
public/worker.js(5 hunks)src/App.jsx(4 hunks)src/components/common/Button/index.jsx(2 hunks)src/components/common/Card/index.jsx(1 hunks)src/components/common/EmptyState/index.jsx(1 hunks)src/components/common/FormField/index.jsx(1 hunks)src/components/common/IconButton/index.jsx(1 hunks)src/components/common/Modal/Modal.module.css(0 hunks)src/components/common/Modal/index.jsx(3 hunks)src/components/common/StatCard/index.jsx(1 hunks)src/components/common/StatusBadge/index.jsx(1 hunks)src/components/features/AdvancedSettings/index.jsx(3 hunks)src/components/features/ApiConfig/ApiConfig.module.css(0 hunks)src/components/features/ConcurrencyControl/ConcurrencyControl.module.css(8 hunks)src/components/features/ConcurrencyControl/index.jsx(3 hunks)src/components/features/Controls/index.jsx(3 hunks)src/components/features/KeyInput/FileImport.jsx(2 hunks)src/components/features/KeyInput/KeyInput.module.css(0 hunks)src/components/features/KeyInput/PasteButton.jsx(2 hunks)src/components/features/LogDisplayControl/index.jsx(1 hunks)src/components/features/LogsPreview/KeyLogModal.jsx(6 hunks)src/components/features/LogsPreview/LogsPreviewPanel.jsx(4 hunks)src/components/features/PaidDetectionControl/PaidDetectionControl.module.css(0 hunks)src/components/features/PaidDetectionControl/index.jsx(6 hunks)src/components/features/PaidDetectionPrompt/PaidDetectionPrompt.module.css(0 hunks)src/components/features/Results/Results.module.css(0 hunks)src/components/features/Results/StatsCards.jsx(2 hunks)src/components/features/Results/VirtualizedList.jsx(5 hunks)src/components/features/Results/index.jsx(1 hunks)src/components/features/RetryControl/RetryControl.module.css(7 hunks)src/components/features/RetryControl/index.jsx(3 hunks)src/components/layout/AppLayout/AppLayout.module.css(3 hunks)src/components/layout/AppLayout/index.jsx(1 hunks)src/components/layout/NavBar/NavBar.module.css(0 hunks)src/components/layout/Sidebar/Sidebar.module.css(3 hunks)src/components/layout/ThemeToggle/ThemeToggle.module.css(0 hunks)src/components/layout/ThemeToggle/index.jsx(0 hunks)src/contexts/AppStateContext.jsx(6 hunks)src/hooks/useVirtualization.js(1 hunks)src/locales/en.json(1 hunks)src/locales/zh.json(1 hunks)src/styles/components.css(2 hunks)src/styles/globals.css(5 hunks)
💤 Files with no reviewable changes (9)
- src/components/features/PaidDetectionControl/PaidDetectionControl.module.css
- src/components/layout/NavBar/NavBar.module.css
- src/components/layout/ThemeToggle/ThemeToggle.module.css
- src/components/features/PaidDetectionPrompt/PaidDetectionPrompt.module.css
- src/components/common/Modal/Modal.module.css
- src/components/features/KeyInput/KeyInput.module.css
- src/components/features/Results/Results.module.css
- src/components/layout/ThemeToggle/index.jsx
- src/components/features/ApiConfig/ApiConfig.module.css
🧰 Additional context used
🧬 Code graph analysis (22)
src/components/features/RetryControl/index.jsx (1)
src/components/common/FormField/index.jsx (1)
FormField(3-39)
src/components/features/AdvancedSettings/index.jsx (3)
src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/IconButton/index.jsx (1)
IconButton(4-27)src/components/features/LogDisplayControl/index.jsx (1)
LogDisplayControl(5-41)
src/components/features/LogsPreview/KeyLogModal.jsx (3)
src/components/common/Modal/index.jsx (1)
Modal(7-129)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/common/Card/index.jsx (1)
Card(3-70)
src/hooks/useVirtualization.js (1)
src/components/features/Results/VirtualizedList.jsx (1)
keyData(12-12)
src/components/features/LogDisplayControl/index.jsx (2)
src/contexts/AppStateContext.jsx (2)
useAppState(524-530)useAppState(524-530)src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/features/ConcurrencyControl/index.jsx (1)
src/components/common/FormField/index.jsx (1)
FormField(3-39)
src/App.jsx (4)
src/components/common/Card/index.jsx (1)
Card(3-70)src/components/features/ApiConfig/index.jsx (1)
ApiConfig(5-14)src/components/features/KeyInput/index.jsx (1)
KeyInput(7-36)src/components/features/Results/index.jsx (1)
Results(11-83)
src/components/common/Modal/index.jsx (5)
src/components/features/AdvancedSettings/index.jsx (1)
handleOverlayClick(38-42)src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/IconButton/index.jsx (1)
IconButton(4-27)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/features/Controls/index.jsx (1)
src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/features/KeyInput/FileImport.jsx (1)
src/components/common/IconButton/index.jsx (1)
IconButton(4-27)
src/components/features/Results/index.jsx (4)
src/components/features/Results/StatsCards.jsx (1)
StatsCards(6-97)src/components/features/Results/ResultTabs.jsx (1)
ResultTabs(5-39)src/components/features/Results/VirtualizedList.jsx (1)
VirtualizedList(210-281)src/components/features/Results/CopyButtons.jsx (1)
CopyButtons(6-148)
src/components/features/KeyInput/PasteButton.jsx (1)
src/components/common/IconButton/index.jsx (1)
IconButton(4-27)
src/components/common/StatCard/index.jsx (4)
src/components/common/Card/index.jsx (2)
baseClass(42-42)classes(47-53)src/components/common/FormField/index.jsx (3)
baseClass(49-49)sizeClass(52-53)classes(55-57)src/components/common/StatusBadge/index.jsx (3)
baseClass(22-22)sizeClass(24-25)classes(27-29)src/components/common/EmptyState/index.jsx (2)
sizeClass(12-13)classes(15-17)
src/components/features/Results/VirtualizedList.jsx (4)
src/components/features/BalanceDisplay/KeyBalanceDisplay.jsx (3)
useLanguage(7-7)useAppState(8-8)KeyBalanceDisplay(6-77)src/contexts/AppStateContext.jsx (5)
useAppState(524-530)useAppState(524-530)key(25-25)key(50-50)state(370-370)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/StatusBadge/index.jsx (1)
getStatusClass(10-20)
src/components/features/PaidDetectionControl/index.jsx (3)
src/contexts/AppStateContext.jsx (1)
state(370-370)src/components/common/Button/index.jsx (1)
Button(3-40)src/components/common/Card/index.jsx (1)
Card(3-70)
src/components/common/IconButton/index.jsx (1)
src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/common/StatusBadge/index.jsx (1)
src/components/features/Results/VirtualizedList.jsx (1)
getStatusClass(19-29)
src/components/common/EmptyState/index.jsx (5)
src/components/features/Results/VirtualizedList.jsx (1)
EmptyState(203-208)src/components/common/FormField/index.jsx (2)
sizeClass(52-53)classes(55-57)src/components/common/StatCard/index.jsx (2)
sizeClass(13-13)classes(14-16)src/components/common/StatusBadge/index.jsx (2)
sizeClass(24-25)classes(27-29)src/components/common/Card/index.jsx (1)
classes(47-53)
src/components/features/Results/StatsCards.jsx (2)
src/components/common/StatCard/index.jsx (1)
StatCard(3-36)src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/common/Card/index.jsx (5)
src/components/common/Button/index.jsx (2)
baseClass(14-14)classes(21-23)src/components/common/FormField/index.jsx (2)
baseClass(49-49)classes(55-57)src/components/common/StatCard/index.jsx (2)
baseClass(12-12)classes(14-16)src/components/common/StatusBadge/index.jsx (2)
baseClass(22-22)classes(27-29)src/components/common/EmptyState/index.jsx (1)
classes(15-17)
src/components/common/FormField/index.jsx (2)
src/components/common/Card/index.jsx (2)
baseClass(42-42)classes(47-53)src/components/common/StatusBadge/index.jsx (3)
baseClass(22-22)sizeClass(24-25)classes(27-29)
src/components/features/LogsPreview/LogsPreviewPanel.jsx (2)
src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (22)
public/worker.js (1)
353-356: LGTM! Consistent statusCode surfacing across error paths.The statusCode is now consistently extracted and included in KEY_STATUS_UPDATE payloads across all finalization paths (success, max retries, non-retry, and exceptions). This provides better observability for downstream status-driven UI components.
Also applies to: 384-387, 412-415, 459-462
src/locales/zh.json (1)
209-210: LGTM! Clear locale additions for detailed logs feature.The new locale keys follow existing naming conventions and provide clear descriptions for the detailed logs toggle feature.
src/components/features/RetryControl/RetryControl.module.css (1)
1-236: LGTM! Comprehensive migration to theme-aware CSS variables.The refactor consistently replaces hard-coded colors with CSS variables (
--bg-card,--border-color,--bg-input, etc.) across both light and dark themes. This enables consistent theming while maintaining visual hierarchy.src/styles/components.css (2)
231-234: LGTM! Simplified switch toggle positioning.The change from transform-based animation to fixed positioning aligns with the PR's objective of removing transitions and hover effects.
351-518: LGTM! Well-structured card variant system.The new card variants provide a consistent design language with:
- Proper semantic naming (card--primary, card--info, etc.)
- Complete dark theme support
- Responsive mobile adaptations
- Backward compatibility aliases
Note: The fixed heights (392px for results cards) work well with the fixed-height virtualization changes in
useVirtualization.js.src/styles/globals.css (2)
296-839: LGTM! Comprehensive horizontal key item styling system.The new
.key-item-horizontallayout introduces a sophisticated status-driven design with:
- Clear visual hierarchy (status indicator → key capsule → actions)
- Complete theming for all statuses (valid, invalid, rate-limited, paid, testing, retrying)
- Proper light/dark theme support using CSS variables
- Horizontal scrolling capsule for long keys
The structure is complex but well-organized with clear sections for each status variant.
1183-1343: LGTM! Well-considered mobile responsiveness.The responsive adjustments for horizontal key items appropriately:
- Reduce component sizes while maintaining touch-friendly targets (36-40px buttons)
- Hide non-critical status info on mobile to save space (line 1216)
- Use flexible capsule widths for better adaptation
- Apply progressive enhancement across breakpoints (768px → 480px)
src/components/features/KeyInput/FileImport.jsx (1)
28-43: LGTM! Clean migration to IconButton component.The refactor properly replaces the inline button with the
IconButtoncomponent, maintaining all functionality while improving consistency with the broader component system. Thevariant="ghost"andsize="small"props are appropriate for an icon-only action button.src/App.jsx (2)
21-41: LGTM! Consistent Card component adoption.The migration to the
Cardcomponent properly:
- Uses semantic variants (
function,usage) that map to CSS definitions incomponents.css- Maintains existing layout structure and content
- Improves UI consistency across the application
The Card variants align with the new card system (
.card--primary,.card--info) introduced insrc/styles/components.css.
44-44: LGTM! UI simplification aligns with CSS refactor.The settings button class update to
btn--settingscorrectly uses the new CSS definition fromcomponents.css(line 398). The right panel simplification removes unnecessary wrapping, as theResultscomponent handles its own layout (seen in relevant snippets).Also applies to: 61-61
src/components/features/KeyInput/PasteButton.jsx (1)
4-4: LGTM! Clean refactor to shared IconButton.The migration from a native button to the shared
IconButtoncomponent is well-executed. All necessary props (icon, onClick, disabled, title, variant, size, className) are correctly passed, and the inline positioning style is preserved.Also applies to: 40-59
src/components/layout/AppLayout/index.jsx (1)
39-46: LGTM! Touch support added for mobile compatibility.The addition of a
touchstartevent listener alongsidemousedownensures the outside-click behavior works on touch devices. The cleanup correctly removes both listeners, preventing memory leaks.src/components/features/Controls/index.jsx (1)
8-8: LGTM! Clean migration to shared Button component.All three buttons have been successfully refactored to use the shared
Buttoncomponent while preserving their original behavior, icons, and labels. The dynamic variant switching for the start/cancel button based onstate.isTestingis well-implemented.Also applies to: 92-143
src/components/common/Card/index.jsx (1)
47-69: LGTM; consistent class composition and optional header.Implementation is straightforward and consistent with other components.
src/components/features/PaidDetectionControl/index.jsx (1)
73-81: LGTM; consistent with shared Button/Card and confirm Modal.Disables interactions during testing and wraps status in Card. Modal additions look correct.
Also applies to: 89-121, 132-133
src/components/features/LogsPreview/KeyLogModal.jsx (2)
198-228: LGTM: Card usage is consistent.The refactor to wrap summary sections in
Cardcomponents aligns with the PR's goal to standardize UI building blocks.
236-272: LGTM: Event rendering with Card.Each event is correctly wrapped in a
Card, maintaining a consistent visual hierarchy.src/components/features/Results/index.jsx (1)
26-38: LGTM: Conditional tooltip rendering.The tooltip is correctly shown based on
state.showLogTooltipwith proper dismiss handling.src/contexts/AppStateContext.jsx (2)
504-509: Verify user expectations for auto-clearing logs.When
showDetailedLogstoggles tofalse, all logs are immediately cleared. Users might expect logs to persist after toggling, especially if they want to temporarily disable collection while preserving existing data.Consider whether this behavior aligns with user expectations. If logs should persist, remove the auto-clear effect. If this is intentional, add a confirmation dialog or tooltip explaining that disabling will clear existing logs.
If you decide to keep auto-clear, document this behavior in the UI (e.g., in a tooltip or help text for the toggle control).
112-113: LGTM: showDetailedLogs implementation is correct.The new
showDetailedLogsstate is properly initialized from localStorage, persisted on changes, and integrated with the log collector's enable/disable mechanism.Also applies to: 208-212, 419-419, 492-492
src/components/common/Modal/index.jsx (2)
27-37: LGTM: Modal interaction handling is solid.Body scroll locking, Escape key handling, and overlay click handling are all correctly implemented with proper cleanup.
Also applies to: 39-48, 52-56
66-72: LGTM: Card-based modal structure is well-designed.The refactor to use
Cardas the modal content container, with support for confirm/alert layouts and custom children, provides a flexible and consistent foundation.Also applies to: 91-125
src/components/features/ConcurrencyControl/ConcurrencyControl.module.css
Outdated
Show resolved
Hide resolved
Deploying api-key-tester with
|
| Latest commit: |
0c68747
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ecba547f.api-key-tester-81w.pages.dev |
| Branch Preview URL: | https://experimental.api-key-tester-81w.pages.dev |
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 (1)
src/components/features/Results/VirtualizedList.jsx (1)
13-129: Visibility toggle leaks to other keys
isKeyVisibleis stored per rendered index. React Window recycles row components, so switching tabs or filtering reuses the same index with a different key while the previoustruestate sticks. The new key then renders unmasked without user intent, accidentally exposing secrets. Reset the flag whenever the key identity changes (or hoist visibility state keyed by the actual API key) so every key defaults to masked until explicitly revealed.- const [isKeyVisible, setIsKeyVisible] = useState(false); + const [isKeyVisible, setIsKeyVisible] = useState(false); + + useEffect(() => { + setIsKeyVisible(false); + }, [keyData.key]); @@ - setIsKeyVisible(!isKeyVisible); + setIsKeyVisible(prev => !prev);
♻️ Duplicate comments (3)
src/components/common/Modal/index.jsx (1)
73-95: Past issue resolved: Title rendering now handles complex elements.The
typeof title === 'string'check (lines 77-81) correctly implements the fix suggested in the previous review. String titles are wrapped in anh3tag for semantic HTML, while React elements are rendered directly, preventing invalid HTML when block-level elements are passed as titles (as inKeyLogModal.jsxlines 184-187).src/components/features/LogsPreview/KeyLogModal.jsx (1)
184-187: Past issue resolved: Title structure is now valid HTML.The title now uses a React fragment containing two
divelements. This is valid because theModalcomponent (lines 77-81 inModal/index.jsx) checkstypeof title === 'string'and only wraps string titles in anh3tag. React elements are rendered directly without theh3wrapper, avoiding the previous "div inside h3" HTML validation error.src/components/features/ConcurrencyControl/ConcurrencyControl.module.css (1)
139-143: Active state styling is now correct.The past review comment flagged that line 140 had
background: #ffffff(white) for the active state, which was incorrect. The current code showsbackground: #667eea(primary blue), which is the appropriate color for an active state. This issue has been resolved.Note: The past review comment identified this issue, and it has since been fixed. The active button now uses the primary color
#667eeainstead of white.
🧹 Nitpick comments (5)
src/styles/globals.css (2)
314-353: Consider extracting status indicator styles to reduce duplication.The status indicator styling is repeated for each of the 6 status types (valid, invalid, rate-limited, paid, testing, retrying) with only border and background colors changing. This creates ~240 lines of highly duplicated code.
Consider using CSS custom properties or a more DRY approach:
/* Base status indicator */ .key-status-indicator { display: flex; align-items: center; justify-content: center; min-width: 80px; padding: 4px 8px; border-radius: 20px; border: 1px solid var(--status-border-color, #28a745); background: var(--status-bg-color, rgba(40, 167, 69, 0.1)); } /* Status-specific colors */ .key-item-horizontal.status-valid { --status-border-color: #28a745; --status-bg-color: rgba(40, 167, 69, 0.1); } .key-item-horizontal.status-invalid { --status-border-color: #dc3545; --status-bg-color: rgba(220, 53, 69, 0.1); } /* etc. */This would reduce the 6 status indicator blocks (~42 lines each) to a single base + 6 compact overrides.
1193-1237: Mobile optimization looks good with minor accessibility concern.The mobile responsive styles are well-structured with appropriate breakpoints. However, line 1226 completely hides
.key-status-info-externalon mobile withdisplay: none. This removes potentially important status information from mobile users.Consider alternatives to complete removal:
- Truncate instead of hide:
.key-status-info-external { font-size: 9px; max-width: 40px; overflow: hidden; text-overflow: ellipsis; }
- Show on tap/focus:
.key-status-info-external { position: absolute; opacity: 0; pointer-events: none; } .key-item-horizontal:active .key-status-info-external, .key-item-horizontal:focus-within .key-status-info-external { opacity: 1; pointer-events: auto; }
- Move to tooltip if status code info is important enough.
src/components/common/FormField/index.jsx (2)
12-17: ID generation uses Math.random which is unstable across renders.While
useRefis correctly used to persist the ID reference, the initial value usesMath.random().toString(36)which can still cause issues:
Server-side rendering (SSR) mismatch: If this component is rendered server-side, the ID generated on the server will differ from the ID generated during client-side hydration, causing React hydration errors.
Snapshot testing: Tests that capture component output will have non-deterministic IDs, making snapshots fragile.
Use a more deterministic approach for ID generation:
+let fieldIdCounter = 0; + const FormField = ({ label, children, error, help, required = false, className = '', ...props }) => { - const idRef = useRef(props.id || `field-${Math.random().toString(36).substr(2, 9)}`); + const idRef = useRef(props.id || `field-${++fieldIdCounter}`); const fieldId = idRef.current;Or use a more robust approach with React's
useIdhook (React 18+):import React, { useId, useRef } from 'react'; const FormField = ({ label, children, error, help, required = false, className = '', ...props }) => { const generatedId = useId(); const fieldId = props.id || generatedId; // ... rest of component
27-35: Form control detection could miss custom components.The
isFormControlcheck (line 31) only detects intrinsic HTML elements and components withdisplayName: 'Input'. Custom form libraries (Material-UI, Chakra, etc.) use different naming conventions and won't receive proper ARIA attributes.Consider a more flexible detection approach:
- const isFormControl = ['input', 'textarea', 'select', 'Input'].includes(tag); + // Check if element has typical form control props + const isFormControl = ( + ['input', 'textarea', 'select', 'Input'].includes(tag) || + child.props.onChange || + child.props.value !== undefined || + child.props.name + );Or add a prop to explicitly mark controls:
<FormField.Input isFormControl /> <CustomSelect isFormControl />src/components/features/ConcurrencyControl/ConcurrencyControl.module.css (1)
55-63: Consider adding visual feedback for disabled sliders.The slider's thumb color is
#495057(dark gray) in light mode (line 71) and#ffffff(white) in dark mode (line 179), but there's no visual distinction when the slider is disabled beyond theopacity: 0.6at line 88. Users may not clearly recognize that the control is inactive.Add distinct disabled thumb styling:
.concurrencySlider:disabled::-webkit-slider-thumb { cursor: not-allowed; background: #adb5bd; /* Lighter gray */ box-shadow: none; } .concurrencySlider:disabled::-moz-range-thumb { cursor: not-allowed; background: #adb5bd; box-shadow: none; } :global(.dark-theme) .concurrencySlider:disabled::-webkit-slider-thumb { background: #6c757d; /* Darker gray */ } :global(.dark-theme) .concurrencySlider:disabled::-moz-range-thumb { background: #6c757d; }Also applies to: 174-176
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/components/common/FormField/index.jsx(1 hunks)src/components/common/Modal/index.jsx(3 hunks)src/components/common/StatCard/index.jsx(1 hunks)src/components/features/ConcurrencyControl/ConcurrencyControl.module.css(6 hunks)src/components/features/ConcurrencyControl/index.jsx(2 hunks)src/components/features/LogsPreview/KeyLogModal.jsx(3 hunks)src/components/features/Results/VirtualizedList.jsx(5 hunks)src/components/features/Results/index.jsx(2 hunks)src/components/features/RetryControl/index.jsx(2 hunks)src/components/layout/Sidebar/Sidebar.module.css(4 hunks)src/hooks/useVirtualization.js(1 hunks)src/styles/components.css(2 hunks)src/styles/globals.css(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/features/RetryControl/index.jsx
- src/styles/components.css
🧰 Additional context used
🧬 Code graph analysis (7)
src/components/common/Modal/index.jsx (5)
src/components/features/AdvancedSettings/index.jsx (1)
handleOverlayClick(38-42)src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/IconButton/index.jsx (1)
IconButton(4-27)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/common/FormField/index.jsx (4)
src/components/common/StatCard/index.jsx (3)
baseClass(12-12)sizeClass(13-13)classes(14-16)src/components/common/Card/index.jsx (2)
baseClass(42-42)classes(47-53)src/components/common/StatusBadge/index.jsx (3)
baseClass(22-22)sizeClass(24-25)classes(27-29)src/components/common/EmptyState/index.jsx (2)
sizeClass(12-13)classes(15-17)
src/components/features/ConcurrencyControl/index.jsx (3)
src/components/common/FormField/index.jsx (1)
FormField(3-48)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/features/RetryControl/index.jsx (2)
handleSliderChange(19-22)handleInputChange(11-17)
src/components/features/Results/index.jsx (4)
src/components/features/Results/StatsCards.jsx (1)
StatsCards(6-97)src/components/features/Results/ResultTabs.jsx (1)
ResultTabs(5-39)src/components/features/Results/VirtualizedList.jsx (2)
VirtualizedList(209-280)EmptyState(202-207)src/components/features/Results/CopyButtons.jsx (1)
CopyButtons(6-148)
src/components/common/StatCard/index.jsx (4)
src/components/common/FormField/index.jsx (3)
baseClass(58-58)sizeClass(61-62)classes(64-66)src/components/common/Card/index.jsx (2)
baseClass(42-42)classes(47-53)src/components/common/StatusBadge/index.jsx (3)
baseClass(22-22)sizeClass(24-25)classes(27-29)src/components/common/EmptyState/index.jsx (2)
sizeClass(12-13)classes(15-17)
src/components/features/Results/VirtualizedList.jsx (2)
src/components/features/BalanceDisplay/KeyBalanceDisplay.jsx (3)
useLanguage(7-7)useAppState(8-8)KeyBalanceDisplay(6-77)src/components/common/StatusBadge/index.jsx (1)
getStatusClass(10-20)
src/components/features/LogsPreview/KeyLogModal.jsx (3)
src/components/common/Modal/index.jsx (1)
Modal(7-135)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/common/Card/index.jsx (1)
Card(3-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (17)
src/components/common/Modal/index.jsx (6)
2-3: LGTM!The new imports for
CardandIconButtonenable the refactored modal structure with better component composition.
8-24: LGTM!The comprehensive props with sensible defaults provide flexibility while maintaining ease of use. The addition of
closeOnOverlayClick,showCloseButton, andzIndexcovers common modal requirements.
27-37: LGTM!The body scroll lock effect correctly prevents background scrolling when the modal is open and properly cleans up on unmount.
39-48: LGTM!The Escape key handler uses optional chaining for
onClose?.(), safely handling cases whereonClosemight not be provided.
52-56: LGTM!The overlay click handler correctly checks
closeOnOverlayClickand verifiese.target === e.currentTargetto ensure clicks on the overlay itself (not on child elements) trigger the close action.
97-131: LGTM!The conditional rendering based on modal type (
confirm,alert, orcustom) provides appropriate layouts for different use cases. The confirm/alert types render predefined button layouts with localized labels, while custom types allow full flexibility.src/components/features/LogsPreview/KeyLogModal.jsx (4)
5-7: LGTM!The new imports for
Modal,Card, andEmptyStateenable the refactored UI with better component composition and consistent styling.
192-229: LGTM!The refactored layout uses
Cardcomponents consistently for status, context, and metadata sections. The grid layout provides a clean two-column structure on medium+ screens. Loading and empty states are handled with theEmptyStatecomponent for consistency.
232-275: LGTM!Each event is wrapped in a
Cardcomponent with consistent padding. Thedetails/summaryelements provide collapsible request/response data, improving readability for large payloads. The empty state for no events usesEmptyStatefor consistency.
115-115: Verify modal visibility condition
TheisOpenflag now requires bothstate.isLogModalOpenandstate.showDetailedLogs, so turning off detailed logs will prevent the modal from opening even when it’s requested. Confirm this stricter gating is intentional.src/styles/globals.css (1)
295-311: LGTM! New horizontal layout is well-structured.The new
.key-item-horizontalclass introduces a modern card-based layout with proper theming support. The explicittransition: noneat line 310 correctly implements the PR objective to remove hover effects and transitions.src/components/features/ConcurrencyControl/index.jsx (2)
13-23: LGTM! Input validation is correct.The
handleInputChangeandhandleSliderChangefunctions properly validate and constrain the concurrency value. The slider is capped at 50 (line 39) while the numeric input allows up to 100 (line 16), which aligns with the different UX needs of each control type.
27-59: Fix duplicate ID attribute.Lines 35 and 46 both assign
id="concurrency-count"to different controls (the slider and the number input). This violates the HTML specification requirement that IDs must be unique within a document and breaks accessibility features like label associations and screen reader navigation.Apply this diff to fix the duplicate ID:
<input id="concurrency-count" type="range" className={styles.concurrencySlider} min="1" max="50" value={Math.min(state.concurrency, 50)} onChange={handleSliderChange} disabled={state.isTesting} aria-label={t('concurrencyControl') || 'Concurrency slider'} /> <input - id="concurrency-count-number" + id="concurrency-number-input" type="number" className={styles.concurrencySliderValue} value={state.concurrency} min="1" max="100" onChange={handleInputChange} disabled={state.isTesting} aria-label={t('concurrencyControl') + ' ' + (t('value') || 'value')} />Also verify the FormField's
idprop (line 28) should match the primary control (the slider):<FormField - id="concurrency-count" + id="concurrency-slider" label={t('concurrencyControl')} >Likely an incorrect or invalid review comment.
src/components/common/FormField/index.jsx (1)
50-86: LGTM! Input component is well-structured.The
Inputsubcomponent correctly usesforwardRefto expose refs, properly handles textarea vs input rendering, and computes CSS classes cleanly. ThedisplayNameis set for debugging, and the pattern of attaching it to FormField as a subcomponent (FormField.Input) is a good practice.src/components/features/ConcurrencyControl/ConcurrencyControl.module.css (1)
1-12: LGTM! Container now uses CSS variables for theming.Lines 9-10 correctly replace hard-coded color values with CSS variables (
var(--bg-card)andvar(--border-color)), enabling proper light/dark theme support. This aligns with the broader theming refactor across the codebase.src/components/layout/Sidebar/Sidebar.module.css (2)
156-162: Mobile collapse behavior preservedKeeping the transform/transition pairing inside the mobile breakpoint keeps the drawer sliding fully off‑canvas when collapsed, which matches the existing UX while the reduced-motion override below handles the accessibility side. Looks good.
200-204: Reduced-motion fallback restoredThanks for reinstating the
prefers-reduced-motionguard—this cleanly neuters the sidebar transition for people who opt out of animations.
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 (12)
src/__tests__/hooks/useVirtualization.test.js(2 hunks)src/components/features/ApiConfig/ModelSelector.jsx(1 hunks)src/components/features/ApiConfig/ProxySettings.jsx(0 hunks)src/components/features/KeyInput/PasteButton.jsx(2 hunks)src/components/features/LogsPreview/KeyLogModal.jsx(3 hunks)src/components/features/Results/VirtualizedList.jsx(6 hunks)src/components/layout/Sidebar/Sidebar.module.css(4 hunks)src/hooks/useVirtualization.js(1 hunks)src/locales/en.json(1 hunks)src/locales/zh.json(1 hunks)src/styles/components.css(4 hunks)src/styles/globals.css(6 hunks)
💤 Files with no reviewable changes (1)
- src/components/features/ApiConfig/ProxySettings.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/layout/Sidebar/Sidebar.module.css
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/features/ApiConfig/ModelSelector.jsx (2)
src/contexts/LanguageContext.jsx (1)
t(81-121)src/contexts/AppStateContext.jsx (1)
state(370-370)
src/components/features/LogsPreview/KeyLogModal.jsx (3)
src/components/common/Modal/index.jsx (1)
Modal(7-135)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/common/Card/index.jsx (1)
Card(3-70)
src/components/features/KeyInput/PasteButton.jsx (2)
src/contexts/LanguageContext.jsx (1)
t(81-121)src/contexts/AppStateContext.jsx (1)
state(370-370)
src/__tests__/hooks/useVirtualization.test.js (2)
src/components/features/Results/VirtualizedList.jsx (1)
useVirtualization(250-250)src/hooks/useVirtualization.js (2)
useVirtualization(3-19)useVirtualization(3-19)
src/components/features/Results/VirtualizedList.jsx (5)
src/components/features/BalanceDisplay/KeyBalanceDisplay.jsx (3)
useLanguage(7-7)useAppState(8-8)KeyBalanceDisplay(6-77)src/contexts/AppStateContext.jsx (5)
useAppState(524-530)useAppState(524-530)key(25-25)key(50-50)state(370-370)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/StatusBadge/index.jsx (1)
getStatusClass(10-20)src/hooks/useVirtualization.js (1)
getItemHeight(4-13)
🔇 Additional comments (14)
src/components/features/ApiConfig/ModelSelector.jsx (1)
123-139: LGTM! Visual layout improvement.The restructured flex layout groups the label and paid-detection status inline, improving visual cohesion. The change is purely presentational and does not alter functionality or accessibility.
src/hooks/useVirtualization.js (1)
4-13: LGTM! Simplified API addresses past review.The refactor removes unused parameters (
keyData,apiType) and returns viewport-based heights only. This aligns with the resolution of the past review comment and simplifies the hook's API surface.Verified: Call sites in VirtualizedList.jsx (line 276) have been updated to call
getItemHeight()without parameters.src/__tests__/hooks/useVirtualization.test.js (1)
9-145: LGTM! Comprehensive test coverage.The updated tests properly mock
window.matchMediafor different viewport scenarios and include cleanup inafterEach. The tests verify:
- Desktop (74px), mobile (66px), and extra-small (62px) heights
- Consistent behavior across multiple calls
- Graceful handling when
window.matchMediais undefined- Parameters are correctly ignored
The test coverage aligns well with the simplified hook API.
src/locales/en.json (1)
206-209: LGTM! Locale keys added for detailed logs feature.The new localization keys
showDetailedLogsandshowDetailedLogsDescriptionsupport the detailed logs visibility feature introduced in this PR. The JSON structure is valid.src/locales/zh.json (1)
206-209: LGTM! Chinese translations for detailed logs feature.The Chinese translations mirror the English locale additions, maintaining consistency across both supported languages.
src/components/features/Results/VirtualizedList.jsx (4)
107-117: LGTM! Key masking logic is sound.The
formatKeyfunction correctly masks keys by showing the first 4 and last 4 characters with 10 asterisks in between. The edge case check (key.length <= 8) prevents masking very short keys, which is appropriate.
119-156: LGTM! Clipboard fallback properly implemented.The clipboard copy implementation correctly:
- Prioritizes the modern Clipboard API
- Falls back to
execCommand('copy')for unsupported browsers- Handles errors gracefully with try/catch
- Cleans up the temporary textarea in finally block
This addresses the past review comment about clipboard access guarding.
166-234: LGTM! Improved horizontal layout with proper accessibility.The restructured KeyItem layout provides:
- Clear status indicator on the left
- Semi Design-inspired input wrapper for the key display
- Action buttons with proper ARIA labels and titles
- Clickable info row for opening logs (addressing past nested interactive controls concern)
- SVG icons for visibility toggle and copy actions
The accessibility attributes (
aria-label,title) ensure screen reader compatibility.
275-277: LGTM! Call site updated for simplified hook API.The
getItemSizefunction correctly callsgetItemHeight()without parameters, aligning with the simplified virtualization hook API.src/components/features/LogsPreview/KeyLogModal.jsx (2)
180-191: LGTM! Modal component properly integrated.The migration to the dedicated
Modalcomponent is well-structured:
isOpenstate includesstate.showDetailedLogscheck, gating visibility appropriately- Title uses a React fragment to avoid invalid HTML (addresses past review comment at e0211d8)
- Props correctly passed:
onClose,maxWidth,className
192-280: LGTM! Card and EmptyState components improve structure.The modal body refactor enhances maintainability:
- Loading state uses
EmptyStatewith translated message- Two-column grid layout with
Cardcomponents for summary and context- Event list wrapped in
Cardboundaries with proper conditional rendering- Consistent use of
EmptyStatefor no-data scenariosThe refactor aligns with the broader Card/EmptyState component introduction across the PR.
src/styles/components.css (3)
222-234: LGTM! Adjacent sibling combinator is more precise.The change from
~(general sibling) to+(adjacent sibling) combinator is correct for switch styling. Since.switch-slideris the immediate sibling of.switch-input,+is semantically more precise.
351-529: LGTM! Comprehensive card variant system with dark theme support.The extensive card variant additions provide:
- Unified naming convention (
card--primary,card--info,card--results,card--empty-state)- Consistent button variant (
btn--settings)- Dark theme adaptations for all variants
- Responsive adjustments for mobile viewports
- Compatibility aliases for gradual migration (
function-card,usage-card, etc.)The CSS architecture supports the component refactoring seen throughout this PR, with proper fallbacks and progressive enhancement.
119-124: Verify.form-field.textareaapplies to all textareas
Ensure every<textarea class="form-field">also has thetextareaclass; otherwise this rule won’t match.
| const { t } = useLanguage(); | ||
| const { state, dispatch } = useAppState(); | ||
| const [isPasting, setIsPasting] = useState(false); | ||
| const [isHovered, setIsHovered] = useState(false); |
There was a problem hiding this comment.
Inconsistency with PR objectives: hover effects and transitions added, not removed.
The PR title states "移除悬停效果和过渡" (remove hover effects and transitions), but this code introduces:
- Hover state tracking (
isHovered) - Mouse enter/leave handlers
- Scale transform on hover
- Transition effects for transform and opacity
Apply this diff to align with the PR objectives:
- const [isHovered, setIsHovered] = useState(false);
const handlePaste = async () => { <svg
width="16"
height="16"
viewBox="0 0 24 24"
fill="currentColor"
onClick={handlePaste}
- onMouseEnter={() => setIsHovered(true)}
- onMouseLeave={() => setIsHovered(false)}
title={t('paste')}
style={{
position: 'absolute',
top: '8px',
right: '16px',
zIndex: 10,
cursor: state.isTesting ? 'not-allowed' : 'pointer',
- opacity: state.isTesting ? 0.5 : 1,
- transform: isHovered && !state.isTesting ? 'scale(1.1)' : 'scale(1)',
- transition: 'transform 0.2s ease, opacity 0.2s ease'
+ opacity: state.isTesting ? 0.5 : 1
}}
>Also applies to: 46-47, 56-57
🤖 Prompt for AI Agents
In src/components/features/KeyInput/PasteButton.jsx around line 9 (and also
affecting lines 46-47 and 56-57), the PR is supposed to remove hover effects and
transitions but instead adds hover state and related handlers; remove the hover
implementation by deleting the useState hook for isHovered, removing any
onMouseEnter/onMouseLeave handlers, and eliminating any conditional usage of
isHovered (including scale transform logic); also remove CSS
transitions/transform/opacity changes tied to hover (classNames or inline
styles) so the button has no hover scaling/transition behavior.
| <svg | ||
| width="16" | ||
| height="16" | ||
| viewBox="0 0 24 24" | ||
| fill="currentColor" | ||
| onClick={handlePaste} | ||
| onMouseEnter={() => setIsHovered(true)} | ||
| onMouseLeave={() => setIsHovered(false)} | ||
| title={t('paste')} | ||
| style={{ | ||
| position: 'absolute', | ||
| top: '8px', | ||
| right: '8px', | ||
| zIndex: 10 | ||
| right: '16px', | ||
| zIndex: 10, | ||
| cursor: state.isTesting ? 'not-allowed' : 'pointer', | ||
| opacity: state.isTesting ? 0.5 : 1, | ||
| transform: isHovered && !state.isTesting ? 'scale(1.1)' : 'scale(1)', | ||
| transition: 'transform 0.2s ease, opacity 0.2s ease' | ||
| }} | ||
| onClick={handlePaste} | ||
| disabled={state.isTesting} | ||
| title={t('paste')} | ||
| > | ||
| <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> | ||
| <rect x="8" y="2" width="8" height="4" rx="1" ry="1" /> | ||
| <path d="m16 4h2a2 2 0 0 1 2 2v14a2 2 0 0 1-2 2H6a2 2 0 0 1-2-2V6a2 2 0 0 1 2-2h2" /> | ||
| </svg> | ||
| </button> | ||
| <path d="M16 1H4c-1.1 0-2 .9-2 2v14h2V3h12V1zm3 4H8c-1.1 0-2 .9-2 2v14c0 1.1.9 2 2 2h11c1.1 0 2-.9 2-2V7c0-1.1-.9-2-2-2zm0 16H8V7h11v14z"></path> | ||
| </svg> |
There was a problem hiding this comment.
Add keyboard accessibility and ARIA attributes.
The SVG lacks keyboard navigation and proper accessibility attributes. Interactive SVGs must be keyboard-accessible and announce their purpose to screen readers.
Apply this diff to add accessibility support:
+ const handleKeyDown = (e) => {
+ if ((e.key === 'Enter' || e.key === ' ') && !state.isTesting && !isPasting) {
+ e.preventDefault();
+ handlePaste();
+ }
+ };
+
return (
<svg
width="16"
height="16"
viewBox="0 0 24 24"
fill="currentColor"
+ role="button"
+ tabIndex={state.isTesting ? -1 : 0}
+ aria-label={t('paste')}
onClick={handlePaste}
- onMouseEnter={() => setIsHovered(true)}
- onMouseLeave={() => setIsHovered(false)}
+ onKeyDown={handleKeyDown}
title={t('paste')}
style={{Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/features/KeyInput/PasteButton.jsx around lines 40 to 61, the
SVG is not keyboard-accessible or announced to screen readers; make it an
accessible interactive element by adding role="button",
tabIndex={state.isTesting ? -1 : 0}, and aria-disabled={state.isTesting} (or
aria-label={t('paste')} if not using title), implement an onKeyDown handler that
invokes handlePaste when Enter or Space is pressed (and ignores input when
state.isTesting), and keep existing visual disabled cues (cursor/opacity) so
keyboard users get the same affordance; ensure title or aria-label is present
and remove reliance on title alone for screen readers.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/features/Results/VirtualizedList.jsx (2)
304-315: Prevent row-state bleed with a stable itemKey (possible secret exposure).With VariableSizeList keyed by index, reordering/filtered updates can reuse row instances and carry isKeyVisible to the wrong item. A key’s visibility may appear unmasked for another key after list changes.
Add a stable itemKey based on the key value.
<List ref={listRef} height={listHeight} itemCount={filteredKeys.length} itemSize={getItemSize} itemData={filteredKeys} + itemKey={(index, data) => data[index]?.key ?? index} overscanCount={3} width="100%" className="virtualized-list" >
100-105: Avoid double activation on a real button; add type="button".
- handleKeyDown on a native can double-trigger with onClick (Enter/Space). Remove it and rely on button semantics.
- Add type="button" to all action buttons to prevent unintended form submits.
- const handleKeyDown = (event) => { - if (event.key === 'Enter' || event.key === ' ') { - event.preventDefault(); - handleOpenLogs(); - } - };<button + type="button" className="semi-button semi-button-borderless semi-button-small semi-button-tertiary" onClick={handleToggleKeyVisibility}<button + type="button" className="semi-button semi-button-borderless semi-button-small semi-button-tertiary" onClick={handleCopyKey}- <button + <button + type="button" className="key-info-row key-info-clickable" onClick={handleOpenLogs} - onKeyDown={handleKeyDown} aria-label={t('openLogs') || 'Open logs for this key'}Also applies to: 183-200, 201-211, 216-221
♻️ Duplicate comments (3)
src/styles/globals.css (1)
625-656: Restore a visible scrollbar (duplicate of earlier feedback).The capsule still hides scrollbars entirely (
display: none/scrollbar-width: none). That leaves keyboard and pointer users without any affordance for horizontal overflow, even with the new gradient. Please reinstate a thin, low-contrast scrollbar (or an equivalent accessible indicator) so the scroll state remains discoverable.src/components/features/Results/VirtualizedList.jsx (2)
119-139: Clipboard fallback added — previous issue resolved.Defensive Clipboard API usage with textarea fallback is implemented. Looks good.
Also applies to: 141-156
215-234: A11y: nested-interactive issue fixed.Switching the middle region to a proper resolves the prior role="button" nesting problem. Good fix.
🧹 Nitpick comments (7)
src/components/features/ApiConfig/ModelSelector.jsx (1)
134-137: Consider using the StatusBadge component for semantic consistency.Since the PR introduces a new StatusBadge component, consider using it here instead of a
<small>tag for better semantic markup and consistency across the application.Example refactor:
+import StatusBadge from '../../common/StatusBadge'; + // ... rest of imports - <small className="form-info enabled text-xs"> - {t('paidDetectionEnabled')} - </small> + <StatusBadge status="enabled" size="sm"> + {t('paidDetectionEnabled')} + </StatusBadge>Note: Adjust the StatusBadge props based on the actual component API.
src/styles/components.css (4)
119-124: Scope monospace more safely for textareasUse a tokenized monospace stack to avoid platform-specific “Courier New” quirks.
-.form-field.textarea, -textarea.form-field { +.form-field.textarea, +textarea.form-field { resize: vertical; min-height: 120px; - font-family: 'Courier New', monospace; + font-family: var(--font-mono, ui-monospace, SFMono-Regular, Menlo, Consolas, "Liberation Mono", monospace); }
366-371: Prefer variables over hardcoded info colorsUse theme tokens with fallbacks to keep light/dark/parity consistent.
-.card--info { - background: #e3f2fd; - border-color: #bbdefb; - color: #1976d2; +.card--info { + background: var(--info-bg, #e3f2fd); + border-color: var(--info-border, #bbdefb); + color: var(--info-fg, #1976d2); padding: var(--spacing-sm) var(--spacing-md); }
409-428: .btn--settings should reuse base button and add a visible focus stateConsider composing with .btn-base in markup to avoid duplication, and add a focus-visible ring for accessibility.
.btn--settings { display: flex; align-items: center; justify-content: center; gap: 8px; width: 100%; min-height: 48px; padding: var(--spacing-md) var(--spacing-xl); background: var(--color-secondary); color: white; border: none; border-radius: var(--border-radius); font-size: var(--font-size-base); font-weight: 600; cursor: pointer; text-decoration: none; box-sizing: border-box; margin-bottom: 0; box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1); } + +.btn--settings:focus-visible { + outline: 2px solid var(--focus-ring, var(--primary-color)); + outline-offset: 2px; +}Also verify that --color-secondary is defined in the theme.
441-444: Dark theme parity for results aliasIf keeping aliases, ensure dark theme rules also target .results-card to avoid mismatched borders/backgrounds.
-.dark-theme .card--results { +.dark-theme .card--results, +.dark-theme .results-card { background: #2a2a2a; border-color: #444444; }src/components/features/Results/VirtualizedList.jsx (2)
19-29: De-duplicate status class mapping to avoid drift.This mapping duplicates StatusBadge logic and has a different default ('status-unknown' vs StatusBadge’s 'status-testing'). Prefer centralizing via StatusBadge or a shared util.
43-53: Remove unused helper.getLocalizedError isn’t used; drop it to reduce bundle size.
- const getLocalizedError = (error) => { - if (!error) return ''; - - // 如果错误是翻译键，直接使用翻译 - if (error.startsWith('errorMessages.')) { - return t(error) || error; - } - - // 直接翻译显示 - return error; - };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
new-api(1 hunks)src/__tests__/hooks/useVirtualization.test.js(2 hunks)src/components/features/ApiConfig/ModelSelector.jsx(1 hunks)src/components/features/ApiConfig/ProxySettings.jsx(0 hunks)src/components/features/KeyInput/PasteButton.jsx(2 hunks)src/components/features/LogsPreview/KeyLogModal.jsx(3 hunks)src/components/features/Results/VirtualizedList.jsx(6 hunks)src/components/layout/Sidebar/Sidebar.module.css(4 hunks)src/hooks/useVirtualization.js(1 hunks)src/locales/en.json(1 hunks)src/locales/zh.json(1 hunks)src/styles/components.css(4 hunks)src/styles/globals.css(6 hunks)
💤 Files with no reviewable changes (1)
- src/components/features/ApiConfig/ProxySettings.jsx
✅ Files skipped from review due to trivial changes (1)
- new-api
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/features/KeyInput/PasteButton.jsx
- src/hooks/useVirtualization.js
🧰 Additional context used
🧬 Code graph analysis (4)
src/__tests__/hooks/useVirtualization.test.js (2)
src/components/features/Results/VirtualizedList.jsx (1)
useVirtualization(250-250)src/hooks/useVirtualization.js (2)
useVirtualization(3-19)useVirtualization(3-19)
src/components/features/ApiConfig/ModelSelector.jsx (1)
src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/features/LogsPreview/KeyLogModal.jsx (3)
src/components/common/Modal/index.jsx (1)
Modal(7-135)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/common/Card/index.jsx (1)
Card(3-70)
src/components/features/Results/VirtualizedList.jsx (5)
src/components/features/BalanceDisplay/KeyBalanceDisplay.jsx (3)
useLanguage(7-7)useAppState(8-8)KeyBalanceDisplay(6-77)src/contexts/AppStateContext.jsx (5)
useAppState(524-530)useAppState(524-530)key(25-25)key(50-50)state(370-370)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/StatusBadge/index.jsx (1)
getStatusClass(10-20)src/hooks/useVirtualization.js (1)
getItemHeight(4-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/components/layout/Sidebar/Sidebar.module.css (1)
200-205: Reduced-motion fallback reinstated.Thanks for restoring the reduced-motion override; nice job closing the accessibility gap.
src/components/features/ApiConfig/ModelSelector.jsx (1)
123-131: LGTM! Clean layout refactor.The flex container provides a clear horizontal layout for the label and detected models badge. The conditional rendering and styling are appropriate.
src/styles/components.css (1)
223-225: Unify CSS variable naming for primary color tokensBoth --color-primary and --primary-color are used. Pick one scheme to avoid theme bugs and fallback inconsistencies.
Please confirm which is canonical (e.g., --color-primary and --color-primary-alpha) and standardize across this file.
Also applies to: 339-341
| </label> | ||
|
|
||
| {/* 付费检测状态信息 */} | ||
| {state.apiType === 'gemini' && state.enablePaidDetection && ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated paid detection condition.
The condition state.apiType === 'gemini' && state.enablePaidDetection (or its reversed form) appears 7 times throughout this file (lines 134, 143, 145, 148, 159, 161, 169). Extract it to a memoized computed value to improve maintainability and reduce duplication.
Apply this refactor near the top of the component:
const ModelSelector = () => {
const { t } = useLanguage();
const { state, dispatch } = useAppState();
const { detectModels, isDetecting } = useApiTester();
const [showPaidDetectionPrompt, setShowPaidDetectionPrompt] = useState(false);
const [isCustomModel, setIsCustomModel] = useState(false);
+
+ // Memoize paid detection condition to avoid duplication
+ const isGeminiPaidDetection = useMemo(
+ () => state.apiType === 'gemini' && state.enablePaidDetection,
+ [state.apiType, state.enablePaidDetection]
+ );
const currentModels = useMemo(() => MODEL_OPTIONS[state.apiType] || [], [state.apiType]);Then replace all 7 occurrences with isGeminiPaidDetection.
📝 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.
| {state.apiType === 'gemini' && state.enablePaidDetection && ( | |
| // File: src/components/features/ApiConfig/ModelSelector.jsx | |
| const ModelSelector = () => { | |
| const { t } = useLanguage(); | |
| const { state, dispatch } = useAppState(); | |
| const { detectModels, isDetecting } = useApiTester(); | |
| const [showPaidDetectionPrompt, setShowPaidDetectionPrompt] = useState(false); | |
| const [isCustomModel, setIsCustomModel] = useState(false); | |
| // Memoize paid detection condition to avoid duplication | |
| const isGeminiPaidDetection = useMemo( | |
| () => state.apiType === 'gemini' && state.enablePaidDetection, | |
| [state.apiType, state.enablePaidDetection] | |
| ); | |
| const currentModels = useMemo( | |
| () => MODEL_OPTIONS[state.apiType] || [], | |
| [state.apiType] | |
| ); | |
| // …rest of your hooks and handlers… | |
| return ( | |
| <> | |
| {/* …other JSX… */} | |
| {/* Example replacement at line 134 */} | |
| {isGeminiPaidDetection && ( | |
| /* …paid-detection UI… */ | |
| )} | |
| {/* Make the same replacement for the other 6 occurrences */} | |
| </> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In src/components/features/ApiConfig/ModelSelector.jsx around lines 134–169, the
condition state.apiType === 'gemini' && state.enablePaidDetection is duplicated
seven times; create a single memoized boolean near the top of the component
(e.g. const isGeminiPaidDetection = useMemo(() => state.apiType === 'gemini' &&
state.enablePaidDetection, [state.apiType, state.enablePaidDetection]) — add
useMemo to the imports if missing — then replace all seven occurrences with
isGeminiPaidDetection to remove duplication and improve maintainability.
| // 格式化密钥显示 - 参考 new-api 的遮罩规则 | ||
| const formatKey = (key, isVisible) => { | ||
| if (isVisible) { | ||
| // 显示完整密钥 | ||
| return key; | ||
| } else { | ||
| // 显示遮罩版本：前4位 + 10个星号 + 后4位 | ||
| if (key.length <= 8) return key; | ||
| return key.substring(0, 4) + '**********' + key.substring(key.length - 4); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Mask short keys too (don’t reveal ≤8‑char keys when hidden).
Current logic returns the full key when length ≤ 8, leaking secrets. Always mask when not visible.
- const formatKey = (key, isVisible) => {
- if (isVisible) {
- // 显示完整密钥
- return key;
- } else {
- // 显示遮罩版本：前4位 + 10个星号 + 后4位
- if (key.length <= 8) return key;
- return key.substring(0, 4) + '**********' + key.substring(key.length - 4);
- }
- };
+ const formatKey = (key, isVisible) => {
+ if (!key) return '';
+ if (isVisible) return key;
+ const len = key.length;
+ if (len <= 8) {
+ // 短密钥也需遮罩，至少保留前后各2位
+ return `${key.slice(0, 2)}****${key.slice(-2)}`;
+ }
+ return `${key.slice(0, 4)}**********${key.slice(-4)}`;
+ };📝 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.
| // 格式化密钥显示 - 参考 new-api 的遮罩规则 | |
| const formatKey = (key, isVisible) => { | |
| if (isVisible) { | |
| // 显示完整密钥 | |
| return key; | |
| } else { | |
| // 显示遮罩版本：前4位 + 10个星号 + 后4位 | |
| if (key.length <= 8) return key; | |
| return key.substring(0, 4) + '**********' + key.substring(key.length - 4); | |
| } | |
| }; | |
| // 格式化密钥显示 - 参考 new-api 的遮罩规则 | |
| const formatKey = (key, isVisible) => { | |
| if (!key) return ''; | |
| if (isVisible) return key; | |
| const len = key.length; | |
| if (len <= 8) { | |
| // 短密钥也需遮罩，至少保留前后各2位 | |
| return `${key.slice(0, 2)}****${key.slice(-2)}`; | |
| } | |
| return `${key.slice(0, 4)}**********${key.slice(-4)}`; | |
| }; |
🤖 Prompt for AI Agents
In src/components/features/Results/VirtualizedList.jsx around lines 107 to 117,
the formatKey function currently returns the full key when key.length <= 8 which
leaks secrets; change the logic so that when isVisible is false you never return
the original key — for keys longer than 8 keep the existing pattern (first 4 +
10 stars + last 4), and for keys of length 8 or less return a masked string
(e.g., repeat '*' for the key length) so short keys are fully masked.
| }, | ||
| "showDetailedLogs": "Show Detailed Logs", | ||
| "showDetailedLogsDescription": "Control whether to show detailed API call logs" | ||
| } |
There was a problem hiding this comment.
Add missing strings used by UI (prevents raw key labels).
VirtualizedList uses t('copyKey' | 'showKey' | 'hideKey' | 'openLogs'); without entries, labels show literal keys.
},
"showDetailedLogs": "Show Detailed Logs",
- "showDetailedLogsDescription": "Control whether to show detailed API call logs"
+ "showDetailedLogsDescription": "Control whether to show detailed API call logs",
+ "copyKey": "Copy key",
+ "showKey": "Show key",
+ "hideKey": "Hide key",
+ "openLogs": "Open logs for this key"
}📝 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.
| }, | |
| "showDetailedLogs": "Show Detailed Logs", | |
| "showDetailedLogsDescription": "Control whether to show detailed API call logs" | |
| } | |
| }, | |
| "showDetailedLogs": "Show Detailed Logs", | |
| "showDetailedLogsDescription": "Control whether to show detailed API call logs", | |
| "copyKey": "Copy key", | |
| "showKey": "Show key", | |
| "hideKey": "Hide key", | |
| "openLogs": "Open logs for this key" | |
| } |
🤖 Prompt for AI Agents
In src/locales/en.json around lines 206 to 209, the UI is missing translation
keys used by VirtualizedList (copyKey, showKey, hideKey, openLogs), causing raw
key labels to appear; add these string entries to the JSON with appropriate
English text (e.g., "copyKey": "Copy", "showKey": "Show key", "hideKey": "Hide
key", "openLogs": "Open logs") ensuring proper JSON comma placement and
consistent style with existing keys.
| }, | ||
| "showDetailedLogs": "显示详细日志", | ||
| "showDetailedLogsDescription": "控制是否显示API调用的详细日志信息" | ||
| } |
There was a problem hiding this comment.
补齐缺失的文案键，避免显示原始键名。
VirtualizedList 依赖 t('copyKey' | 'showKey' | 'hideKey' | 'openLogs')，目前未定义。
},
"showDetailedLogs": "显示详细日志",
- "showDetailedLogsDescription": "控制是否显示API调用的详细日志信息"
+ "showDetailedLogsDescription": "控制是否显示API调用的详细日志信息",
+ "copyKey": "复制密钥",
+ "showKey": "显示密钥",
+ "hideKey": "隐藏密钥",
+ "openLogs": "打开该密钥的日志"
}📝 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.
| }, | |
| "showDetailedLogs": "显示详细日志", | |
| "showDetailedLogsDescription": "控制是否显示API调用的详细日志信息" | |
| } | |
| }, | |
| "showDetailedLogs": "显示详细日志", | |
| "showDetailedLogsDescription": "控制是否显示API调用的详细日志信息", | |
| "copyKey": "复制密钥", | |
| "showKey": "显示密钥", | |
| "hideKey": "隐藏密钥", | |
| "openLogs": "打开该密钥的日志" | |
| } |
🤖 Prompt for AI Agents
In src/locales/zh.json around lines 206 to 209, the VirtualizedList expects
translation keys copyKey, showKey, hideKey and openLogs which are missing; add
these keys with appropriate Chinese translations (e.g., "复制键", "显示键", "隐藏键",
"打开日志") into the JSON near the existing entries so t(...) resolves to localized
strings instead of falling back to raw keys.
| .switch-input:checked+.switch-slider:before { | ||
| left: 22px; | ||
| background: white; | ||
| } |
There was a problem hiding this comment.
Switch knob final position likely off by 2px
For a 44px track and 18px knob, the right position should be left: 24px (symmetrical 2px gutters).
-.switch-input:checked+.switch-slider:before {
- left: 22px;
+.switch-input:checked+.switch-slider:before {
+ left: 24px;
background: white;
}🤖 Prompt for AI Agents
In src/styles/components.css around lines 231 to 234, the .switch-input:checked
+ .switch-slider:before rule moves the 18px knob to left: 22px which is 2px
short for a 44px track; change the value to left: 24px so the knob is centered
with symmetrical 2px gutters (update the CSS rule accordingly).
| .card--results { | ||
| display: flex; | ||
| flex-direction: column; | ||
| flex: 1; | ||
| margin-bottom: 0; | ||
| background: #f8f9fa; | ||
| border: 2px solid #e9ecef; | ||
| border-radius: 12px; | ||
| padding: 16px; | ||
| max-height: 340px; | ||
| min-height: 340px; | ||
| } |
There was a problem hiding this comment.
Alias divergence: .results-card and .card--results are not equivalent (heights differ, responsive not synced)
This will cause inconsistent layouts depending on which alias is used. Unify definitions and responsive rules.
-/* 结果展示卡片 */
-.card--results {
+/* 结果展示卡片（主类与别名保持一致） */
+.card--results,
+.results-card {
display: flex;
flex-direction: column;
flex: 1;
margin-bottom: 0;
background: #f8f9fa;
border: 2px solid #e9ecef;
border-radius: 12px;
padding: 16px;
- max-height: 340px;
- min-height: 340px;
+ max-height: 340px;
+ min-height: 340px;
}
@@
- .card--results {
+ .card--results,
+ .results-card {
max-height: 400px;
min-height: 400px;
padding: 12px;
}
@@
- .card--results {
+ .card--results,
+ .results-card {
max-height: 350px;
min-height: 350px;
padding: 10px;
}
@@
-.results-card {
- display: flex;
- flex-direction: column;
- flex: 1;
- margin-bottom: 0;
- background: #f8f9fa;
- border: 2px solid #e9ecef;
- border-radius: 12px;
- padding: 16px;
- max-height: 392px;
- min-height: 392px;
-}
+/* 删除独立定义，已与 .card--results 合并以保持一致 */Also applies to: 477-481, 485-489, 509-520
| display: flex; | ||
| align-items: center; | ||
| padding: 16px 20px; | ||
| background: var(--bg-input); | ||
| /* 使用主题变量 */ | ||
| border: 2px solid var(--border-color); | ||
| border-radius: 12px; | ||
| gap: 16px; | ||
| box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1); | ||
| margin-bottom: 1px; | ||
| height: 72px; | ||
| width: 100%; | ||
| box-sizing: border-box; | ||
| cursor: pointer; | ||
| transition: none; | ||
| /* 移除过渡效果 */ | ||
| } |
There was a problem hiding this comment.
Avoid hard-coding the card height.
height: 72px; (and the mobile overrides) will clip or overflow longer status texts, error messages, or multi-line metadata inside .key-display-container. Use min-height (or remove the explicit height) so the card can expand with its content.
🤖 Prompt for AI Agents
In src/styles/globals.css around lines 297 to 313, the rule for
.key-display-container sets a fixed height (height: 72px) which can clip
multi-line content; replace the fixed height with a flexible constraint such as
min-height: 72px (or remove the height declaration entirely) and ensure any
mobile overrides also use min-height or no explicit height so the card can grow
with its content; keep existing padding, box-sizing and alignment to preserve
visual layout while allowing expansion.
weiruchenai1
force-pushed
the
experimental
branch
from
47002f9 to
0c68747
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/features/RetryControl/RetryControl.module.css (1)
139-199: Active preset contrast regressedThe active preset state now renders white text on
#e8eaed, giving a contrast ratio of ~1.2:1. That fails WCAG 2.1 AA (4.5:1) and makes the label nearly unreadable in both light (Lines 139-143) and dark (Lines 195-199) themes. Please restore a high-contrast pairing—e.g., keep the light background but switch the text to a dark tone such as#1f2933(or your text token), or revert to the accent background that supported white text.public/worker.js (1)
32-37: Redact secrets in logs before posting to main threadRequest logs currently include Authorization/x‑api keys and may expose query keys (e.g., Gemini). Mask these before self.postMessage.
-const createRequestLog = ({ url, method, headers, body }) => ({ - url, - method, - headers: headersToObject(headers), - body: truncateForLog(body) -}); +const createRequestLog = ({ url, method, headers, body }) => { + const h = headersToObject(headers) || {}; + if (h.Authorization) h.Authorization = 'Bearer ****'; + if (h['x-api-key']) h['x-api-key'] = '****'; + if (h['x-goog-api-key']) h['x-goog-api-key'] = '****'; + // Redact key in URL query + const safeUrl = typeof url === 'string' ? url.replace(/([?&](?:key|api_key|apiKey))=[^&#]*/gi, '$1=****') : url; + return { + url: safeUrl, + method, + headers: h, + body: truncateForLog(body) + }; +};
♻️ Duplicate comments (7)
src/components/features/KeyInput/PasteButton.jsx (2)
9-9: Duplicate concern: hover state contradicts PR objectives.This concern was already raised in a previous review. The PR title indicates removing hover effects and transitions, but this code introduces them.
40-62: Duplicate concern: keyboard accessibility missing.A previous review already noted that the SVG lacks keyboard accessibility (role, tabIndex, onKeyDown handler). This remains unaddressed.
src/locales/zh.json (1)
206-209: Duplicate concern: VirtualizedList translation keys still missing.A previous review noted that VirtualizedList expects keys like
copyKey,showKey,hideKey, andopenLogswhich are not present in the localization file.src/styles/components.css (2)
231-234: Switch knob final position off by 2px.For a 44px track with an 18px knob, the checked position should be
left: 24pxto maintain symmetrical 2px gutters (44 - 18 - 2 = 24), notleft: 22px.Apply this diff:
.switch-input:checked+.switch-slider:before { - left: 22px; + left: 24px; background: white; }
374-385: Height mismatch between card variants creates layout inconsistency.
.card--resultsat lines 374-385 hasmax-height: 340px; min-height: 340px, while its alias.results-cardat lines 509-520 hasmax-height: 392px; min-height: 392px. This 52px difference will cause inconsistent layouts depending on which class is used.Unify the heights. If 340px is the intended height, apply this diff:
.results-card { display: flex; flex-direction: column; flex: 1; margin-bottom: 0; background: #f8f9fa; border: 2px solid #e9ecef; border-radius: 12px; padding: 16px; - max-height: 392px; - min-height: 392px; + max-height: 340px; + min-height: 340px; }Alternatively, merge the selectors to ensure they always stay in sync:
-/* 结果展示卡片 */ -.card--results { +/* 结果展示卡片（主类与别名保持一致） */ +.card--results, +.results-card { display: flex; flex-direction: column; flex: 1; margin-bottom: 0; background: #f8f9fa; border: 2px solid #e9ecef; border-radius: 12px; padding: 16px; max-height: 340px; min-height: 340px; }Then remove the duplicate
.results-carddefinition at lines 509-520.src/styles/globals.css (1)
771-954: Critical: All status variants use identical colors, losing visual distinction.Lines 771-796 define the same
--status-*custom property values for all six status variants (valid, invalid, rate-limited, paid, testing, retrying). As a result, every card renders with identical neutral colors, eliminating the visual cues that differentiate statuses—a significant UX regression.Restore per-status color palettes. For example:
.key-item-horizontal.status-valid { - --status-bg-light: #ffffff; - --status-bg-dark: #2c2c2c; - --status-border-light: #e0e0e0; - --status-border-dark: #404040; - --status-capsule-light: rgba(108, 117, 125, 0.2); - --status-capsule-dark: rgba(33, 37, 41, 0.9); - --status-text-light: #343a40; - --status-text-dark: white; + --status-bg-light: rgba(40, 167, 69, 0.08); + --status-bg-dark: rgba(40, 167, 69, 0.18); + --status-border-light: rgba(40, 167, 69, 0.45); + --status-border-dark: rgba(56, 189, 105, 0.65); + --status-capsule-light: rgba(40, 167, 69, 0.12); + --status-capsule-dark: rgba(40, 167, 69, 0.35); + --status-text-light: #166534; + --status-text-dark: #d1f7dc; /* ... remaining properties with green palette */ } .key-item-horizontal.status-invalid { - --status-bg-light: #ffffff; + --status-bg-light: rgba(220, 53, 69, 0.08); /* ... remaining properties with red palette */ } .key-item-horizontal.status-rate-limited { - --status-bg-light: #ffffff; + --status-bg-light: rgba(255, 193, 7, 0.08); /* ... remaining properties with yellow palette */ } .key-item-horizontal.status-paid { - --status-bg-light: #ffffff; + --status-bg-light: rgba(13, 110, 253, 0.08); /* ... remaining properties with blue palette */ } .key-item-horizontal.status-testing, .key-item-horizontal.status-retrying { /* Keep neutral gray for in-progress states */ }Apply distinct color palettes to each status variant so users can immediately identify the state of each key card.
src/components/features/Results/VirtualizedList.jsx (1)
107-117: Don’t reveal short keys when hidden (mask ≤8 chars too).Always mask when not visible to avoid secret leakage.
- const formatKey = (key, isVisible) => { - if (isVisible) { - // 显示完整密钥 - return key; - } else { - // 显示遮罩版本：前4位 + 10个星号 + 后4位 - if (key.length <= 8) return key; - return key.substring(0, 4) + '**********' + key.substring(key.length - 4); - } - }; + const formatKey = (key, isVisible) => { + if (!key) return ''; + if (isVisible) return key; + const len = key.length; + if (len <= 8) { + // 短密钥也需遮罩，至少保留前后各2位 + return `${key.slice(0, 2)}****${key.slice(-2)}`; + } + return `${key.slice(0, 4)}**********${key.slice(-4)}`; + };
🧹 Nitpick comments (15)
src/components/features/Controls/index.jsx (2)
92-114: Consider using the Button component's icon prop.The Button component provides a dedicated
iconprop that handles spacing automatically (seesrc/components/common/Button/index.jsxlines 30-34). Currently, SVG icons are embedded inchildren, which bypasses this built-in handling and may conflict with thegap-xsutility class.Apply this diff to leverage the icon prop:
- <Button - variant={state.isTesting ? 'danger' : 'primary'} - size="large" + <Button + variant={state.isTesting ? 'danger' : 'primary'} + size="large" + icon={ + state.isTesting ? ( + <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> + <rect x="6" y="4" width="4" height="16" /> + <rect x="14" y="4" width="4" height="16" /> + </svg> + ) : ( + <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> + <polygon points="5,3 19,12 5,21" /> + </svg> + ) + } onClick={handleStartTest} - className="btn-fixed-lg flex items-center gap-xs" + className="btn-fixed-lg" > - {state.isTesting ? ( - <> - <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> - <rect x="6" y="4" width="4" height="16" /> - <rect x="14" y="4" width="4" height="16" /> - </svg> - {t('cancelTest')} - </> - ) : ( - <> - <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> - <polygon points="5,3 19,12 5,21" /> - </svg> - {t('startTest')} - </> - )} + {state.isTesting ? t('cancelTest') : t('startTest')} </Button>
116-129: Consider using the Button component's icon prop.Similar to the start button, these buttons embed SVG icons in
childreninstead of using the Button component's dedicatediconprop. This bypasses the built-in spacing logic and may conflict with thegap-xsutility class.Apply this pattern to both buttons:
For the Dedupe button:
- <Button - variant="secondary" - size="large" + <Button + variant="secondary" + size="large" + icon={ + <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> + <polyline points="23 4 23 10 17 10" /> + <polyline points="1 20 1 14 7 14" /> + <path d="m3.51 9a9 9 0 0 1 14.85-3.36L23 10M1 14l4.64 4.36A9 9 0 0 0 20.49 15" /> + </svg> + } onClick={handleDedupeKeys} disabled={state.isTesting} - className="btn-fixed-lg flex items-center gap-xs" + className="btn-fixed-lg" > - <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> - <polyline points="23 4 23 10 17 10" /> - <polyline points="1 20 1 14 7 14" /> - <path d="m3.51 9a9 9 0 0 1 14.85-3.36L23 10M1 14l4.64 4.36A9 9 0 0 0 20.49 15" /> - </svg> {t('dedupeKeys')} </Button>For the Clear button:
- <Button - variant="secondary" - size="large" + <Button + variant="secondary" + size="large" + icon={ + <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> + <polyline points="3,6 5,6 21,6" /> + <path d="m19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" /> + </svg> + } onClick={handleClear} disabled={state.isTesting} - className="btn-fixed-lg flex items-center gap-xs" + className="btn-fixed-lg" > - <svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> - <polyline points="3,6 5,6 21,6" /> - <path d="m19 6v14a2 2 0 0 1-2 2H7a2 2 0 0 1-2-2V6m3 0V4a2 2 0 0 1 2-2h4a2 2 0 0 1 2 2v2" /> - </svg> {t('clear')} </Button>Also applies to: 131-143
public/worker.js (3)
353-355: Status code propagation looks good; consider deriving once per attemptLGTM. Minor: compute const statusCode = extractStatusCode(result.error) right after testApiKey() returns and reuse in all KEY_STATUS_UPDATEs for this attempt to avoid repeated parsing. Also consider including statusCode in the final postLogEvent.extra for consistency; ensure the UI handles null.
Also applies to: 384-386, 412-414, 459-461
504-506: Make extractStatusCode more robustTighten matching:
- Allow trailing spaces after the closing parenthesis.
- Match “HTTP 4xx/5xx” case‑insensitively and without the pre‑check.
Apply this minimal tweak first:
- const match = error.match(/\((\d{3})\)$/); + const match = error.match(/\((\d{3})\)\s*$/);Optionally simplify the HTTP branch and add radix=10:
// outside selected lines; illustrative full replacement function extractStatusCode(error) { if (!error || typeof error !== 'string') return null; const paren = error.match(/\((\d{3})\)\s*$/); if (paren) return parseInt(paren[1], 10); const http = error.match(/HTTP\s+(\d{3})/i); if (http) return parseInt(http[1], 10); return null; }Also applies to: 510-510
483-487: Avoid retrying on 403; consider 408 instead403 is typically non‑transient. Recommend removing 403 from the retryable list and optionally adding 408 (Request Timeout).
- if ([403, 502, 503, 504].includes(statusCode)) { + if ([408, 502, 503, 504].includes(statusCode)) { return true; }src/components/features/LogDisplayControl/index.jsx (1)
16-30: Improve switch accessibility: associate label and use switch semantics.Link the visible label to the input and add role/aria for better a11y.
- <label className="text-sm font-medium text-primary"> + <label className="text-sm font-medium text-primary" htmlFor="toggle-detailed-logs"> {t('showDetailedLogs')} </label> <div className="space-y-sm"> <div className="flex items-center gap-sm"> <label className="switch-base"> <input type="checkbox" + id="toggle-detailed-logs" checked={state.showDetailedLogs} onChange={handleToggleDetailedLogs} disabled={state.isTesting} className="switch-input" + role="switch" + aria-checked={state.showDetailedLogs} /> <span className="switch-slider"></span> </label>src/components/features/Results/StatsCards.jsx (1)
11-46: Compute counts in one pass to avoid repeated filters.Single reduce over keyResults improves readability and performance.
- const mainStats = [ - { - key: 'total', - value: state.keyResults.length, - className: 'total' - }, - { - key: 'valid', - value: state.enablePaidDetection ? - state.keyResults.filter(k => k.status === 'valid').length : - state.keyResults.filter(k => k.status === 'valid' || k.status === 'paid').length, - className: 'valid' - }, - { - key: 'invalid', - value: state.keyResults.filter(k => k.status === 'invalid').length, - className: 'invalid' - }, - { - key: 'rateLimited', - value: state.keyResults.filter(k => k.status === 'rate-limited').length, - className: 'rate-limited' - } - ]; - - const hasPaidDetection = state.apiType === 'gemini' && state.enablePaidDetection; - - if (hasPaidDetection) { - mainStats.push({ - key: 'paidKeys', - value: state.keyResults.filter(k => k.status === 'paid').length, - className: 'paid' - }); - } + const counts = state.keyResults.reduce((acc, k) => { + acc.total += 1; + acc[k.status] = (acc[k.status] || 0) + 1; + return acc; + }, { total: 0 }); + + const hasPaidDetection = state.apiType === 'gemini' && state.enablePaidDetection; + const validCount = hasPaidDetection + ? (counts['valid'] || 0) + : (counts['valid'] || 0) + (counts['paid'] || 0); + + const mainStats = [ + { key: 'total', value: counts.total, className: 'total' }, + { key: 'valid', value: validCount, className: 'valid' }, + { key: 'invalid', value: counts['invalid'] || 0, className: 'invalid' }, + { key: 'rateLimited', value: counts['rate-limited'] || 0, className: 'rate-limited' }, + ...(hasPaidDetection + ? [{ key: 'paidKeys', value: counts['paid'] || 0, className: 'paid' }] + : []) + ];src/components/features/AdvancedSettings/index.jsx (1)
46-61: Add dialog semantics for the modal container.Improve accessibility with role="dialog", aria-modal, and label association.
- <Card - variant="base" - padding="none" - className="w-full max-w-2xl max-h-90vh overflow-y-auto m-md" - > - <div className="flex items-center justify-between p-lg border-b"> - <h3 className="text-lg font-semibold text-primary">{t('advancedSettings')}</h3> + <Card + variant="base" + padding="none" + className="w-full max-w-2xl max-h-90vh overflow-y-auto m-md" + role="dialog" + aria-modal="true" + aria-labelledby="advancedSettingsTitle" + > + <div className="flex items-center justify-between p-lg border-b"> + <h3 id="advancedSettingsTitle" className="text-lg font-semibold text-primary">{t('advancedSettings')}</h3>Additionally, consider moving the inline z-index on the overlay into a CSS class for consistency.
src/components/common/Card/index.jsx (2)
27-39: Verify padding class naming consistency.'lg' returns 'p-lg' while others use 'card-padding*'. Ensure 'p-lg' exists or align naming (e.g., 'card-padding-lg').
9-11: Drop hover prop to match “remove hover effects and transitions”.To reflect the PR goal, remove hover prop and card-hover class.
- hover = false, className = '', onClick, ...props }) => { @@ - const hoverClass = hover ? 'card-hover' : ''; const clickableClass = onClick ? 'cursor-pointer' : ''; const classes = [ baseClass, paddingClass, - hoverClass, clickableClass, className ].filter(Boolean).join(' ');Also applies to: 44-53
src/components/features/Results/VirtualizedList.jsx (2)
158-162: Use functional state update when toggling visibility.Avoid stale closures; use prev => !prev.
- const handleToggleKeyVisibility = (e) => { + const handleToggleKeyVisibility = (e) => { e.stopPropagation(); - setIsKeyVisible(!isKeyVisible); + setIsKeyVisible(prev => !prev); };
216-221: Redundant keyboard handler on a real button.Buttons already handle Enter/Space. Remove onKeyDown to prevent double-invoke.
- <button - className="key-info-row key-info-clickable" - onClick={handleOpenLogs} - onKeyDown={handleKeyDown} - aria-label={t('openLogs') || 'Open logs for this key'} - > + <button + className="key-info-row key-info-clickable" + onClick={handleOpenLogs} + aria-label={t('openLogs') || 'Open logs for this key'} + >src/contexts/AppStateContext.jsx (1)
504-509: Clear logs when disabling detailed logs: good. Also guard opening modal when disabled.To keep state consistent, ignore OPEN_LOG_MODAL when showDetailedLogs is false.
Add a guard in the reducer:
// Inside appReducer, before returning in OPEN_LOG_MODAL: case 'OPEN_LOG_MODAL': if (!state.showDetailedLogs) { return state; // ignore when detailed logs are off } return { ...state, activeLogKey: action.payload, isLogModalOpen: true };src/components/common/Modal/index.jsx (2)
69-69: Consider renaming 'log-modal-content' to a more generic class name.The class name
log-modal-contentappears to be a remnant from domain-specific usage. For a reusable Modal component, consider using a more generic name likemodal-content.Apply this diff to use a more generic class name:
- className={`log-modal-content ${maxWidth} m-md ${className}`} + className={`modal-content ${maxWidth} m-md ${className}`}
111-116: Add optional chaining for onConfirm to prevent runtime errors.Line 113 calls
onConfirmdirectly without checking if it exists. If a confirm-type modal is rendered without providingonConfirm, clicking the Confirm button will throw a runtime error.Apply this diff to add defensive optional chaining:
<Button variant="primary" - onClick={onConfirm} + onClick={onConfirm ? () => onConfirm() : undefined} > {confirmText || t('confirm')} </Button>Note: This matches the optional chaining pattern used for
onCloseat lines 42 and 54.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
public/worker.js(5 hunks)src/App.jsx(4 hunks)src/__tests__/hooks/useVirtualization.test.js(2 hunks)src/components/common/Button/index.jsx(2 hunks)src/components/common/Card/index.jsx(1 hunks)src/components/common/EmptyState/index.jsx(1 hunks)src/components/common/FormField/index.jsx(1 hunks)src/components/common/IconButton/index.jsx(1 hunks)src/components/common/Modal/Modal.module.css(0 hunks)src/components/common/Modal/index.jsx(3 hunks)src/components/common/StatCard/index.jsx(1 hunks)src/components/common/StatusBadge/index.jsx(1 hunks)src/components/features/AdvancedSettings/index.jsx(3 hunks)src/components/features/ApiConfig/ApiConfig.module.css(0 hunks)src/components/features/ApiConfig/ModelSelector.jsx(1 hunks)src/components/features/ApiConfig/ProxySettings.jsx(0 hunks)src/components/features/ConcurrencyControl/ConcurrencyControl.module.css(6 hunks)src/components/features/ConcurrencyControl/index.jsx(2 hunks)src/components/features/Controls/index.jsx(3 hunks)src/components/features/KeyInput/FileImport.jsx(2 hunks)src/components/features/KeyInput/KeyInput.module.css(0 hunks)src/components/features/KeyInput/PasteButton.jsx(2 hunks)src/components/features/LogDisplayControl/index.jsx(1 hunks)src/components/features/LogsPreview/KeyLogModal.jsx(3 hunks)src/components/features/LogsPreview/LogsPreviewPanel.jsx(4 hunks)src/components/features/PaidDetectionControl/PaidDetectionControl.module.css(0 hunks)src/components/features/PaidDetectionControl/index.jsx(6 hunks)src/components/features/PaidDetectionPrompt/PaidDetectionPrompt.module.css(0 hunks)src/components/features/Results/Results.module.css(0 hunks)src/components/features/Results/StatsCards.jsx(2 hunks)src/components/features/Results/VirtualizedList.jsx(6 hunks)src/components/features/Results/index.jsx(2 hunks)src/components/features/RetryControl/RetryControl.module.css(7 hunks)src/components/features/RetryControl/index.jsx(2 hunks)src/components/layout/AppLayout/AppLayout.module.css(3 hunks)src/components/layout/AppLayout/index.jsx(1 hunks)src/components/layout/NavBar/NavBar.module.css(0 hunks)src/components/layout/Sidebar/Sidebar.module.css(4 hunks)src/components/layout/ThemeToggle/ThemeToggle.module.css(0 hunks)src/components/layout/ThemeToggle/index.jsx(0 hunks)src/contexts/AppStateContext.jsx(6 hunks)src/hooks/useVirtualization.js(1 hunks)src/locales/en.json(1 hunks)src/locales/zh.json(1 hunks)src/styles/components.css(4 hunks)src/styles/globals.css(6 hunks)
💤 Files with no reviewable changes (10)
- src/components/features/ApiConfig/ProxySettings.jsx
- src/components/features/ApiConfig/ApiConfig.module.css
- src/components/common/Modal/Modal.module.css
- src/components/features/PaidDetectionControl/PaidDetectionControl.module.css
- src/components/features/KeyInput/KeyInput.module.css
- src/components/layout/NavBar/NavBar.module.css
- src/components/layout/ThemeToggle/index.jsx
- src/components/features/PaidDetectionPrompt/PaidDetectionPrompt.module.css
- src/components/features/Results/Results.module.css
- src/components/layout/ThemeToggle/ThemeToggle.module.css
🚧 Files skipped from review as they are similar to previous changes (13)
- src/components/common/EmptyState/index.jsx
- src/components/features/ApiConfig/ModelSelector.jsx
- src/components/layout/AppLayout/index.jsx
- src/App.jsx
- src/components/common/StatCard/index.jsx
- src/components/common/StatusBadge/index.jsx
- src/components/features/RetryControl/index.jsx
- src/locales/en.json
- src/components/common/Button/index.jsx
- src/components/features/PaidDetectionControl/index.jsx
- src/components/common/IconButton/index.jsx
- src/components/features/KeyInput/FileImport.jsx
- src/components/features/LogsPreview/LogsPreviewPanel.jsx
🧰 Additional context used
🧬 Code graph analysis (13)
src/components/features/LogDisplayControl/index.jsx (2)
src/contexts/AppStateContext.jsx (3)
useAppState(524-530)useAppState(524-530)state(370-370)src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/features/ConcurrencyControl/index.jsx (2)
src/components/common/FormField/index.jsx (1)
FormField(3-48)src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/features/Results/index.jsx (5)
src/components/features/Results/StatsCards.jsx (1)
StatsCards(6-97)src/components/features/Results/ResultTabs.jsx (1)
ResultTabs(5-39)src/components/features/Results/VirtualizedList.jsx (2)
VirtualizedList(247-318)EmptyState(240-245)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/features/Results/CopyButtons.jsx (1)
CopyButtons(6-148)
src/components/features/KeyInput/PasteButton.jsx (2)
src/contexts/LanguageContext.jsx (1)
t(81-121)src/contexts/AppStateContext.jsx (1)
state(370-370)
src/components/features/LogsPreview/KeyLogModal.jsx (5)
src/contexts/AppStateContext.jsx (1)
state(370-370)src/components/common/Modal/index.jsx (1)
Modal(7-135)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/EmptyState/index.jsx (1)
EmptyState(3-39)src/components/common/Card/index.jsx (1)
Card(3-70)
src/components/features/Results/StatsCards.jsx (2)
src/components/common/StatCard/index.jsx (1)
StatCard(3-36)src/contexts/LanguageContext.jsx (1)
t(81-121)
src/components/features/AdvancedSettings/index.jsx (3)
src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/IconButton/index.jsx (1)
IconButton(4-27)src/components/features/LogDisplayControl/index.jsx (1)
LogDisplayControl(5-41)
src/components/common/Modal/index.jsx (5)
src/components/features/AdvancedSettings/index.jsx (1)
handleOverlayClick(38-42)src/components/common/Card/index.jsx (1)
Card(3-70)src/components/common/IconButton/index.jsx (1)
IconButton(4-27)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/common/Card/index.jsx (5)
src/components/common/Button/index.jsx (2)
baseClass(14-14)classes(21-23)src/components/common/FormField/index.jsx (2)
baseClass(58-58)classes(64-66)src/components/common/StatCard/index.jsx (2)
baseClass(12-12)classes(14-16)src/components/common/StatusBadge/index.jsx (2)
baseClass(22-22)classes(27-29)src/components/common/EmptyState/index.jsx (1)
classes(15-17)
src/components/features/Controls/index.jsx (1)
src/components/common/Button/index.jsx (1)
Button(3-40)
src/components/common/FormField/index.jsx (2)
src/components/common/Button/index.jsx (4)
baseClass(14-14)variantClass(15-18)sizeClass(19-20)classes(21-23)src/components/common/Card/index.jsx (2)
baseClass(42-42)classes(47-53)
src/__tests__/hooks/useVirtualization.test.js (2)
src/components/features/Results/VirtualizedList.jsx (1)
useVirtualization(250-250)src/hooks/useVirtualization.js (2)
useVirtualization(3-19)useVirtualization(3-19)
src/components/features/Results/VirtualizedList.jsx (4)
src/components/features/BalanceDisplay/KeyBalanceDisplay.jsx (3)
useLanguage(7-7)useAppState(8-8)KeyBalanceDisplay(6-77)src/contexts/LanguageContext.jsx (1)
t(81-121)src/components/common/StatusBadge/index.jsx (1)
getStatusClass(10-20)src/hooks/useVirtualization.js (1)
getItemHeight(4-13)
🔇 Additional comments (21)
src/components/features/Controls/index.jsx (1)
8-8: LGTM!The import is correct and supports the refactoring to use the shared Button component.
src/components/layout/Sidebar/Sidebar.module.css (1)
200-205: Reduced-motion override restored — thanks. Instant collapses without animation now respect users’ motion preferences.src/components/layout/AppLayout/AppLayout.module.css (1)
41-65: LGTM! Flex-based layout correctly implements equal-height columns.The transition from grid to flex is well-executed. The
align-items: stretchcombined withflex: 1on the columns ensures equal heights, and the responsive breakpoint correctly switches to a vertical stack on smaller screens.src/locales/zh.json (1)
206-209: LGTM! New localization keys support detailed logs feature.The added keys
showDetailedLogsandshowDetailedLogsDescriptionproperly support the new log display control feature introduced in this PR.src/components/common/FormField/index.jsx (1)
1-92: LGTM! Well-implemented accessible form field component.The FormField component correctly implements accessibility features:
- Stable ID generation via
useRefprevents regeneration on each render- Proper
aria-invalidandaria-describedbyapplied to form controls- Error and help blocks use stable IDs for ARIA association
- Input subcomponent uses
forwardReffor proper ref forwardingsrc/components/features/ConcurrencyControl/index.jsx (1)
27-59: LGTM! FormField integration and accessibility improvements are well-executed.The component correctly wraps controls in FormField and adds proper ARIA labels to both the slider and numeric input for screen reader support.
src/components/features/ConcurrencyControl/ConcurrencyControl.module.css (1)
1-242: LGTM! Comprehensive theming refactor with CSS variables.The file has been successfully migrated to use CSS variables throughout, replacing hard-coded colors with theme-aware tokens like
var(--bg-card),var(--border-color), andvar(--bg-input). Dark theme adaptations are properly implemented.src/styles/components.css (2)
119-124: LGTM! Textarea styling correctly scoped.The
.form-field.textareaselector properly applies special styling only to textarea-type form fields.
351-453: LGTM! New card variants and settings button properly implemented.The new card variants (
.card--primary,.card--info,.card--results,.card--empty-state) and.btn--settingsare well-structured with proper dark theme adaptations.src/styles/globals.css (2)
406-541: LGTM! Semi Design-inspired input/button components well-implemented.The semi-input-wrapper and semi-button components are properly styled with focus states, hover effects, and size variants that align with the Semi Design aesthetic.
625-659: LGTM! Scrollbar accessibility addressed with gradient fade indicator.The capsule scrolling now includes a right-edge gradient fade to indicate overflow while hiding the scrollbar, addressing the previous accessibility concern about hidden scrollbars.
src/components/features/LogDisplayControl/index.jsx (1)
9-12: Reducer action and translation keys validated -SET_SHOW_DETAILED_LOGSis implemented and all related i18n keys (showDetailedLogs,showDetailedLogsDescription,enabled,disabled) are defined.src/components/features/Results/StatsCards.jsx (1)
73-91: All translation keys are defined. The keys total, valid, invalid, rateLimited, paidKeys, testing, and retrying exist in both src/locales/en.json and zh.json.src/__tests__/hooks/useVirtualization.test.js (1)
9-15: Solid coverage and isolation of matchMedia mocks.Scenarios and restoration look correct; tests align with the new no-arg API. LGTM.
Also applies to: 32-49, 51-68, 70-87, 89-111, 113-122, 124-145
src/components/features/LogsPreview/KeyLogModal.jsx (1)
179-191: Modal usage and title rendering look correct.Fragment title avoids invalid h3 nesting; maxWidth/className wiring is fine.
src/components/features/Results/VirtualizedList.jsx (1)
274-277: LGTM: itemSize delegates to viewport-based height.Matches the simplified hook API.
src/contexts/AppStateContext.jsx (4)
111-113: Add showDetailedLogs to initial state: good.Defaults to false with localStorage hydration.
208-213: Reducer action for showDetailedLogs is correct.State update is straightforward.
419-425: Persistence wiring includes showDetailedLogs: good.Saved with other global settings.
492-503: Collector enablement tied to showDetailedLogs: good.Effect dependency is correct; hydration guarded.
src/components/common/Modal/index.jsx (1)
73-95: Title rendering implementation correctly addresses past feedback.The type check
typeof title === 'string'properly handles both string and React element titles, resolving the invalid HTML concern raised in the previous review. The implementation maintains accessibility with appropriate heading semantics for strings while allowing flexible custom content.
| {(title || action) && ( | ||
| <div className="card-header flex items-center justify-between mb-4"> | ||
| {title && <h3 className="card-title text-lg font-semibold">{title}</h3>} | ||
| {action} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid invalid HTML by not wrapping non-string titles in h3.
Render string titles inside h3; render React nodes as-is.
- {(title || action) && (
- <div className="card-header flex items-center justify-between mb-4">
- {title && <h3 className="card-title text-lg font-semibold">{title}</h3>}
- {action}
- </div>
- )}
+ {(title || action) && (
+ <div className="card-header flex items-center justify-between mb-4">
+ {title && (
+ typeof title === 'string'
+ ? <h3 className="card-title text-lg font-semibold">{title}</h3>
+ : title
+ )}
+ {action}
+ </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.
| {(title || action) && ( | |
| <div className="card-header flex items-center justify-between mb-4"> | |
| {title && <h3 className="card-title text-lg font-semibold">{title}</h3>} | |
| {action} | |
| </div> | |
| )} | |
| {(title || action) && ( | |
| <div className="card-header flex items-center justify-between mb-4"> | |
| {title && ( | |
| typeof title === 'string' | |
| ? <h3 className="card-title text-lg font-semibold">{title}</h3> | |
| : title | |
| )} | |
| {action} | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In src/components/common/Card/index.jsx around lines 61 to 66, the current code
always wraps title in an <h3>, which can produce invalid HTML when title is a
React node; change the render so that if title is a string (typeof title ===
"string" or use React.isValidElement to detect nodes) you output the <h3
className="card-title...">{title}</h3>, otherwise render the title value
directly (no h3 wrapper). Keep the surrounding header markup and ensure
null/undefined titles are still skipped.
| max="100" | ||
| onChange={handleInputChange} | ||
| disabled={state.isTesting} | ||
| aria-label={t('concurrencyControl') + ' ' + (t('value') || 'value')} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify translation key 'value' exists.
Line 54 constructs an aria-label using t('value'). Ensure this key exists in the localization files (zh.json and en.json) to avoid displaying a raw key name.
Run the following script to check if the 'value' translation key exists:
🏁 Script executed:
#!/bin/bash
# Check if 'value' key exists in localization files
rg -n '"value"' src/locales/Length of output: 0
🏁 Script executed:
#!/bin/bash
# List locale JSON files and check for 'value' key
find src/locales -type f -name '*.json' -print -exec grep -n '"value"' {} +Length of output: 39
Add missing translation key ‘value’
The key 'value' isn’t defined in src/locales/en.json or src/locales/zh.json, so t('value') will fall back to the raw key. Add appropriate entries for both locales.
🤖 Prompt for AI Agents
In src/components/features/ConcurrencyControl/index.jsx around line 54, the call
t('value') references a missing translation key; add a "value" entry to both
locale files (src/locales/en.json and src/locales/zh.json) — e.g. add "value":
"value" to en.json and "value": "值" (or the preferred Chinese term) to zh.json —
then save and reload the app or rebuild translations so t('value') returns the
localized string instead of the raw key.
| <> | ||
| {(state.showResults || state.isTesting || state.keyResults.length > 0) ? ( | ||
| <> | ||
| <StatsCards /> | ||
|
|
||
| <div className="function-card results-card"> | ||
| {state.showLogTooltip && ( | ||
| <div className="results-tooltip" role="note"> | ||
| <div className="results-tooltip__text">{t('results.logTooltip')}</div> | ||
| <button | ||
| type="button" | ||
| className="results-tooltip__close" | ||
| onClick={handleTooltipDismiss} | ||
| aria-label={t('close') || 'Close'} | ||
| > | ||
| × | ||
| </button> | ||
| </div> | ||
| )} | ||
| <ResultTabs /> | ||
| <div className="results-content"> | ||
| <VirtualizedList /> | ||
| </div> | ||
| <CopyButtons /> | ||
| </div> | ||
| </> | ||
| ) : ( | ||
| <div className="function-card initial-empty-state-card"> | ||
| <div className="empty-state"> | ||
| <div className="empty-icon"> | ||
| <svg width="48" height="48" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> | ||
| <circle cx="11" cy="11" r="8" /> | ||
| <path d="m21 21-4.35-4.35" /> | ||
| </svg> | ||
| </div> | ||
| <div className="empty-text">{t('resultsWillShow')}</div> | ||
| {/* 检测结果统计 - 始终显示，但在没有结果时显示0 */} | ||
| <StatsCards /> | ||
|
|
||
| {/* 密钥列表区域 - 始终显示结果卡片 */} | ||
| <div className="card--primary card--results"> | ||
| {state.showLogTooltip && ( | ||
| <div className="results-tooltip" role="note"> | ||
| <div className="results-tooltip__text">{t('results.logTooltip')}</div> | ||
| <button | ||
| type="button" | ||
| className="results-tooltip__close" | ||
| onClick={handleTooltipDismiss} | ||
| aria-label={t('close') || 'Close'} | ||
| > | ||
| × | ||
| </button> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Tab列表始终显示 */} | ||
| <ResultTabs /> | ||
|
|
||
| <div className="results-content"> | ||
| {(state.showResults || state.isTesting || state.keyResults.length > 0) ? ( | ||
| <VirtualizedList /> | ||
| ) : ( | ||
| <EmptyState | ||
| icon={ | ||
| <svg width="48" height="48" viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth="2" strokeLinecap="round" strokeLinejoin="round"> | ||
| <circle cx="11" cy="11" r="8" /> | ||
| <path d="m21 21-4.35-4.35" /> | ||
| </svg> | ||
| } | ||
| message={t('resultsWillShow')} | ||
| /> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
| {/* 复制按钮始终显示在底部 */} | ||
| <CopyButtons /> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
Results layout consolidation looks good.
Consistent EmptyState use and always-visible sections improve UX. No functional issues spotted.
Optionally confirm translations exist for results.logTooltip and resultsWillShow:
🏁 Script executed:
#!/bin/bash
rg -n "results\\.logTooltip|resultsWillShow" -S src/locales || trueLength of output: 147
Add missing translation for results.logTooltip
The key results.logTooltip isn’t defined in your locale files—add it to src/locales/zh.json and en.json alongside resultsWillShow.
🤖 Prompt for AI Agents
In src/components/features/Results/index.jsx around lines 21 to 62, the UI
references a missing translation key results.logTooltip; add this key to both
locale files (src/locales/en.json and src/locales/zh.json) alongside the
existing resultsWillShow entry, providing appropriate English and Chinese
strings (e.g., English: "Hover to view log details" and Chinese: "将鼠标悬停以查看日志详情"
or your preferred copy), then run the localization/test script to ensure the new
key is picked up and no i18n warnings remain.
| const getItemHeight = useCallback(() => { | ||
| // 根据屏幕大小返回固定高度 | ||
| if (window.matchMedia && window.matchMedia('(max-width: 480px)').matches) { | ||
| return 62; // 超小屏幕：60px + wrapper padding(2px) | ||
| } else if (window.matchMedia && window.matchMedia('(max-width: 768px)').matches) { | ||
| return 66; // 移动端：64px + wrapper padding(2px) | ||
| } else { | ||
| return 74; // 桌面端：72px + wrapper padding(2px) | ||
| } | ||
|
|
||
| // 如果有重试信息，增加一行 | ||
| if (keyData.retryCount > 0) { | ||
| baseHeight += lineHeight; | ||
| } | ||
|
|
||
| // 如果有状态信息（有效密钥信息），增加一行 | ||
| if (keyData.status === 'valid' || keyData.status === 'paid') { | ||
| baseHeight += lineHeight; | ||
| } | ||
|
|
||
| // 如果是硅基流动且是有效密钥，为余额显示增加额外高度 | ||
| if (apiType === 'siliconcloud' && (keyData.status === 'valid' || keyData.status === 'paid')) { | ||
| baseHeight += lineHeight; // 为余额显示增加一行高度 | ||
| } | ||
|
|
||
| return Math.max(baseHeight, 68); // 设置最小高度为68px | ||
| }, []); |
There was a problem hiding this comment.
Guard against undefined window to avoid SSR/test crashes.
Accessing window.matchMedia can throw outside the browser.
- const getItemHeight = useCallback(() => {
- // 根据屏幕大小返回固定高度
- if (window.matchMedia && window.matchMedia('(max-width: 480px)').matches) {
+ const getItemHeight = useCallback(() => {
+ // 根据屏幕大小返回固定高度（在无 window 环境下安全回退）
+ const canMatch = typeof window !== 'undefined' && typeof window.matchMedia === 'function';
+ if (canMatch && window.matchMedia('(max-width: 480px)').matches) {
return 62; // 超小屏幕：60px + wrapper padding(2px)
- } else if (window.matchMedia && window.matchMedia('(max-width: 768px)').matches) {
+ } else if (canMatch && window.matchMedia('(max-width: 768px)').matches) {
return 66; // 移动端：64px + wrapper padding(2px)
} else {
return 74; // 桌面端：72px + wrapper padding(2px)
}
}, []);Optional: re-compute/reset list on viewport change (matchMedia 'change' or resize) to keep item heights in sync.
📝 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.
| const getItemHeight = useCallback(() => { | |
| // 根据屏幕大小返回固定高度 | |
| if (window.matchMedia && window.matchMedia('(max-width: 480px)').matches) { | |
| return 62; // 超小屏幕：60px + wrapper padding(2px) | |
| } else if (window.matchMedia && window.matchMedia('(max-width: 768px)').matches) { | |
| return 66; // 移动端：64px + wrapper padding(2px) | |
| } else { | |
| return 74; // 桌面端：72px + wrapper padding(2px) | |
| } | |
| // 如果有重试信息，增加一行 | |
| if (keyData.retryCount > 0) { | |
| baseHeight += lineHeight; | |
| } | |
| // 如果有状态信息（有效密钥信息），增加一行 | |
| if (keyData.status === 'valid' || keyData.status === 'paid') { | |
| baseHeight += lineHeight; | |
| } | |
| // 如果是硅基流动且是有效密钥，为余额显示增加额外高度 | |
| if (apiType === 'siliconcloud' && (keyData.status === 'valid' || keyData.status === 'paid')) { | |
| baseHeight += lineHeight; // 为余额显示增加一行高度 | |
| } | |
| return Math.max(baseHeight, 68); // 设置最小高度为68px | |
| }, []); | |
| const getItemHeight = useCallback(() => { | |
| // 根据屏幕大小返回固定高度（在无 window 环境下安全回退） | |
| const canMatch = typeof window !== 'undefined' && typeof window.matchMedia === 'function'; | |
| if (canMatch && window.matchMedia('(max-width: 480px)').matches) { | |
| return 62; // 超小屏幕：60px + wrapper padding(2px) | |
| } else if (canMatch && window.matchMedia('(max-width: 768px)').matches) { | |
| return 66; // 移动端：64px + wrapper padding(2px) | |
| } else { | |
| return 74; // 桌面端：72px + wrapper padding(2px) | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In src/hooks/useVirtualization.js around lines 4 to 13, the useCallback directly
calls window.matchMedia which will throw in SSR/test environments; guard against
undefined window by checking typeof window !== 'undefined' before accessing
matchMedia and return a sensible default height when window is unavailable. Also
wire up (optionally) a matchMedia 'change' listener or window resize listener to
recompute/reset the list item height when the viewport changes, update the
stored height via state or ref, and ensure you remove the listener in cleanup to
avoid leaks.
| /* 新的横向布局密钥项 */ | ||
| .key-item-horizontal { | ||
| display: flex; | ||
| align-items: center; | ||
| padding: 16px 20px; | ||
| background: var(--bg-input); | ||
| /* 使用主题变量 */ | ||
| border: 2px solid var(--border-color); | ||
| border-radius: 12px; | ||
| gap: 16px; | ||
| box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1); | ||
| margin-bottom: 1px; | ||
| height: 72px; | ||
| width: 100%; | ||
| box-sizing: border-box; | ||
| cursor: pointer; | ||
| transition: none; | ||
| /* 移除过渡效果 */ | ||
| } |
There was a problem hiding this comment.
Avoid hard-coding card height.
Line 307 sets a fixed height: 72px which will clip longer status texts, error messages, or multi-line metadata. Use min-height instead to allow the card to expand with its content.
Apply this diff:
.key-item-horizontal {
display: flex;
align-items: center;
padding: 16px 20px;
background: var(--bg-input);
border: 2px solid var(--border-color);
border-radius: 12px;
gap: 16px;
box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1);
margin-bottom: 1px;
- height: 72px;
+ min-height: 72px;
width: 100%;
box-sizing: border-box;
cursor: pointer;
transition: none;
}📝 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.
| /* 新的横向布局密钥项 */ | |
| .key-item-horizontal { | |
| display: flex; | |
| align-items: center; | |
| padding: 16px 20px; | |
| background: var(--bg-input); | |
| /* 使用主题变量 */ | |
| border: 2px solid var(--border-color); | |
| border-radius: 12px; | |
| gap: 16px; | |
| box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1); | |
| margin-bottom: 1px; | |
| height: 72px; | |
| width: 100%; | |
| box-sizing: border-box; | |
| cursor: pointer; | |
| transition: none; | |
| /* 移除过渡效果 */ | |
| } | |
| /* 新的横向布局密钥项 */ | |
| .key-item-horizontal { | |
| display: flex; | |
| align-items: center; | |
| padding: 16px 20px; | |
| background: var(--bg-input); | |
| border: 2px solid var(--border-color); | |
| border-radius: 12px; | |
| gap: 16px; | |
| box-shadow: 0 2px 8px rgba(0, 0, 0, 0.1); | |
| margin-bottom: 1px; | |
| min-height: 72px; | |
| width: 100%; | |
| box-sizing: border-box; | |
| cursor: pointer; | |
| transition: none; | |
| } |
🤖 Prompt for AI Agents
In src/styles/globals.css around lines 295 to 313, the .key-item-horizontal rule
currently uses a fixed height: 72px which can clip multi-line content; replace
the fixed height with min-height: 72px (and remove height) so the card keeps the
intended baseline size but can expand for longer text, keeping display:flex and
align-items as-is (adjust vertical alignment to start if needed for multiline
layouts).
Summary by CodeRabbit