Skip to content
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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

fmorency
Copy link
Contributor

@fmorency fmorency commented Nov 15, 2024

This PR fixes the Admin page themes.

This PR contains #41

2024-11-15_08-53
2024-11-15_08-53_1

Summary by CodeRabbit

Release Notes

  • New Features

    • No new features added; however, several components have undergone significant visual updates for improved user experience.
  • Bug Fixes

    • Updated text assertions in tests to reflect accurate messaging for "no proposals" state.
  • Style

    • Enhanced styling across various components, including ValidatorList, GroupProposals, and VoteDetailsModal, to align with a new design system.
    • Adjusted color schemes and hover effects for improved readability and consistency.
  • Chores

    • Removed the AdminOptions component and its associated tests to streamline the codebase.

@fmorency fmorency self-assigned this Nov 15, 2024
Copy link

coderabbitai bot commented Nov 15, 2024

Walkthrough

This pull request involves the deletion of the AdminOptions component and its associated unit tests from the codebase. Additionally, modifications were made to the index.tsx file to remove exports related to AdminOptions. Several other components, including ValidatorList, CountdownTimer, GroupProposals, YourGroups, and VoteDetailsModal, received updates primarily focused on styling and minor logic adjustments. The tailwind.config.js file was also updated to reflect new theme colors for both light and dark modes.

Changes

File Path Change Summary
components/admins/components/tests/adminOptions.test.tsx Deleted unit tests for AdminOptions component.
components/admins/components/adminOptions.tsx Deleted AdminOptions component.
components/admins/components/index.tsx Removed export for adminOptions.
components/admins/components/validatorList.tsx Updated styling and structure of ValidatorList; modified class names and hover effects.
components/groups/components/CountdownTimer.tsx Enhanced styling by adding classes and cleaning up class definitions in CountdownTimer.
components/groups/components/tests/groupProposals.test.tsx Updated assertion text in tests for ProposalsForPolicy.
components/groups/components/groupProposals.tsx Adjusted layout and styling in GroupProposals, including header and table updates.
components/groups/components/myGroups.tsx Updated UI and styling for YourGroups, including header and table structure modifications.
components/groups/modals/voteDetailsModal.tsx Modified visual styling and logic adjustments in VoteDetailsModal.
tailwind.config.js Updated theme colors for both light and dark modes in Tailwind configuration.

Possibly related PRs

  • fix: admin page layout & refactor #38: The changes in this PR involve modifications to the ValidatorList component, which includes the addition of an isLoading prop that enhances loading state management, similar to the loading state checks in the deleted adminOptions.test.tsx file.

Suggested reviewers

  • chalabi2

🐰 In the meadow, changes bloom,
AdminOptions gone, there's more room.
Styles refreshed, colors anew,
ValidatorList shines, with a clearer view.
In every corner, a touch of delight,
Hop along, everything feels just right! 🌼


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

Attention: Patch coverage is 62.66667% with 56 lines in your changes missing coverage. Please review.

Project coverage is 58.92%. Comparing base (dbc6ca0) to head (f87bb77).

Files with missing lines Patch % Lines
components/groups/modals/voteDetailsModal.tsx 37.20% 27 Missing ⚠️
components/groups/components/myGroups.tsx 63.76% 25 Missing ⚠️
components/admins/components/validatorList.tsx 75.00% 4 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

Copy link

@coderabbitai coderabbitai bot left a 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 names

The 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 choices

Consider 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 classes

While 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 readability

The 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 definitions

The 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 state

The 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:

  1. Correct number of skeleton rows are rendered
  2. Skeleton elements have proper styling classes
  3. 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 text

The 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 accessibility

While the styling changes look good, consider improving accessibility by:

  1. Adding an aria-label for screen readers
  2. 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 message

The 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 logic

The 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 name

The 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 changes

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between dbc6ca0 and f87bb77.

📒 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)
🏁 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.

Comment on lines 28 to +29
<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">
Copy link

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.

Suggested change
<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',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
'neutral-content': '#FFFFFF',
'neutral-content': '#161616',

Comment on lines +147 to 161
<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>
Copy link

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

Comment on lines +357 to +375
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%]">
Copy link

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.

Suggested change
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';
Copy link

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.

Comment on lines +424 to +428
<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>
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant