Skip to content

Add in-product notification center - #3472

Draft
CORCTON wants to merge 14 commits into
goplus:devfrom
CORCTON:feat/in-product-notifications
Draft

CORCTON wants to merge 14 commits into
goplus:devfrom
CORCTON:feat/in-product-notifications

Conversation

@CORCTON

@CORCTON CORCTON commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • add a navbar notification center with a red-dot unread indicator, cursor pagination, notification details, and internal actions
  • return unread notifications before read notifications in one flat list without separate sections or pagination
  • derive hasUnread from the notification list response and refresh on sign-in and notification-center open without polling
  • keep notifications in place after marking them read and temporarily disable page loading while that update is pending
  • guard user switches, stale requests, and cursor duplicates after notifications move into the read group on subsequent fetches
  • add notification API coverage, accessibility metadata, and OpenAPI documentation
  • keep Copilot below the standard modal layer so notification dialogs remain modal

Backend: goplus/builder-backend#355

This PR provides reusable notification UI. Feedback replies for #1789 will become the first producer in a separate PR.

Testing

  • notification API and component tests: 13 passed
  • npm run type-check
  • npm run lint -- --no-fix

@CORCTON

CORCTON commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/review -claude

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: in-product notifications

Solid, well-structured feature. The stale-response guarding (requestSequence / unreadRequestSequence) and the isCurrentNotification() membership+timestamp check make the optimistic mark-as-read and user-switch races robust, and the tests cover them directly. Vue reactivity of the mutated array items is correct, teardown of the timer and visibilitychange listener is clean, and the OpenAPI contract matches the frontend types (field names, readAt nullability, optional actionPath, 1-based pageIndex).

Findings below are mostly low-severity; the unreadCount overwrite on "Load more" (P2) is the one worth confirming as intentional. No blocking issues.

Also verified as fine (no change needed): the const: true on the PATCH read body is valid OpenAPI 3.1; the unconditional interval timer is harmless for signed-out users since refreshUnreadCount early-returns; the poll + visibility overlap is deduped by the sequence guard; and notification.body/title are rendered with escaped {{ }} interpolation (no v-html).

Comment thread spx-gui/src/components/navbar/NavbarNotifications.vue Outdated
Comment thread spx-gui/src/components/navbar/NavbarNotifications.vue
Comment thread spx-gui/src/components/navbar/NavbarNotifications.vue Outdated
Comment thread spx-gui/src/components/navbar/NavbarNotifications.vue

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: in-product notifications (follow-up)

One additional low-severity note that couldn't be attached to the previous review.

Comment thread docs/openapi.yaml

This branch was successfully deployed

1 active deployment
Preview – builder b00e75bf Deployed Sep 1, 2026 by vercel[bot]
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