Connected Navbar Notif Bell + Added Regression Coverage by tifpham021 · Pull Request #287 · GTBitsOfGood/servicestart

tifpham021 · 2026-09-27T00:28:09Z

Closes #191

Description

The notif bell in the navbar was not connected to the existing notification sidebar
On desktop, clicking the bell did nothing; on some layouts, it navigated to the full Inbox page instead
The unread count also needed to stay consistent after notifications were marked as read

Changes Made

Connected the navbar notification bell to the notification sidebar
Updated the bell to work on desktop, mobile, and vertical navbar layouts
Made the sidebar open without sending the user to another page
Made the sidebar close with the X button, Escape key, or an outside click
Added keyboard support and returned focus to the bell after closing
Kept the unread count on the bell and in the sidebar up to date
Fixed the unread badge on the vertical navbars
Added tests for the sidebar, notification filters, read actions, and user/org data

Video Showing Notif Bell is Connected to the Sidebar, Sidebar Filters Working, Counts Being Updated, and Closing of Sidebar

Screen.Recording.2026-09-26.at.8.18.16.PM.mov

Video Showing that it Works Across Diff Screens

Screen.Recording.2026-09-26.at.8.20.06.PM.mov

How to Test

Sign in as a user with notifications and click the navbar bell
Confirm that the sidebar opens without changing the URL
Test the All and Unread filters and mark a notification as read
Confirm that the sidebar and bell counts update together
Close the sidebar using the X button, Escape, and an outside click
Use Enter or Space to open the sidebar and confirm focus returns to the bell after closing
Check the bell in desktop, mobile, and vertical navbar layouts

Checklist:

I have mentioned a ticket above
My changes meet the acceptance criteria of said ticket
I have self-reviewed my changes
I have requested a review from the appropriate team member(s)
I have fixed all merge conflicts
I have ensured CI checks pass

netlify · 2026-09-27T00:28:15Z

✅ Deploy Preview for servicestart ready!

Name	Link
🔨 Latest commit	`af89426`
🔍 Latest deploy log	https://app.netlify.com/projects/servicestart/deploys/6abaf0902575040008391467
😎 Deploy Preview	https://deploy-preview-287--servicestart.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

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

greptile-apps · 2026-09-27T00:31:45Z

Disclaimer: This comment was AI-generated and is not necessarily completely accurate. Please take code comments with a grain of salt.

Confidence Score: 3/5

[Medium risk] Refactors notification UI to use a shared sidebar component.

The PR does not appear safe to merge while the full Inbox remains inaccessible from vertical navigation and the mobile drawer still navigates instead of opening the sidebar.

Findings

Full Inbox Access Lost ▶
Mobile Inbox Still Navigates ▶
Modal Title Styling ▶
Modal Description Misses Required Styling ▶

Summary

The PR connects notification controls across navbar layouts to the sidebar, synchronizes unread counts, adds request-race guards, and expands regression coverage. Review score: 60/100.

The new modal title styling needs to meet the repository requirement.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  Bell[Navbar notification control] --> Sidebar[Notifications sidebar]
  Sidebar --> Hook[useNotifications]
  Hook --> API[Notifications API]
  Hook --> Event[Unread-count event]
  Event --> Badge[Navbar unread badge]

_{Reviews (2) · Last reviewed commit: "fix notification request races"}

greptile-apps · 2026-09-27T00:31:48Z

+              if (isNotifications) {
+                return (
+                  <NotificationsSidebar
+                    key={item.href}
+                    trigger={
+                      <button
+                        type="button"
+                        aria-label="Notifications"
+                        className="w-full text-left"
+                      >
+                        {navItem}
+                      </button>
+                    }
+                  />


Full Inbox Access Lost On desktop with a vertical navbar, clicking Inbox now opens only the sidebar. The icon-only vertical navbar does the same. Neither sidebar offers a link to the full Inbox page, so admins using these layouts lose the normal navigation path to its “New +” notification-composition action.

greptile-apps · 2026-09-27T00:31:48Z

  const bellButton = (
-    <Link
-      href="/inbox"
-      aria-label="Notifications"
-      className="relative rounded-md p-2 text-grey-text-strong transition-colors hover:bg-brand-text/10"
-    >
-      <BogIcon name="bell" size={24} />
-      {unreadCount > 0 && (
-        <span className="absolute right-0 top-0 flex h-5 min-w-5 items-center justify-center rounded-full bg-status-red-text px-1 text-xs font-bold text-white">
-          {unreadCount > 99 ? "99+" : unreadCount}
-        </span>
-      )}
-    </Link>
+    <NotificationCounter
+      unreadCount={unreadCount}
+      iconSize={24}
+      className="rounded-md p-2 text-grey-text-strong transition-colors hover:bg-brand-text/10"
+      badgeClassName="right-0 top-0"
+    />
  );


Mobile Inbox Still Navigates The top-bar bell now opens the sidebar, but the Inbox item in the mobile drawer remains a link to /inbox. Selecting that notification item navigates away instead of opening the sidebar.

greptile-apps · 2026-09-27T00:31:48Z

+        <Dialog.Description className="sr-only">
+          View and manage your notifications.
+        </Dialog.Description>


Modal Description Misses Required Styling The new dialog description has only sr-only styling. The repository’s modal-style requirement says descriptions must use the paragraph-1 CSS variables and white text. This requirement needs to be satisfied before merging.

Rule Used: Modal titles should use the heading-1 CSS variables from src/styles/globals.css for font-size, font-family, and line-height (responsive for desktop and mobile). Modal descriptions should use the paragraph-1 CSS variables from the same file. Bot... (source)

Learned From
GTBitsOfGood/design-system#76

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

tifpham021 · 2026-09-27T01:08:33Z

@greptileai

greptile-apps · 2026-09-27T01:11:37Z

+        </Dialog.Description>
+        <div className="shrink-0 px-5 pt-6 mobile:px-8 mobile:pt-12">
+          <div className="mb-4 mobile:mb-8 flex items-center justify-between">
+            <Dialog.Title className="text-heading-3 text-grey-text-strong">


Modal Title Styling The sidebar title uses text-heading-3 and grey text. The repository requires modal titles to use responsive heading-1 CSS variables and white text. This requirement must be satisfied before merging.

Rule Used: Modal titles should use the heading-1 CSS variables from src/styles/globals.css for font-size, font-family, and line-height (responsive for desktop and mobile). Modal descriptions should use the paragraph-1 CSS variables from the same file. Bot... (source)

Learned From
GTBitsOfGood/design-system#76

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

The vertical navbars now open the sidebar from their Inbox item, which left no path to /inbox (and its New + action) in those layouts. Add a "View full inbox" link to the sidebar header that closes the panel and navigates to the inbox page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VeYmbN1aRFTdeZM8vU2cC3

shaply · 2026-09-28T16:24:57Z

Pushed 83788b0 to address Greptile's P1 "Full Inbox Access Lost": the vertical navbars' Inbox item now opens the sidebar, which left no way to reach /inbox (and its New + button) in those layouts. The sidebar header now has a View full inbox link that closes the panel and navigates to /inbox, with a unit test.

Left as-is:

"Mobile Inbox Still Navigates": the mobile top-bar bell opens the sidebar as Connect the navbar notification bell and add regression coverage #191 asks; the drawer's Inbox item stays a link to the full inbox page.
Modal typography (both P2s): that rule comes from the design-system repo's modals. White title/description text would be invisible on this white drawer, and the title styling is unchanged from main.

Generated by Claude Code

…-regression

connect notification bell to sidebar

2bb540a

tifpham021 had a problem deploying to Preview September 27, 2026 00:28 — with GitHub Actions Failure

greptile-apps Bot reviewed Sep 27, 2026

View reviewed changes

fix notification request races

86fa98e

tifpham021 deployed to Preview September 27, 2026 01:00 — with GitHub Actions Active

greptile-apps Bot reviewed Sep 27, 2026

View reviewed changes

tifpham021 requested a review from shaply September 27, 2026 01:15

shaply deployed to Preview September 28, 2026 15:53 — with GitHub Actions Active

Merge branch 'main' into tiffany/277-connecting-navbar-notif-bell-and…

af89426

…-regression

shaply deployed to Preview September 28, 2026 22:56 — with GitHub Actions Active

shaply approved these changes Sep 29, 2026

View reviewed changes

shaply merged commit de534e6 into main Sep 29, 2026
25 checks passed

shaply deployed to Preview September 29, 2026 00:58 — with GitHub Actions Active

Navigation Menu

Repository navigation

Connected Navbar Notif Bell + Added Regression Coverage - #287

Connected Navbar Notif Bell + Added Regression Coverage#287
shaply merged 4 commits into
mainfrom
tiffany/277-connecting-navbar-notif-bell-and-regression

tifpham021 commented Sep 27, 2026 •

edited

Loading

Uh oh!

netlify Bot commented Sep 27, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 27, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 27, 2026

Uh oh!

greptile-apps Bot Sep 27, 2026

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Sep 27, 2026

Uh oh!

tifpham021 commented Sep 27, 2026

Uh oh!

greptile-apps Bot Sep 27, 2026

Uh oh!

shaply commented Sep 28, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tifpham021 commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes Made

Video Showing Notif Bell is Connected to the Sidebar, Sidebar Filters Working, Counts Being Updated, and Closing of Sidebar

Video Showing that it Works Across Diff Screens

How to Test

Uh oh!

netlify Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for servicestart ready!

Uh oh!

greptile-apps Bot commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 3/5

Findings

Summary

Uh oh!

greptile-apps Bot Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

tifpham021 commented Sep 27, 2026

Uh oh!

greptile-apps Bot Sep 27, 2026

Choose a reason for hiding this comment

Uh oh!

shaply commented Sep 28, 2026

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tifpham021 commented Sep 27, 2026 •

edited

Loading

netlify Bot commented Sep 27, 2026 •

edited

Loading

greptile-apps Bot commented Sep 27, 2026 •

edited

Loading