-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: admin theme #42
base: main
Are you sure you want to change the base?
fix: admin theme #42
Conversation
WalkthroughThis pull request involves the deletion of the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
==========================================
- Coverage 59.07% 58.92% -0.16%
==========================================
Files 148 147 -1
Lines 13860 13822 -38
==========================================
- Hits 8188 8144 -44
- Misses 5672 5678 +6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (12)
components/groups/components/CountdownTimer.tsx (1)
31-31
: Clean up extra spaces in class namesThe countdown class has extra spaces which should be removed for consistency.
Apply this diff to clean up the class names:
- <span className="countdown text-xl"> + <span className="countdown text-xl">tailwind.config.js (1)
96-99
: Document color system choicesConsider adding comments to document:
- The rationale behind the chosen colors
- Accessibility considerations
- Usage guidelines for status colors
This will help maintain consistency as the theme evolves.Also applies to: 119-122
components/admins/components/validatorList.tsx (2)
Line range hint
83-87
: Consider moving inline styles to Tailwind classesWhile the text color update is good, consider moving the inline styles to Tailwind classes for better maintainability:
-className="text-secondary-content" -style={{ fontSize: '20px', fontWeight: 700, lineHeight: '24px' }} +className="text-secondary-content text-xl font-bold leading-6"
198-200
: Consider formatting consensus power for better readabilityThe consensus power is displayed as a raw number which might be hard to read for large values.
-{validator.consensus_power?.toString() ?? 'N/A'} +{validator.consensus_power + ? new Intl.NumberFormat().format(Number(validator.consensus_power)) + : 'N/A' +}components/groups/components/myGroups.tsx (3)
112-122
: Standardize column width definitionsThe column widths are inconsistent:
- Some columns use
w-1/6
- "Group Name" uses
w-[25%]
- "Active proposals" uses
w-[15%]
This could cause layout issues as the widths don't total 100%.
Consider standardizing all column widths to use either fractions or percentages:
-<th className="bg-transparent w-1/6">Group Name</th> -<th className="bg-transparent w-1/6">Active proposals</th> -<th className="bg-transparent w-1/6">Authors</th> -<th className="bg-transparent w-1/6">Group Balance</th> -<th className="bg-transparent w-1/6">Qualified Majority</th> -<th className="bg-transparent w-1/6">Group address</th> +<th className="bg-transparent w-[25%]">Group Name</th> +<th className="bg-transparent w-[15%]">Active proposals</th> +<th className="bg-transparent w-[15%]">Authors</th> +<th className="bg-transparent w-[15%]">Group Balance</th> +<th className="bg-transparent w-[15%]">Qualified Majority</th> +<th className="bg-transparent w-[15%]">Group address</th>
124-152
: Add tests for skeleton loading stateThe skeleton loading implementation looks good, but it lacks test coverage. Consider adding tests to verify the loading state renders correctly.
Would you like me to help generate test cases for the skeleton loading state? The tests should verify:
- Correct number of skeleton rows are rendered
- Skeleton elements have proper styling classes
- Loading state transitions correctly
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 126-150: components/groups/components/myGroups.tsx#L126-L150
Added lines #L126 - L150 were not covered by tests
227-227
: Use semantic color classes for textThe text color is hardcoded using
text-black dark:text-white
while other components use semantic color classes.Consider using semantic color classes for consistency:
-className="group text-black dark:text-white rounded-lg cursor-pointer transition-colors" +className="group text-base-content rounded-lg cursor-pointer transition-colors"components/groups/components/groupProposals.tsx (2)
271-280
: Enhance search input accessibilityWhile the styling changes look good, consider improving accessibility by:
- Adding an
aria-label
for screen readers- Associating the search icon with the input using
aria-labelledby
<div className="relative"> <input type="text" placeholder="Search for a proposal..." + aria-label="Search proposals" className="input input-bordered w-[224px] h-[40px] rounded-[12px] border-none bg-secondary text-secondary-content pl-10" value={searchTerm} onChange={e => setSearchTerm(e.target.value)} /> - <SearchIcon className="h-6 w-6 absolute left-3 top-1/2 transform -translate-y-1/2 text-gray-400 " /> + <SearchIcon + aria-hidden="true" + className="h-6 w-6 absolute left-3 top-1/2 transform -translate-y-1/2 text-gray-400" + /> </div>
388-388
: Consider a more concise messageThe current message "No proposal was found" is grammatically correct but could be more concise.
- <div className="text-center py-8 text-gray-500">No proposal was found.</div> + <div className="text-center py-8 text-gray-500">No proposals found</div>components/groups/modals/voteDetailsModal.tsx (3)
Line range hint
355-382
: Simplify message rendering logicThe message rendering logic has repetitive className usage and complex nesting. Consider extracting common styles and simplifying the structure.
+ const messageStyles = { + heading: "font-medium text-primary-content", + content: "text-primary-content", + container: (depth: number) => ({ marginLeft: `${depth * 20}px` }) + }; return ( - <div key={key} style={{ marginLeft: `${depth * 20}px` }}> - <h4 className="font-medium text-primary-content">{key}:</h4> + <div key={key} style={messageStyles.container(depth)}> + <h4 className={messageStyles.heading}>{key}:</h4> {typeof value === 'string' && value.match(/^[a-zA-Z0-9]{40,}$/) ? ( <TruncatedAddressWithCopy slice={14} address={value} /> ) : ( - <p className="text-primary-content" title={String(value)}> + <p className={messageStyles.content} title={String(value)}>🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 376-376: components/groups/modals/voteDetailsModal.tsx#L376
Added line #L376 was not covered by tests
[warning] 380-382: components/groups/modals/voteDetailsModal.tsx#L380-L382
Added lines #L380 - L382 were not covered by tests
Line range hint
229-229
: Fix typo in function nameThe function name 'executeWithdrawl' is misspelled. It should be 'executeWithdrawal'.
- const executeWithdrawl = async () => { + const executeWithdrawal = async () => {Make sure to update all references to this function in the component.
Line range hint
355-534
: Add test coverage for new UI changesThe static analysis indicates that the new UI changes lack test coverage. This includes message rendering, theme-based styling, and modal interactions.
Consider adding the following test cases:
describe('VoteDetailsModal', () => { it('should render message fields correctly', () => { // Test message rendering }); it('should apply correct theme-based styles', () => { // Test theme changes }); it('should handle modal interactions', () => { // Test modal open/close }); });Would you like me to help create these test cases or open a GitHub issue to track this task?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 463-465: components/groups/modals/voteDetailsModal.tsx#L463-L465
Added lines #L463 - L465 were not covered by tests
[warning] 469-469: components/groups/modals/voteDetailsModal.tsx#L469
Added line #L469 was not covered by tests
[warning] 476-477: components/groups/modals/voteDetailsModal.tsx#L476-L477
Added lines #L476 - L477 were not covered by tests
[warning] 481-482: components/groups/modals/voteDetailsModal.tsx#L481-L482
Added lines #L481 - L482 were not covered by tests
[warning] 489-489: components/groups/modals/voteDetailsModal.tsx#L489
Added line #L489 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
components/admins/components/__tests__/adminOptions.test.tsx
(0 hunks)components/admins/components/adminOptions.tsx
(0 hunks)components/admins/components/index.tsx
(0 hunks)components/admins/components/validatorList.tsx
(4 hunks)components/groups/components/CountdownTimer.tsx
(1 hunks)components/groups/components/__tests__/groupProposals.test.tsx
(1 hunks)components/groups/components/groupProposals.tsx
(4 hunks)components/groups/components/myGroups.tsx
(5 hunks)components/groups/modals/voteDetailsModal.tsx
(10 hunks)tailwind.config.js
(1 hunks)
💤 Files with no reviewable changes (3)
- components/admins/components/tests/adminOptions.test.tsx
- components/admins/components/adminOptions.tsx
- components/admins/components/index.tsx
🧰 Additional context used
📓 Learnings (1)
components/admins/components/validatorList.tsx (1)
Learnt from: chalabi2
PR: liftedinit/manifest-app#9
File: components/admins/components/validatorList.tsx:77-204
Timestamp: 2024-11-12T13:03:18.927Z
Learning: In `components/admins/components/validatorList.tsx`, the loading state is managed by the parent component, so it's not necessary to handle it within `ValidatorList`.
🪛 GitHub Check: codecov/patch
components/admins/components/validatorList.tsx
[warning] 147-147: components/admins/components/validatorList.tsx#L147
Added line #L147 was not covered by tests
[warning] 153-153: components/admins/components/validatorList.tsx#L153
Added line #L153 was not covered by tests
[warning] 156-156: components/admins/components/validatorList.tsx#L156
Added line #L156 was not covered by tests
[warning] 159-159: components/admins/components/validatorList.tsx#L159
Added line #L159 was not covered by tests
components/groups/components/myGroups.tsx
[warning] 126-150: components/groups/components/myGroups.tsx#L126-L150
Added lines #L126 - L150 were not covered by tests
components/groups/modals/voteDetailsModal.tsx
[warning] 355-355: components/groups/modals/voteDetailsModal.tsx#L355
Added line #L355 was not covered by tests
[warning] 357-357: components/groups/modals/voteDetailsModal.tsx#L357
Added line #L357 was not covered by tests
[warning] 366-366: components/groups/modals/voteDetailsModal.tsx#L366
Added line #L366 was not covered by tests
[warning] 376-376: components/groups/modals/voteDetailsModal.tsx#L376
Added line #L376 was not covered by tests
[warning] 380-382: components/groups/modals/voteDetailsModal.tsx#L380-L382
Added lines #L380 - L382 were not covered by tests
[warning] 463-465: components/groups/modals/voteDetailsModal.tsx#L463-L465
Added lines #L463 - L465 were not covered by tests
[warning] 469-469: components/groups/modals/voteDetailsModal.tsx#L469
Added line #L469 was not covered by tests
[warning] 476-477: components/groups/modals/voteDetailsModal.tsx#L476-L477
Added lines #L476 - L477 were not covered by tests
[warning] 481-482: components/groups/modals/voteDetailsModal.tsx#L481-L482
Added lines #L481 - L482 were not covered by tests
[warning] 489-489: components/groups/modals/voteDetailsModal.tsx#L489
Added line #L489 was not covered by tests
[warning] 511-511: components/groups/modals/voteDetailsModal.tsx#L511
Added line #L511 was not covered by tests
[warning] 517-518: components/groups/modals/voteDetailsModal.tsx#L517-L518
Added lines #L517 - L518 were not covered by tests
[warning] 520-520: components/groups/modals/voteDetailsModal.tsx#L520
Added line #L520 was not covered by tests
[warning] 522-524: components/groups/modals/voteDetailsModal.tsx#L522-L524
Added lines #L522 - L524 were not covered by tests
[warning] 528-528: components/groups/modals/voteDetailsModal.tsx#L528
Added line #L528 was not covered by tests
[warning] 531-533: components/groups/modals/voteDetailsModal.tsx#L531-L533
Added lines #L531 - L533 were not covered by tests
🔇 Additional comments (8)
components/groups/components/CountdownTimer.tsx (1)
29-29
: Verify theme color visibility
The addition of text-primary-content
class suggests this is part of the theme fixes. Please ensure the text remains visible in both light and dark modes.
Let's check the theme configuration and usage:
✅ Verification successful
Theme color visibility is properly configured
The text-primary-content class is correctly implemented in the theme system:
- Light mode: primary-content is set to #161616 (dark text)
- Dark mode: primary-content is set to #FFFFFF (white text)
- The project uses 'class' for dark mode strategy
- The class is consistently used across multiple components for text that needs to contrast with primary backgrounds
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify theme configuration and usage of primary-content color
# Check tailwind config for theme colors
echo "Checking tailwind config for theme colors..."
rg -A 10 "primary-content" tailwind.config.js
# Check for other usages of primary-content to ensure consistency
echo "Checking other usages of primary-content..."
rg "text-primary-content" --type tsx --type jsx
Length of output: 1174
Script:
#!/bin/bash
# Let's try with correct file type patterns and check theme-related configurations
echo "Checking tailwind config for theme setup..."
rg -A 5 "darkMode|theme:" tailwind.config.js
echo "Checking primary-content usage in TypeScript/React files..."
rg "text-primary-content" -g "*.tsx" -g "*.jsx" -g "*.ts" -g "*.js"
echo "Checking CountdownTimer component theme-related classes..."
rg -A 5 -B 5 "className.*text-primary-content" components/groups/components/CountdownTimer.tsx
Length of output: 3788
components/groups/components/__tests__/groupProposals.test.tsx (1)
111-111
: LGTM! Let's verify text consistency.
The test assertion update aligns with the component changes.
Let's verify that this text change is consistent across the codebase:
✅ Verification successful
Text update is consistent across the codebase
The verification shows that:
- The old text "No proposals found" has been completely removed
- The new text "No proposal was found" appears consistently in:
- The component file (
components/groups/components/groupProposals.tsx
) - The test file (
components/groups/components/__tests__/groupProposals.test.tsx
)
- The component file (
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old text and verify the new text is used consistently
# Check for any remaining instances of the old text
echo "Checking for old text 'No proposals found':"
rg -i "No proposals found"
# Check for consistency of new text
echo "Checking usage of new text 'No proposal was found':"
rg -i "No proposal was found"
Length of output: 556
tailwind.config.js (2)
81-89
: Review color contrast ratios for accessibility
The primary color (#A087FF) might not provide sufficient contrast against white backgrounds for smaller text elements. Consider adjusting the primary color to meet WCAG AA standards for all text sizes.
#!/bin/bash
# Check if any components use primary color for small text on white background
rg -l "text-primary.*text-sm|text-sm.*text-primary" --type tsx
115-122
: Verify dark theme readability
The dark theme background (#0E0A1F) is very dark. While the contrast is good, ensure that UI elements remain distinguishable and that there's enough visual hierarchy.
✅ Verification successful
Dark theme implementation appears well-designed and accessible
The dark theme implementation shows careful consideration for readability and contrast:
-
Base colors are properly configured with sufficient contrast ratios:
- Background (#0E0A1F) with white text (#FFFFFF) provides strong contrast
- Base-300 (#272336) is used appropriately for secondary surfaces
- Status colors (info, success, warning, error) match light theme for consistency
-
Extensive dark mode styles are implemented using Tailwind's dark: modifier with:
- Proper text color adjustments (dark:text-white, dark:text-gray-400 etc.)
- Background opacity variations for different surface levels
- Hover state handling with appropriate contrast
- Consistent border colors and shadows
The implementation follows accessibility best practices and maintains visual hierarchy in dark mode.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for potential readability issues in dark mode
# Search for components using base-300 background with dark text
rg "bg-base-300.*text-\w+-content|text-\w+-content.*bg-base-300" --type tsx
# Check for any custom dark mode styles
rg "dark:" --type tsx
Length of output: 186
Script:
#!/bin/bash
# Look for potential readability issues in dark mode
# Search for components using base-300 background with dark text
rg "bg-base-300.*text-\w+-content|text-\w+-content.*bg-base-300"
# Check for any custom dark mode styles that might affect readability
rg "dark:"
# Search for any Tailwind dark mode configuration
rg "darkMode" tailwind.config.js
# Check for any color-related CSS custom properties or variables
rg "var\(--.*color|--.*color\)"
Length of output: 60539
components/admins/components/validatorList.tsx (2)
92-94
: LGTM! Search input styling improvements
The updated styling maintains good contrast while aligning with the new theme.
100-114
: LGTM! Filter buttons styling improvements
The styling provides clear visual distinction between active/inactive states while maintaining theme consistency.
components/groups/components/myGroups.tsx (1)
Line range hint 86-95
: LGTM: Improved theme consistency
The changes properly utilize Tailwind's semantic color classes (text-secondary-content
, bg-secondary
) instead of hardcoded colors, which better supports theme switching.
components/groups/components/groupProposals.tsx (1)
244-249
: LGTM: Theme-consistent styling updates
The styling changes in the header section properly align with the theme system, using semantic color classes like bg-secondary
and text-primary-content
.
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max"> | ||
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max"> | ||
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max text-primary-content"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove duplicate grid container structure
There are two nested div
elements with almost identical grid classes, which creates unnecessary DOM depth.
Apply this diff to simplify the structure:
- <div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max">
- <div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max text-primary-content">
+ <div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max text-primary-content">
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max"> | |
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max"> | |
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max text-primary-content"> | |
<div className="grid grid-flow-col gap-5 mt-2 text-center auto-cols-max text-primary-content"> |
'secondary-content': '#161616', | ||
accent: '#30DB5B', | ||
'accent-content': '#161616', | ||
neutral: '#FFFFFF', | ||
'button-gradient': 'linear-gradient(to right, #A087FF, #380CC5)', | ||
'neutral-content': '#161616', | ||
'neutral-content': '#FFFFFF', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix potential color visibility issue
The neutral-content
color (#FFFFFF) matches the neutral
color (#FFFFFF), which would make text invisible. This appears to be an oversight.
- 'neutral-content': '#FFFFFF',
+ 'neutral-content': '#161616',
📝 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.
'neutral-content': '#FFFFFF', | |
'neutral-content': '#161616', |
<td className="bg-secondary rounded-l-[12px] w-1/6"> | ||
<div className="flex items-center space-x-3"> | ||
<div className="skeleton w-10 h-8 rounded-full shrink-0"></div> | ||
<div className="skeleton h-3 w-24"></div> | ||
</div> | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] w-1/6"> | ||
<td className="bg-secondary w-1/6"> | ||
<div className="skeleton h-2 w-24"></div> | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] w-1/6"> | ||
<td className="bg-secondary w-1/6"> | ||
<div className="skeleton h-2 w-8"></div> | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] w-1/6 rounded-r-[12px] text-right"> | ||
<td className="bg-secondary w-1/6 rounded-r-[12px] text-right"> | ||
<div className="skeleton h-2 w-8 ml-auto"></div> | ||
</td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add visual regression tests for loading state
While the loading UI implementation looks good, it lacks test coverage. Consider adding visual regression tests to ensure the loading state UI remains consistent across changes.
Would you like me to help generate visual regression tests using a tool like Playwright or Cypress?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 147-147: components/admins/components/validatorList.tsx#L147
Added line #L147 was not covered by tests
[warning] 153-153: components/admins/components/validatorList.tsx#L153
Added line #L153 was not covered by tests
[warning] 156-156: components/admins/components/validatorList.tsx#L156
Added line #L156 was not covered by tests
[warning] 159-159: components/admins/components/validatorList.tsx#L159
Added line #L159 was not covered by tests
className="group text-black dark:text-white rounded-lg cursor-pointer" | ||
> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] rounded-l-[12px] px-4 py-4 w-[25%]"> | ||
<td className="bg-secondary group-hover:bg-base-300 rounded-l-[12px] px-4 py-4 w-[25%]"> | ||
{proposal.id.toString()} | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] truncate max-w-xs px-4 py-4 w-[25%]"> | ||
<td className="bg-secondary group-hover:bg-base-300 truncate max-w-xs px-4 py-4 w-[25%]"> | ||
{proposal.title} | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] px-4 py-4 w-[25%]"> | ||
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | ||
{timeLeft} | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] px-4 py-4 w-[25%]"> | ||
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | ||
{proposal.messages.length > 0 | ||
? proposal.messages.map((message, index) => ( | ||
<div key={index}>{getHumanReadableType((message as any)['@type'])}</div> | ||
)) | ||
: 'No messages'} | ||
</td> | ||
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] rounded-r-[12px] px-4 py-4 w-[25%]"> | ||
<td className="bg-secondary group-hover:bg-base-300 rounded-r-[12px] px-4 py-4 w-[25%]"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve table row keyboard accessibility
While the styling changes look good, the table rows should be keyboard accessible since they're clickable.
<tr
key={proposal.id.toString()}
onClick={() => handleRowClick(proposal)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ handleRowClick(proposal);
+ }
+ }}
+ role="button"
+ tabIndex={0}
className="group text-black dark:text-white rounded-lg cursor-pointer"
>
📝 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.
className="group text-black dark:text-white rounded-lg cursor-pointer" | |
> | |
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] rounded-l-[12px] px-4 py-4 w-[25%]"> | |
<td className="bg-secondary group-hover:bg-base-300 rounded-l-[12px] px-4 py-4 w-[25%]"> | |
{proposal.id.toString()} | |
</td> | |
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] truncate max-w-xs px-4 py-4 w-[25%]"> | |
<td className="bg-secondary group-hover:bg-base-300 truncate max-w-xs px-4 py-4 w-[25%]"> | |
{proposal.title} | |
</td> | |
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] px-4 py-4 w-[25%]"> | |
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | |
{timeLeft} | |
</td> | |
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] px-4 py-4 w-[25%]"> | |
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | |
{proposal.messages.length > 0 | |
? proposal.messages.map((message, index) => ( | |
<div key={index}>{getHumanReadableType((message as any)['@type'])}</div> | |
)) | |
: 'No messages'} | |
</td> | |
<td className="dark:bg-[#FFFFFF0F] bg-[#FFFFFF] rounded-r-[12px] px-4 py-4 w-[25%]"> | |
<td className="bg-secondary group-hover:bg-base-300 rounded-r-[12px] px-4 py-4 w-[25%]"> | |
key={proposal.id.toString()} | |
onClick={() => handleRowClick(proposal)} | |
onKeyDown={(e) => { | |
if (e.key === 'Enter' || e.key === ' ') { | |
e.preventDefault(); | |
handleRowClick(proposal); | |
} | |
}} | |
role="button" | |
tabIndex={0} | |
className="group text-black dark:text-white rounded-lg cursor-pointer" | |
> | |
<td className="bg-secondary group-hover:bg-base-300 rounded-l-[12px] px-4 py-4 w-[25%]"> | |
{proposal.id.toString()} | |
</td> | |
<td className="bg-secondary group-hover:bg-base-300 truncate max-w-xs px-4 py-4 w-[25%]"> | |
{proposal.title} | |
</td> | |
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | |
{timeLeft} | |
</td> | |
<td className="bg-secondary group-hover:bg-base-300 px-4 py-4 w-[25%]"> | |
{proposal.messages.length > 0 | |
? proposal.messages.map((message, index) => ( | |
<div key={index}>{getHumanReadableType((message as any)['@type'])}</div> | |
)) | |
: 'No messages'} | |
</td> | |
<td className="bg-secondary group-hover:bg-base-300 rounded-r-[12px] px-4 py-4 w-[25%]"> |
@@ -66,7 +66,7 @@ | |||
|
|||
const { theme } = useTheme(); | |||
|
|||
const textColor = theme === 'dark' ? '#E0D1D4' : '#2e2e2e'; | |||
const textColor = theme === 'dark' ? '#FFFFFF' : '#161616'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Move theme colors to configuration
Hardcoding theme colors directly in the component reduces maintainability. Consider moving these colors to your theme configuration file for better maintainability and consistency.
- const textColor = theme === 'dark' ? '#FFFFFF' : '#161616';
+ import { useThemeColors } from '@/theme';
+ const { textColor } = useThemeColors();
Committable suggestion skipped: line range outside the PR's diff.
<div className="modal-box relative max-w-4xl min-h-96 max-h-[80vh] overflow-y-hidden flex flex-col md:flex-row md:ml-20 -mt-12 rounded-[24px] shadow-lg bg-secondary transition-all duration-300 z-[1000]"> | ||
<form method="dialog" onSubmit={onClose}> | ||
<button className="btn btn-sm btn-circle btn-ghost absolute right-2 top-2">✕</button> | ||
<button className="btn btn-sm btn-circle text-black dark:text-white btn-ghost absolute right-2 top-2"> | ||
✕ | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance modal accessibility
The modal implementation could benefit from additional accessibility features.
- <dialog id={modalId} className="modal">
+ <dialog
+ id={modalId}
+ className="modal"
+ aria-labelledby="modal-title"
+ role="dialog"
+ aria-modal="true"
+ >
<div className="modal-box relative max-w-4xl min-h-96 max-h-[80vh] overflow-y-hidden flex flex-col md:flex-row md:ml-20 -mt-12 rounded-[24px] shadow-lg bg-secondary transition-all duration-300 z-[1000]">
- <button className="btn btn-sm btn-circle text-black dark:text-white btn-ghost absolute right-2 top-2">
+ <button
+ className="btn btn-sm btn-circle text-black dark:text-white btn-ghost absolute right-2 top-2"
+ aria-label="Close modal"
+ >
Committable suggestion skipped: line range outside the PR's diff.
This PR fixes the
Admin
page themes.This PR contains #41
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
ValidatorList
,GroupProposals
, andVoteDetailsModal
, to align with a new design system.Chores
AdminOptions
component and its associated tests to streamline the codebase.