Skip to content

feat: display "since QX YYYY" for ongoing projects without end date#5

Merged
jhb-dev merged 1 commit into
mainfrom
claude/issue-4-20250804-1645
Oct 26, 2025
Merged

feat: display "since QX YYYY" for ongoing projects without end date#5
jhb-dev merged 1 commit into
mainfrom
claude/issue-4-20250804-1645

Conversation

@jhb-dev
Copy link
Copy Markdown
Contributor

@jhb-dev jhb-dev commented Aug 4, 2025

  • Add "'since" label to CMS labels global for translation support
  • Update ProjectDate component to show "since QX YYYY" when no end date exists
  • Preserve existing behavior for projects with end dates

Closes #4

- Add ''since'' label to CMS labels global for translation support
- Update ProjectDate component to show ''since QX YYYY'' when no end date exists
- Preserve existing behavior for projects with end dates

Closes #4

Co-authored-by: Jens Becker <jhb-dev@users.noreply.github.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2025 4:53pm
website-cms ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 4, 2025 4:53pm

@claude
Copy link
Copy Markdown

claude Bot commented Aug 4, 2025

Claude finished @jhb-dev's task —— View job


Pull Request Review

This PR implements displaying "since QX YYYY" for ongoing projects without end dates. Overall, the implementation is clean and follows the repository's established patterns well.

Code Quality & Best Practices

Strong Points:

  • Translation-first approach: Properly adds the "since" label to the CMS labels global in cms/src/globals/Labels.ts:41, following the repository's multi-language requirements
  • Consistent with existing patterns: Uses the same labelField('since') pattern as other global labels
  • Proper label access: Correctly accesses labels via labels.global.since in the component
  • Clean conditional logic: The ternary operator clearly separates the two display modes (with/without end date)
  • Preserves existing behavior: Projects with end dates continue to show the range format

Architecture Compliance:

  • ✅ Follows CLAUDE.md translation requirements (no hardcoded strings)
  • ✅ Uses established component patterns and global state access
  • ✅ Maintains existing TypeScript typing

⚠️ Potential Issues

1. Label Validation & Error Handling

web/src/components/ProjectDate.astro:30

<span>{labels.global.since} {dateStringToQuarter(startDate)}</span>

Issue: No null safety check for labels.global.since. If the label is missing from CMS data, this could cause runtime errors.

Recommendation: Add defensive programming:

<span>{labels.global.since || 'since'} {dateStringToQuarter(startDate)}</span>

2. Date Parsing Edge Cases

The dateStringToQuarter utility (web/src/utils/dateToQuarter.ts:6-7) uses new Date(date) without validation:

const dateObj = new Date(date)
return `Q${Math.ceil((dateObj.getMonth() + 1) / 3)}/${dateObj.getFullYear()}`

Issue: Invalid date strings will produce "Invalid Date" and could result in "QNaN/NaN" output.

Recommendation: Add date validation in the utility or component.

🚀 Performance Considerations

  • Excellent: No performance concerns. The changes are purely presentational and don't add computational overhead
  • Good: Conditional rendering prevents unnecessary DOM elements

🔒 Security Assessment

  • No security concerns: The changes only affect text display and don't introduce any security vulnerabilities
  • Safe: Uses typed CMS data and doesn't expose user input directly

🧪 Test Coverage

Current State: No test files found in the repository for this component.

Recommendations for Future Testing:

  1. Unit tests for edge cases:

    • Projects with missing start/end dates
    • Invalid date formats
    • Missing label translations
  2. Visual regression tests:

    • Both display modes (with/without end date)
    • Multi-language label rendering

📋 Minor Suggestions

Accessibility Enhancement

Consider adding semantic meaning to the date information:

<p class:list={[className, 'text-slate-500']}>
  <time datetime={startDate}>
    {/* existing content */}
  </time>
</p>

Code Documentation

The component could benefit from a brief JSDoc comment explaining the two display modes.

📝 Files Reviewed

  • cms/src/globals/Labels.ts - ✅ Properly adds translation label
  • web/src/components/ProjectDate.astro - ✅ Clean implementation with minor robustness concerns

🎯 Overall Assessment

Verdict: ✅ Approve with minor suggestions

This is a well-implemented feature that follows the codebase conventions and properly handles internationalization. The main suggestions are defensive programming improvements rather than functional issues.

Key Strengths:

  • Follows established translation patterns
  • Clean, readable code
  • Preserves existing functionality
  • Good separation of concerns

Action Items (optional improvements):

  • Add null safety for label access
  • Consider date validation
  • Add semantic HTML for accessibility

@jhb-dev jhb-dev merged commit 5eaf738 into main Oct 26, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: display "since QX YYYY" for ongoing projects without an end date

1 participant