Skip to content

Notifications Icon on Login - #334

Merged
ND68 merged 2 commits into
mainfrom
andy/328-notifications-icon-on-login
Apr 11, 2026
Merged

ND68 merged 2 commits into
mainfrom
andy/328-notifications-icon-on-login

Conversation

@idke64

@idke64 idke64 commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Description

Resolves ticket number: #328

Explain what your code changes:
Removes notification icon on login page and also deletes notifications from sessionStorage on logout.

List the steps you took to test your code:
Just made sure everything worked on localhost

Checklist

  • The ticket is mentioned above
  • The changes fulfill the success criteria of the ticket
  • The changes were self-reviewed
  • A review was requested

@netlify

netlify Bot commented Apr 11, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for hope-for-haiti ready!

Name Link
🔨 Latest commit abed52f
🔍 Latest deploy log https://app.netlify.com/projects/hope-for-haiti/deploys/69dab6362c41ea0008fa2f31
😎 Deploy Preview https://deploy-preview-334--hope-for-haiti.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@greptile-apps

greptile-apps Bot commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR hides the floating notification bell on the login/unauthenticated view by adding a if (!user) return null guard in FloatingNotification, and eagerly clears chat_notifications from sessionStorage before calling signOut() so the cached data is gone even if the sign-out causes an immediate full-page redirect (before the existing cleanup useEffect in NotificationHandler would otherwise fire).

Score: 95 / 100

Checklist items in good shape — no unused imports, no console.logs, hooks are called unconditionally before the early return. The only minor point is the unreadCount line being evaluated before the early-return guard, which is a trivial no-op style nit.

Confidence Score: 5/5

Safe to merge — changes are minimal, correct, and solve the stated ticket.

Both changes are straightforward and well-scoped: the early-return guard in FloatingNotification is Rules-of-Hooks compliant and directly fixes the UI bug; the sessionStorage cleanup before signOut() is a sound defensive measure given that a full-page redirect can race the existing useEffect cleanup. No P0/P1 findings; the single P2 comment is a trivial micro-optimisation.

No files require special attention.

Important Files Changed

Filename Overview
src/components/dashboard/FloatingNotification.tsx Adds useUser hook and an early if (!user) return null guard to hide the notification bell when unauthenticated; all hooks are called before the conditional return (Rules of Hooks compliant).
src/components/NavBarLayout.tsx Adds explicit sessionStorage.removeItem("chat_notifications") before signOut() in the logout handler to eagerly clear cached chat data before page redirect; minor whitespace/formatting cleanup.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User clicks logout] --> B[NavBarLayout onClick handler]
    B --> C[sessionStorage.removeItem called]
    C --> D[signOut called]
    D --> E[Page redirects to sign-in]

    F[FloatingNotification renders] --> G{user from useUser}
    G -- null --> H[return null — bell not shown]
    G -- authenticated --> I[Render bell icon]
Loading

Reviews (1): Last reviewed commit: "fixed noti icons and sessionStorage" | Re-trigger Greptile

Comment thread src/components/dashboard/FloatingNotification.tsx Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@ND68
ND68 merged commit 8720928 into main Apr 11, 2026
5 checks passed
@ND68
ND68 deleted the andy/328-notifications-icon-on-login branch April 11, 2026 21:03
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.

2 participants