-
Notifications
You must be signed in to change notification settings - Fork 180
Feature/dynamic candidate management #201
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
base: main
Are you sure you want to change the base?
Feature/dynamic candidate management #201
Conversation
- Add CandidateSection, CandidateCard, CandidateCounter components - Add useCandidateValidation hook with memoization - Add validation for min 2 candidates, empty fields, duplicate names - Add visual candidate counter with warning/success states - Add real-time validation on field blur - Add smooth animations with Framer Motion (300ms) - Add mobile-responsive layout - Add unit tests for validation utilities and hook (18 tests) - Add vitest configuration Closes AOSSIE-Org#157
- Add CandidateSection, CandidateCard, CandidateCounter components - Add useCandidateValidation hook with memoization - Add validation for min 2 candidates, empty fields, duplicate names - Add visual candidate counter with warning/success states - Add real-time validation on field blur - Add smooth animations with Framer Motion (300ms) - Add mobile-responsive layout - Add unit tests for validation utilities and hook (18 tests) - Add vitest configuration Closes AOSSIE-Org#157
WalkthroughAdds an ID-centric dynamic candidate management feature: validation helpers, a validation hook, UI components (CandidateCard, CandidateCounter, CandidateSection), tests, and Vitest configuration; the create page is refactored to use the new candidate subsystem and touched-field validation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CreatePage
participant CandidateSection
participant CandidateCard
participant useValidation as useCandidateValidation
participant Helpers as ValidationHelpers
User->>CreatePage: Open create form / add candidate
CreatePage->>CandidateSection: addCandidate()
CandidateSection->>CandidateSection: create candidate with generateCandidateId()
User->>CandidateCard: Edit name/description
CandidateCard->>CandidateSection: onUpdateCandidate(id, field, value)
CandidateSection->>CreatePage: propagate updated candidates
CreatePage->>useValidation: validate(candidates)
useValidation->>Helpers: findDuplicateIds(candidates)
useValidation->>Helpers: findEmptyFields(candidates)
Helpers-->>useValidation: duplicateIds, emptyFields
useValidation-->>CreatePage: ValidationResult (isValid, errors, messages)
CreatePage->>CandidateSection: pass visible validation errors (touched)
CandidateSection->>CandidateCard: render with isDuplicate / emptyFields flags
User->>CandidateCard: Click delete
CandidateCard->>CandidateSection: onRemoveCandidate(id)
CandidateSection->>CreatePage: update candidates -> revalidate via useValidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
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: 3
🧹 Nitpick comments (2)
client/app/create/page.tsx (2)
74-96: Consider memoizing visible validation errors to avoid recalculation on every render.
getVisibleValidationErrorsis a function that gets called during render (at line 181). Since it's wrapped inuseCallback, the function identity is stable, but it's still executed on every render. Consider usinguseMemoinstead to cache the computed result.-const getVisibleValidationErrors = useCallback(() => { +const visibleValidationErrors = useMemo(() => { const visibleEmptyFields = new Map<number, Set<keyof Candidate>>(); validation.errors.emptyFields.forEach((fields, index) => { const touched = touchedFields.get(index); if (touched) { const visibleFields = new Set<keyof Candidate>(); fields.forEach((field) => { if (touched.has(field)) { visibleFields.add(field); } }); if (visibleFields.size > 0) { visibleEmptyFields.set(index, visibleFields); } } }); return { duplicateIndices: validation.errors.duplicateIndices, emptyFields: visibleEmptyFields, }; }, [validation.errors, touchedFields]);Then update usage at line 181:
-validationErrors={getVisibleValidationErrors()} +validationErrors={visibleValidationErrors}
104-108: Validation on submit doesn't mark all fields as touched before showing error.When the user submits without interacting with candidate fields,
validation.isValidmay be false (e.g., 0 candidates), but the inline field errors won't be visible becausetouchedFieldsis empty. The toast shows the first error, but visual field indicators remain hidden.Consider marking all candidate fields as touched on submit attempt so users see which specific fields need attention.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
client/app/create/components/CandidateCard.tsx(1 hunks)client/app/create/components/CandidateCounter.tsx(1 hunks)client/app/create/components/CandidateSection.tsx(1 hunks)client/app/create/page.tsx(5 hunks)client/app/helpers/candidateValidation.test.ts(1 hunks)client/app/helpers/candidateValidation.ts(1 hunks)client/app/hooks/useCandidateValidation.test.ts(1 hunks)client/app/hooks/useCandidateValidation.ts(1 hunks)client/package.json(2 hunks)client/vitest.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
client/app/helpers/candidateValidation.test.ts (1)
client/app/helpers/candidateValidation.ts (3)
Candidate(6-9)findDuplicateIndices(28-48)findEmptyFields(54-75)
client/app/create/components/CandidateCard.tsx (1)
client/app/helpers/candidateValidation.ts (1)
Candidate(6-9)
client/app/hooks/useCandidateValidation.ts (1)
client/app/helpers/candidateValidation.ts (6)
CandidateValidationErrors(11-14)Candidate(6-9)findDuplicateIndices(28-48)findEmptyFields(54-75)MIN_CANDIDATES(22-22)VALIDATION_MESSAGES(16-20)
client/app/create/page.tsx (4)
client/app/helpers/candidateValidation.ts (1)
Candidate(6-9)client/app/hooks/useCandidateValidation.ts (1)
useCandidateValidation(22-54)client/app/constants.ts (1)
ELECTION_FACTORY_ADDRESS(3-4)client/abi/artifacts/ElectionFactory.ts (1)
ElectionFactory(1-322)
client/app/create/components/CandidateSection.tsx (1)
client/app/helpers/candidateValidation.ts (3)
Candidate(6-9)CandidateValidationErrors(11-14)MIN_CANDIDATES(22-22)
🔇 Additional comments (14)
client/vitest.config.ts (1)
1-17: LGTM! Clean Vitest configuration.The configuration is well-structured with appropriate settings for React testing (jsdom environment, React plugin, globals enabled). The path alias and test file pattern are properly configured.
client/package.json (1)
10-11: LGTM! Test scripts configured correctly.The test scripts are well-defined:
testfor CI/single runs andtest:watchfor development.client/app/helpers/candidateValidation.test.ts (1)
1-125: LGTM! Comprehensive test coverage for validation helpers.The test suite thoroughly covers both
findDuplicateIndicesandfindEmptyFieldsfunctions with appropriate edge cases including case-insensitivity, whitespace handling, and empty value scenarios. The assertions are clear and specific.client/app/create/components/CandidateCard.tsx (1)
1-101: LGTM! Well-implemented component with good accessibility and UX.The component properly handles:
- Validation state visualization with conditional styling
- 300ms animations with performance optimization (willChange)
- Accessibility attributes (aria-invalid, aria-label)
- Mobile-friendly touch targets (min-h-[44px])
- Clear user feedback for duplicate names
client/app/hooks/useCandidateValidation.ts (1)
1-54: LGTM! Well-designed validation hook with proper memoization.The hook correctly:
- Uses
useMemoto avoid unnecessary recomputation- Aggregates validation errors from helper functions
- Returns comprehensive validation state (isValid, errors, errorMessages)
- Has appropriate dependency array with just
candidatesclient/app/hooks/useCandidateValidation.test.ts (1)
1-105: LGTM! Thorough test coverage for the validation hook.The test suite comprehensively covers all validation scenarios including:
- Minimum candidate count enforcement
- Duplicate name detection (case-insensitive)
- Empty field detection
- Multiple simultaneous errors
- Both boundary conditions (exactly 2 and more than 2 candidates)
client/app/create/components/CandidateCounter.tsx (1)
1-28: LGTM! Clean and focused counter component.The component provides clear visual feedback with appropriate color coding (amber for below minimum, green for success). Implementation is straightforward and effective.
client/app/create/components/CandidateSection.tsx (2)
33-50: LGTM! Well-structured header with responsive layout and smooth animations.The header effectively combines the counter, title, and add button with appropriate responsive behavior (flex-col to flex-row) and smooth hover/tap animations.
53-62: LGTM! Clear warning feedback for users.The warning banner appropriately appears when candidates are below the minimum threshold, with good visual design using amber colors and an icon.
client/app/create/page.tsx (1)
178-186: CandidateSection integration looks correct.The component receives all necessary props for candidate management: candidates array, visible validation errors, and callbacks for add/remove/update/blur operations.
client/app/helpers/candidateValidation.ts (4)
6-9: Clean type definition for Candidate.The interface is minimal and appropriate for the validation use case.
16-22: Validation messages align with PR requirements.The messages match the acceptance criteria from issue #157: minimum 2 candidates, no empty fields, and unique names.
28-48: Duplicate detection correctly handles case-insensitivity and empty names.The logic properly:
- Normalizes names with
trim().toLowerCase()- Skips empty names (line 34 check)
- Marks all indices involved in duplicates, not just the second occurrence
54-75: Empty field detection is correct and comprehensive.The function properly checks both
nameanddescriptionfields usingtrim()to handle whitespace-only inputs.
- Add unique ID to Candidate interface with generateCandidateId() - Change validation to use candidate IDs instead of array indices - Fix touchedFields to use ID-based keys (no index shifting needed) - Update CandidateSection to use candidate.id as React key - Update vitest from 1.2.0 to 1.6.1 to fix CSWSH RCE vulnerability - Update all tests to use ID-based approach (20 tests pass)
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: 0
🧹 Nitpick comments (6)
client/app/helpers/candidateValidation.ts (1)
15-17: Replace deprecatedsubstrwithsubstring.
String.prototype.substris deprecated. Usesubstringinstead for the same behavior.export function generateCandidateId(): string { - return `candidate-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`; + return `candidate-${Date.now()}-${Math.random().toString(36).substring(2, 11)}`; }client/app/hooks/useCandidateValidation.test.ts (1)
7-19: Consider consolidating duplicate test scenarios.The test at lines 7-19 and the test at lines 81-91 both verify that two valid candidates result in
isValid: trueand empty error messages. Consider removing one or differentiating them more clearly (e.g., one could focus on boundary condition documentation while the other tests additional assertions).Also applies to: 81-91
client/app/create/components/CandidateCard.tsx (2)
67-77: Add accessible labels for screen readers.The inputs rely only on
placeholderfor context, which is not accessible. Screen readers may not announce placeholders. Consider addingaria-labelor visually-hidden labels for better accessibility.<input type="text" value={candidate.name} onChange={(e) => onUpdate("name", e.target.value)} onBlur={() => onBlur("name")} placeholder="Candidate Name" + aria-label={`Candidate ${index + 1} name`} className={`${inputBaseClasses} ${ hasNameError ? inputErrorClasses : inputNormalClasses }`} aria-invalid={hasNameError} />
85-95: Add aria-label to description textarea.Same accessibility concern applies to the description field.
<textarea value={candidate.description} onChange={(e) => onUpdate("description", e.target.value)} onBlur={() => onBlur("description")} placeholder="Candidate Description" + aria-label={`Candidate ${index + 1} description`} rows={2} className={`${inputBaseClasses} ${ hasDescriptionError ? inputErrorClasses : inputNormalClasses }`} aria-invalid={hasDescriptionError} />client/app/create/page.tsx (1)
104-108: Consider showing all validation errors or a count.Currently only the first error message is shown on submit failure. If a user has multiple issues (e.g., fewer than 2 candidates AND empty fields), they'll only see one error and may need multiple submission attempts to discover all problems.
// Use validation hook results if (!validation.isValid) { - toast.error(validation.errorMessages[0]); + toast.error(validation.errorMessages.join(". ")); return; }Alternatively, you could show a count like "3 validation errors" with details in the UI.
client/app/create/components/CandidateSection.tsx (1)
52-62: Consider adding ARIA attributes for screen reader users.The warning banner appears dynamically but lacks ARIA announcements. Users relying on screen readers may not be notified when the validation state changes.
Apply this diff to improve accessibility:
<motion.div initial={{ opacity: 0, y: -10 }} animate={{ opacity: 1, y: 0 }} - className="flex items-center gap-2 text-amber-700 bg-amber-50 px-3 py-2 rounded-md" + className="flex items-center gap-2 text-amber-700 bg-amber-50 px-3 py-2 rounded-md" + role="alert" + aria-live="polite" > <ExclamationTriangleIcon className="h-5 w-5" /> <span className="text-sm">Add at least {MIN_CANDIDATES} candidates to create an election</span> </motion.div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
client/app/create/components/CandidateCard.tsx(1 hunks)client/app/create/components/CandidateSection.tsx(1 hunks)client/app/create/page.tsx(5 hunks)client/app/helpers/candidateValidation.test.ts(1 hunks)client/app/helpers/candidateValidation.ts(1 hunks)client/app/hooks/useCandidateValidation.test.ts(1 hunks)client/app/hooks/useCandidateValidation.ts(1 hunks)client/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- client/app/helpers/candidateValidation.test.ts
- client/app/hooks/useCandidateValidation.ts
🧰 Additional context used
🧬 Code graph analysis (4)
client/app/create/components/CandidateCard.tsx (1)
client/app/helpers/candidateValidation.ts (1)
Candidate(6-10)
client/app/create/components/CandidateSection.tsx (1)
client/app/helpers/candidateValidation.ts (3)
Candidate(6-10)CandidateValidationErrors(19-22)MIN_CANDIDATES(30-30)
client/app/create/page.tsx (4)
client/app/helpers/candidateValidation.ts (2)
Candidate(6-10)generateCandidateId(15-17)client/app/hooks/useCandidateValidation.ts (1)
useCandidateValidation(22-54)client/app/constants.ts (1)
ELECTION_FACTORY_ADDRESS(3-4)client/abi/artifacts/ElectionFactory.ts (1)
ElectionFactory(1-322)
client/app/hooks/useCandidateValidation.test.ts (2)
client/app/helpers/candidateValidation.ts (2)
Candidate(6-10)VALIDATION_MESSAGES(24-28)client/app/hooks/useCandidateValidation.ts (1)
useCandidateValidation(22-54)
🔇 Additional comments (12)
client/package.json (1)
9-11: LGTM! Test infrastructure properly configured.The test scripts and dependencies are correctly added. Notably, vitest is now at ^1.6.1, which addresses the critical CSWSH vulnerability (CVE-2025-24964) that affected versions ≤1.6.0 flagged in the previous review.
Also applies to: 40-40, 48-48, 53-55
client/app/helpers/candidateValidation.ts (2)
36-56: LGTM! Duplicate detection logic is correct.The case-insensitive comparison with trimming, and the approach of collecting all IDs involved in duplicates (not just the second occurrence), is the right behavior for highlighting all conflicting candidates.
62-83: LGTM! Empty field detection is well-implemented.The function correctly identifies empty fields per candidate using trim checks and returns a properly structured Map for per-field validation display.
client/app/hooks/useCandidateValidation.test.ts (1)
1-105: LGTM! Good test coverage for validation scenarios.The test suite covers essential validation paths: minimum candidates, empty fields, duplicate names (case-insensitive), and error message aggregation. The use of
renderHookis appropriate for testing the hook's memoized validation logic.client/app/create/components/CandidateCard.tsx (1)
40-62: LGTM! Animation and interaction implementation.The Framer Motion configuration with 300ms transitions,
willChangeoptimization hint, and accessible touch targets (44px minimum) align well with the PR objectives for smooth animations and mobile responsiveness.client/app/create/page.tsx (3)
28-29: LGTM! ID-based candidate management resolves previous index mismatch issue.The refactor from index-based to ID-based candidate tracking correctly addresses the touched-fields cleanup bug noted in the previous review. Now when a candidate is removed, the
touchedFieldsMap correctly uses the candidate'sidas the key, so no re-indexing is needed.Also applies to: 42-50
74-96: LGTM! Touched-field filtering logic is correct.The
getVisibleValidationErrorsfunction properly filters empty-field errors to only show for touched fields while always showing duplicate IDs (providing immediate feedback for duplicates regardless of blur state). This is good UX design.
130-134: LGTM! Contract payload mapping is correct.The candidate mapping correctly uses the array index for
candidateID(as expected by the smart contract'sElection.Candidate[]struct) while preserving the internalidfor React state management.client/app/create/components/CandidateSection.tsx (4)
1-7: LGTM! Clean and organized imports.All imports are properly structured and used throughout the component. The "use client" directive is correctly placed for Next.js client component usage.
9-16: LGTM! Interface correctly implements id-based callbacks.The props interface properly uses
id: stringfor all callbacks (onRemoveCandidate,onUpdateCandidate,onFieldBlur), which supports stable identity-based operations instead of index-based ones.
70-86: Excellent fix! The critical index-as-key issue is resolved.Line 74 now correctly uses
key={candidate.id}instead of the previouskey={candidate-${index}}. This ensures React properly tracks component identity during add/remove operations, preventing validation state bugs, animation glitches, and incorrect component reuse.The callbacks on lines 79-81 properly capture
candidate.idfor all operations, andindexis only used as a display prop (e.g., "Candidate #1"), which is the correct approach.
22-89: LGTM! Well-structured component with good UX patterns.The component implementation demonstrates solid practices:
- Derived state calculation (
isBelowMinimum) avoids prop drilling- Proper touch target sizing (
min-h-[44px]) on line 43 ensures accessibility- Conditional rendering elegantly handles empty states and warnings
- AnimatePresence configuration (
mode="popLayout",initial={false}) provides smooth list animations without layout thrashing
PR: Add Dynamic Candidate Management to Election Creation Form
Closes #157
Summary
This PR implements real-time candidate management during election creation, allowing users to add, edit, and remove candidates directly in the form with comprehensive validation and improved UX.
Changes
New Components
graph TD A[CreatePage] --> B[CandidateSection] B --> C[CandidateCounter] B --> D[CandidateCard] A --> E[useCandidateValidation Hook] E --> F[candidateValidation Utils] style A fill:#e1f5fe style B fill:#fff3e0 style C fill:#fff3e0 style D fill:#fff3e0 style E fill:#f3e5f5 style F fill:#f3e5f5Validation Flow
sequenceDiagram participant User participant Form participant Hook as useCandidateValidation participant Toast User->>Form: Type in field Form->>Hook: Validate candidates Hook-->>Form: Return validation state User->>Form: Blur from field Form->>Form: Mark field as touched Form->>Form: Show error if empty User->>Form: Click Submit alt Validation fails Form->>Toast: Show first error message Form->>Form: Block submission else Validation passes Form->>Form: Submit to blockchain endFeatures Implemented
Files Changed
New Files
app/helpers/candidateValidation.ts- Validation constants and utility functionsapp/hooks/useCandidateValidation.ts- React hook for candidate validationapp/create/components/CandidateCounter.tsx- Visual counter componentapp/create/components/CandidateCard.tsx- Individual candidate entry componentapp/create/components/CandidateSection.tsx- Main candidate management UIapp/helpers/candidateValidation.test.ts- Unit tests for validation utilitiesapp/hooks/useCandidateValidation.test.ts- Unit tests for validation hookvitest.config.ts- Test configurationModified Files
app/create/page.tsx- Integrated new components and validation hookpackage.json- Added test scripts and dependencies (vitest, @testing-library/react)Validation Rules
Testing
npm run testAll 18 tests pass:
candidateValidationutilitiesuseCandidateValidationhookScreenshots
Candidate Counter States
Error States
How to Test
/createChecklist
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.