Skip to content

fix(activity): mark visible notifications as read - #380

Closed
ayebrian wants to merge 2 commits into
friendly-social:devfrom
ayebrian:fix/activity-read-on-view
Closed

ayebrian wants to merge 2 commits into
friendly-social:devfrom
ayebrian:fix/activity-read-on-view

Conversation

@ayebrian

Copy link
Copy Markdown

Summary

Mark Activity notifications as read when the user actually sees them, instead of requiring a click through to the corresponding post.

This fixes the case where opening Activity and reading the visible notifications leaves them highlighted in blue as though they had never been seen.

Based on dev at 99749cf (fix(community): read now happens before navigation). The upstream beforeClick callback and read-before-navigation ordering are preserved; this PR adds automatic viewport-based reading and makes the cached read state reflect successful API acknowledgements.

Before / after

Scenario Before (current dev) After
Open Activity with unread notifications Visible cards stay blue until individually clicked. A card triggers activityRead once at least 50% of it is visible in a visible document. Its blue unread background is removed after the request succeeds.
Scroll through the list Simply viewing more notifications does not mark them read. Newly visible cards are marked read as they cross the visibility threshold.
Virtualized overscan Cards can be mounted well outside the visible scroll area; marking every mounted card read would incorrectly acknowledge unseen items. IntersectionObserver checks actual intersection, so mounting an offscreen overscan card alone does not mark it read.
Activity in a hidden tab No automatic read-on-view behavior. Automatic reads are gated by document.visibilityState; returning to the tab checks the currently intersecting cards. Requests already started are not cancelled when the tab becomes hidden.
Click a notification Starts the read mutation before navigating, following the latest upstream fix. Preserves that ordering and skips cards already marked read or with a read request pending in the mounted card.
Read request fails The card is optimistically marked read before the API request, with no rollback on failure. The read flag changes only after API success. The mutation retries twice; after retries are exhausted, the card remains unread and can be retried by re-entering the viewport, returning to the tab, or clicking it.
List response races with a successful read setDetails replaces the cached details and can restore an older isRead: false. An incoming unread value cannot overwrite an already-confirmed read value in the detail cache.
Cache updates after successful reading The click path updates individual details, but does not explicitly update the infinite-list cache. Updates both the individual detail entry and matching items across all currently cached Activity pages.

Implementation

src/app/activity/page.tsx

  • Observe each unread card with IntersectionObserver({threshold: 0.5}).
  • Require both sufficient intersection and a visible document before starting an automatic read.
  • Listen for visibilitychange so cards visible when returning to the tab can be acknowledged.
  • Disconnect the observer and remove the document listener during effect cleanup.
  • Share one guarded read callback between viewport observation and the existing beforeClick path.
  • Use a synchronous per-card pending ref to avoid overlapping requests from those triggers while the card remains mounted.
  • Retry failed mutations twice without creating an effect-driven retry loop.

src/services/activity-service.ts

  • Add activity.markRead(app, id), which awaits a successful backend result before updating caches.
  • Use immutable functional cache updates so completion does not write a stale captured copy of the card.
  • Update matching entries in the infinite-list cache as well as the detail cache.
  • Preserve confirmed isRead: true values when populating details from a potentially stale list response. This relies on the existing one-way read operation; the current client has no mark-as-unread operation.

Reproduction / expected behavior

  1. Have multiple unread reply notifications, including enough entries to require scrolling.
  2. Open Activity without clicking a notification.
  3. Before: notifications already visible on screen remain blue.
  4. After: cards at least halfway visible lose their blue background after the read API succeeds.
  5. Scroll down: additional visible cards are acknowledged, while entries still outside the viewport remain unread.
  6. Return to Activity or reload: successfully acknowledged notifications remain read.
  7. With read requests forced to fail, confirm that affected cards remain unread after the bounded retries. Restore successful responses and scroll an affected card out of view and back in to retry it.

Verification

Static checks passed on the rebased changes:

  • pnpm run compile
  • pnpm exec eslint src/app/activity/page.tsx src/services/activity-service.ts
  • pnpm exec prettier --check src/app/activity/page.tsx src/services/activity-service.ts
  • git diff --check origin/dev...HEAD

Browser regression checks used the actual ActivityPage under React Strict Mode, with a temporary fixture and mocked backend responses:

  • 60 notifications; 41 cards mounted by virtualization/overscan, but only sufficiently visible unread cards acknowledged initially.
  • An already-read card did not issue another read request; a far-offscreen card remained unread.
  • Scrolling acknowledged newly visible cards.
  • Injected HTTP 500 responses produced three attempts (initial request plus two retries), left the card unread, and recovered after viewport re-entry with successful responses.
  • Reloading the fixture after successful acknowledgements did not issue duplicate read requests for those notifications.
  • Injecting stale unread details did not revert a confirmed read flag.
  • Simulated hidden-document visibility blocked automatic reads; making it visible acknowledged intersecting cards.
  • Clicking an unread notification still navigated, and the successful response updated the cache after the Activity component unmounted.
  • Reviewed rendered screenshots at desktop width 1440 and mobile width 360, including mobile light and dark themes.

The browser checks were performed locally against mocked responses, not a signed-in production backend. The temporary harness is not included in the PR. No backend API changes or new dependencies are required.

@y9san9

y9san9 commented Sep 18, 2026

Copy link
Copy Markdown
Member

I think we generally want this to behave like it behaves in email, where additional click is required. Otherwise, we could provide a different API on backend first, instead of spamming with read requests.

However, the conflict handling of running request and mark as read is an important thing that I will merge manually

@y9san9 y9san9 closed this Sep 18, 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

Development

Successfully merging this pull request may close these issues.

2 participants