Skip to content

fix(notifications): tell unread from read and list only pushable notification types - #246

Merged
andrebrait merged 3 commits into
kahme247:mainfrom
andrebrait:feat/notification-read-state
Oct 10, 2026
Merged

andrebrait merged 3 commits into
kahme247:mainfrom
andrebrait:feat/notification-read-state

Conversation

@andrebrait

Copy link
Copy Markdown
Collaborator

Follow-up to #230.

Problem

  1. Read and unread entries in the Notifications tab looked almost the same: unread rows had a 7px dot beside the time and a faint tint, and read rows used the same text styling.
  2. Every toast landed in the Notifications tab, including "Saved", copy results, update notices and agent notify calls, so the notifications the user needs to act on were buried.

Changes

  • Read state: unread rows show an accent dot in a leading slot and a bold --text title on the tinted row. Read rows have a --text-muted regular-weight title and no tint. The description keeps --text-muted: in several themes --text-dim has higher contrast than --text-muted, so it would not dim the text.
  • Only pushable types are listed: toast.* records into toastHistory only with the new history: true option. useNotifications sets it for session notifications (completed, input, error, model switch, the NOTIFICATION_TYPES that can also be pushed), including the toast fallback when a system notification cannot be shown. The notify host tool no longer records into the history.
  • A reused id is unread again: record used to keep the read state of a reused id so that update notices re-shown on every tab focus did not badge again. Update notices are no longer listed. Session notifications reuse <sessionId>:<type> as their id, so keeping the read state made a session's next notification of the same type arrive already read.

Overlap: #240 and #242 touch toastHistory.record. With this PR, #242's keepRead option is no longer needed, because the update notices are not recorded. Whichever PR lands second needs a small rebase.

Verification

  • toast.test.mjs: plain toasts stay out of the history, history: true toasts are listed, and a reused id is unread again.
  • toast.click.test.mjs: a delivered session notification is listed with its <sessionId>:<type> id and is unread. The test fails if history: true is removed from useNotifications.
  • NotificationList.test.mjs: only the unread row carries the unread marker.
  • npm test 1347 tests, 0 failures. Typecheck and eslint are clean.
  • Dev server: two notifications were marked read by leaving the tab, then a third arrived. The new notification showed the dot and a bold title, the read ones were dimmed, and an "OMP update available" toast did not appear in the list.

andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 10, 2026
…nly pushable notification types

Squashed from origin/feat/notification-read-state (9edf773) onto upstream/main.

Integration fixup: drops kahme247#242's keepRead (update notices are no longer recorded), keeps kahme247#240/kahme247#242's onClick on history entries, and adapts toast.click.test.mjs to the history: true opt-in.
Resolve the overlap with kahme247#242: drop its keepRead (update notices are no longer recorded), keep onClick on history entries, and opt the click tests into history: true.
@andrebrait
andrebrait merged commit 7a3f76c into kahme247:main Oct 10, 2026
3 checks passed
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