Skip to content

fixed all issues - #363

Closed
Benalex8797 wants to merge 3 commits into
Goldii-locks:mainfrom
Benalex8797:feat/315-325-326-329
Closed

fixed all issues#363
Benalex8797 wants to merge 3 commits into
Goldii-locks:mainfrom
Benalex8797:feat/315-325-326-329

Conversation

@Benalex8797

Copy link
Copy Markdown

closes #315
closes #325
closes #326
closes #329

SUMMARY:

New files:

NotificationContext.tsx
— NotificationProvider + useNotifications hook with addNotification, markAllRead, clearAll, and unreadCount
NotificationBell.tsx
— bell button with animated unread badge, ring-on-new-notification, press-on-click, and a slide-in dropdown panel showing notification items with type icons
notification-bell.test.tsx
— 24 tests covering context state, badge rendering/capping, aria attributes, panel open/close, animation classes, and keyboard interactions
Modified files:

globals.css
— 4 new keyframes: bell-ring (pivot swing), badge-pop (spring scale), bell-press (scale bounce), panel-slide (fade+translate), each exposed as @theme inline variables and utility classes
layout.tsx
— NotificationProvider wraps the tree
Navbar.tsx
— placed in the controls row alongside

@drips-wave

drips-wave Bot commented Aug 29, 2026

Copy link
Copy Markdown

@Benalex8797 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@godamongstmen897

Copy link
Copy Markdown
Contributor

Thanks @Benalex8797 — flagging this rather than merging. It merges into main without conflicts, but __tests__/dark-mode-switcher.test.tsx tests a DarkModeSwitcher that isn't in the branch: 15 of its 20 cases fail.

The test drives a self-contained, context-reading switcher:

renderWithTheme(<DarkModeSwitcher />);            // no props
expect(screen.getByTestId("theme-icon")).toHaveTextContent("🌙");
expect(screen.getByTestId("dark-mode-switcher"))
  .toHaveAttribute("aria-pressed", "true");

The DarkModeSwitcher this branch ships is a controlled component. It takes isDarkMode / onToggle, and when isDarkMode is null or undefined it deliberately renders dark-mode-switcher-empty-state instead of the toggle — that's the empty-state behaviour from #333. So rendering it with no props gives the empty state, and dark-mode-switcher is never in the tree.

theme-icon also doesn't exist in any component, here or on main — the only occurrence is as a CSS class in app/globals.css. And nothing in the branch calls useTheme() except ThemeContext.tsx itself, so the switcher has no route to the context the test wraps it in.

Two smaller things I hit while checking, worth fixing in the same pass:

  1. ThemeContext fails lint and blocks CI. Seeding state from a mount effect trips react-hooks/set-state-in-effect (app/context/ThemeContext.tsx:27). Because the value's real home is localStorage/matchMedia, useSyncExternalStore is the fix rather than a useEffect + setState — with getServerSnapshot returning "dark" it stays SSR-safe, and reading fresh (no module cache) keeps tests isolated, since they seed localStorage before each render.

  2. vi.spyOn(window, "matchMedia") throws — jsdom doesn't implement matchMedia and vitest.setup.ts doesn't shim it, so those two cases error before asserting anything. A small stub in the setup file fixes it for good.

The rest of the branch looks soundNotificationContext, the globals.css additions, layout.tsx and notification-bell-layout.test.tsx are all fine and pass.

Suggested path: decide which switcher you want. Either add the context-aware variant the test expects (a thin wrapper reading useTheme() and passing isDarkMode/onToggle down would do it, leaving the controlled component and its empty state intact), or rewrite the test to drive the controlled component with explicit props. Then fix the two items above and CI should go green.

@Benalex8797 Benalex8797 closed this by deleting the head repository Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants