Skip to content

refactor: extract NotificationBadge from NavBar with unit tests - #125

Open
devin-ai-integration[bot] wants to merge 1 commit into
developfrom
devin/1779230086-extract-notification-badge
Open

devin-ai-integration[bot] wants to merge 1 commit into
developfrom
devin/1779230086-extract-notification-badge

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented May 19, 2026

Copy link
Copy Markdown

Summary

Extracts the notification badge UI from NavBar.tsx into a standalone NotificationBadge component and adds comprehensive unit tests.

Changes:

  • src/components/NotificationBadge.tsx — New component accepting a notificationCount prop. Hides the badge when count is 0, displays the count otherwise. Preserves all existing data-test attributes and routing behavior.
  • src/components/NavBar.tsx — Simplified by delegating badge rendering to NotificationBadge. Removed unused Badge and NotificationsIcon imports and the customBadge style class (now owned by NotificationBadge).
  • src/components/__tests__/NotificationBadge.test.tsx — 7 unit tests covering:
    • Badge hidden when count is 0
    • Correct count displayed for 1, 5, and 99 notifications
    • Notifications icon renders
    • Links to /notifications
    • Correct data-test attributes present

Review & Testing Checklist for Human

  • Verify the notification badge still appears in the navbar with the correct count when logged in and notifications exist
  • Verify the badge is hidden when there are zero notifications
  • Click the notification icon and confirm it navigates to /notifications

Notes

All existing vitest tests continue to pass (9 passed, 1 skipped). No lint errors introduced.

Link to Devin session: https://app.devin.ai/sessions/165edb2d6c61433a9ad4bb0d1117d6e3
Requested by: @VedantKh


Devin Review

Status Commit
⚪ Not started

Run Devin Review

💡 Connect your GitHub account to enable automatic code reviews.

Open in Devin Review (Staging)

- Extract badge logic into standalone NotificationBadge component
- Accept notificationCount prop (hides badge at 0, shows count otherwise)
- Add 7 unit tests covering zero/single/multiple/large counts, icon, link, and data-test attributes
- Update NavBar to use the new component

Co-Authored-By: Vedant Khanna <vedantkhanna@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Author

Test Results: NotificationBadge Extraction

Ran the app locally (yarn dev) on localhost:3000, logged in as Heath93, and tested the NotificationBadge end-to-end.

UI Tests (all passed)
  • It should display notification count in the NavBar badge — passed
  • It should navigate to /notifications when badge is clicked — passed
  • It should update badge count when notifications are dismissed — passed (8→7→...→0)
  • It should hide badge when all notifications are dismissed — passed
With Notifications (count=8) After Dismiss (count=7) Zero Notifications (badge hidden)
Badge showing 8 Badge showing 7 Badge hidden
Notifications Page (8 items) Zero Notifications Page
8 notifications No notifications
Unit Tests (7/7 passed)

All 7 vitest tests pass:

  • Badge hidden when count is 0
  • Correct count for 1, 5, and 99 notifications
  • Notifications icon renders
  • Links to /notifications
  • Correct data-test attributes

Devin session

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.

1 participant