fix(ui): improve admin dialogs UX and fix i18n issues #514
Conversation
…hange Use requestAnimationFrame to delay rowVirtualizer.measure() call, ensuring DOM has updated before recalculating row positions. This fixes table rows overlapping when switching status filter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add max-height constraint and flex layout to error-rules dialogs to keep save/cancel buttons and close icon visible when content exceeds viewport height. Mirrors the pattern used in request-filters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add max-height constraint and flex layout to sensitive-words dialogs to keep save/cancel buttons and close icon visible when content exceeds viewport height. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add missing translations for timeout, geminiAuthFallback, and cacheTtl sections in provider form components. Changes: - api-test-button.tsx: Replace Chinese timeout labels and Gemini auth fallback warning with t() calls - provider-form.tsx: Replace Chinese Cache TTL labels with t() calls - Add translations to all 5 locales (EN, RU, JA, ZH-CN, ZH-TW) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Move Provider Group field to after Provider Type (before Forward client IP) - Add autocomplete suggestions from existing provider groups - Fetch groups via getDistinctProviderGroupsAction on component mount - Keep ability to enter new custom groups via TagInput 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Apply the same scrollable layout pattern from Request Filters dialog to provider add/edit/clone dialogs: - Fixed header (DialogHeader with flex-shrink-0) - Scrollable content area (overflow-y-auto flex-1) - Fixed footer with action buttons (DialogFooter with flex-shrink-0) This improves UX on smaller screens where provider forms are tall. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add early URL validation in previewProxyUrls() to prevent "Invalid URL" error when user is typing an incomplete URL in the provider form. The error was thrown by new URL() constructor before the try-catch could handle it gracefully in development mode. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Remove duplicate 'addProvider' keys that were erroneously added to the end of settings.json files in en, ja, ru, and zh-TW locales. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Summary of ChangesHello @miraserver, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience and internationalization capabilities of the admin interface. It tackles common UI frustrations like content overflow in dialogs and virtualizer glitches, while also ensuring the application is more robust against invalid inputs and better localized for various languages. The changes aim to make the admin panel more intuitive and reliable for users worldwide. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on internationalizing UI elements, enhancing user experience with scrollable dialogs, and improving the provider management features. Specifically, new translation keys were added across multiple language files for API timeout settings, Gemini authentication fallback warnings, and prompt cache TTL overrides. The ApiTestButton and ProviderForm components were updated to use these new translation keys, replacing hardcoded Chinese strings. The ProviderForm also gained the ability to fetch and suggest existing provider groups for the group tag input, and its layout was refactored to ensure the header and footer remain visible while the content scrolls. Similar scrollable dialog patterns were applied to AddRuleDialog, EditRuleDialog, AddProviderDialog, ProviderRichListItem (for edit/clone dialogs), AddWordDialog, and EditWordDialog components. An optimization was introduced in UserManagementTable to defer rowVirtualizer.measure() to the next animation frame for better rendering performance. Additionally, url.ts was updated to include a try-catch block for URL validation in previewProxyUrls to prevent exceptions. Review comments highlighted the need to translate Chinese comments into English for consistency and to add error handling for the getDistinctProviderGroupsAction promise in ProviderForm.
| useEffect(() => { | ||
| getDistinctProviderGroupsAction().then((res) => { | ||
| if (res.ok && res.data) { | ||
| setGroupSuggestions(res.data); | ||
| } | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
The comment on line 236 is in Chinese. For consistency with the rest of the codebase, please use English for comments. Also, the promise returned by getDistinctProviderGroupsAction is not fully handled. It's good practice to handle the error case, for example by logging it to the console. This helps with debugging if the suggestions fail to load.
// Fetch existing provider groups for suggestions
useEffect(() => {
getDistinctProviderGroupsAction().then((res) => {
if (res.ok && res.data) {
setGroupSuggestions(res.data);
} else if (!res.ok) {
console.error("Failed to fetch provider group suggestions:", res.error);
}
});
}, []);
| // 验证 URL 格式有效性（防止 new URL() 抛出异常） | ||
| try { | ||
| new URL(baseUrl); | ||
| } catch { | ||
| // URL 无效，返回空预览 | ||
| return previews; | ||
| } |
There was a problem hiding this comment.
The comments here are in Chinese. To maintain consistency across the codebase, please write comments in English.
| // 验证 URL 格式有效性（防止 new URL() 抛出异常） | |
| try { | |
| new URL(baseUrl); | |
| } catch { | |
| // URL 无效，返回空预览 | |
| return previews; | |
| } | |
| // Validate URL format to prevent `new URL()` from throwing an exception. | |
| try { | |
| new URL(baseUrl); | |
| } catch { | |
| // If the URL is invalid, return an empty preview object. | |
| return previews; | |
| } |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR addresses multiple UI/UX improvements and i18n fixes for admin dialogs. The changes are well-structured and follow existing patterns in the codebase.
PR Size: XL
- Lines changed: 2538
- Files changed: 15
Split Suggestion: This PR mixes multiple concerns that could be split into smaller, more focused PRs:
- Dialog scrollability fixes (add-rule-dialog, edit-rule-dialog, add-provider-dialog, sensitive-words dialogs)
- Virtualizer row overlap fix (user-management-table.tsx)
- i18n updates (5 locale files)
- Provider form autocomplete feature
- URL validation fix
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Medium Priority Issues (Should Fix)
1. Missing cleanup for requestAnimationFrame in user-management-table.tsx:262-266
- Location: src/app/[locale]/dashboard/_components/user/user-management-table.tsx:266
- Issue: The cleanup function returns the result of
cancelAnimationFrame, which isundefined, instead of being a proper cleanup function - Impact: Minor memory leak potential - the RAF is cancelled but the return pattern is incorrect
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - One minor issue found
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (manual test plan provided)
- Code clarity - Good
Notable Improvements
- Fixed virtualizer row overlap issue with proper RAF timing
- Made dialogs scrollable with fixed headers/footers (improves UX on small screens)
- Removed duplicate i18n keys across multiple locales
- Added autocomplete for Provider Group field
- Fixed hardcoded Chinese strings in provider forms
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
This PR addresses multiple UI/UX improvements and i18n fixes for admin dialogs. The changes are well-structured and follow existing patterns in the codebase.
PR Size: XL
- Lines changed: 2538
- Files changed: 15
Split Suggestion: This PR mixes multiple concerns that could be split into smaller, more focused PRs:
- Dialog scrollability fixes (add-rule-dialog, edit-rule-dialog, add-provider-dialog, sensitive-words dialogs)
- Virtualizer row overlap fix (user-management-table.tsx)
- i18n updates (5 locale files)
- Provider form autocomplete feature
- URL validation fix
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 1 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Medium Priority Issues (Should Fix)
1. Incorrect cleanup function return pattern
File: src/app/[locale]/dashboard/_components/user/user-management-table.tsx:262-268
Current code:
useEffect(() => {
if (\!scrollResetKey) return;
parentRef.current?.scrollTo({ top: 0 });
// Defer measurement to next frame to ensure DOM has updated
const rafId = requestAnimationFrame(() => {
rowVirtualizer.measure();
});
return () => cancelAnimationFrame(rafId); // ⚠️ Issue here
}, [scrollResetKey, rowVirtualizer]);Problem: The cleanup function returns the result of cancelAnimationFrame(rafId) (which is undefined). While this doesn't cause runtime errors, it's not the correct pattern for React cleanup functions.
Why this matters:
- React cleanup functions should not return values
- The current pattern is
return () => cancelAnimationFrame(rafId)which executes the function and returns its result - This is subtly different from the intended pattern
Suggested fix:
return () => {
cancelAnimationFrame(rafId);
};Confidence Score: 82/100
- ✅ Issue exists in NEW code (+30)
- ✅ Can point to exact line (+20)
- ✅ Violates React best practices (+20)
- ✅ Affects code clarity (+10)
⚠️ Low severity (code works but not optimal) (-8)
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - One minor issue found
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate (manual test plan provided)
- Code clarity - Good
Notable Improvements
- ✅ Fixed virtualizer row overlap issue with proper RAF timing
- ✅ Made dialogs scrollable with fixed headers/footers (improves UX on small screens)
- ✅ Removed duplicate i18n keys across multiple locales
- ✅ Added autocomplete for Provider Group field using
getDistinctProviderGroupsAction - ✅ Fixed hardcoded Chinese strings in provider forms
- ✅ Applied proper Flexbox layout to dialogs (
flex flex-col) for consistent scroll behavior
Architectural Observations
- The dialog refactoring follows a consistent pattern across all affected files
- The use of
flex-shrink-0on headers/footers andoverflow-y-autoon content is the correct approach - The virtualizer fix properly defers measurement to the next frame, avoiding race conditions
Automated review by Claude AI
Address PR review comment - add .catch() handler to prevent unhandled promise rejection in getDistinctProviderGroupsAction. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
Summary
addProviderkeys in settings.json (en, ja, ru, zh-TW)Related Issues
Changes
Core Changes
user-management-table.tsx:262-268): Defer rowVirtualizer.measure() to next frame using requestAnimationFrame to prevent row overlap when filter changesprovider-form.tsx:236-242): Fetch existing provider groups and show as autocomplete suggestionsapi-test-button.tsx,provider-form.tsx): Replace hardcoded Chinese strings with translation keys for timeout, geminiAuthFallback, and cacheTtl sectionsurl.ts:512-519): Add early validation in previewProxyUrls() to prevent "Invalid URL" error during typingSupporting Changes
addProvidertranslation keys from settings.json filesTest plan
Description enhanced by Claude AI