More notifications improvements (#2198)

* On mobile, never replace the notifs under the user due to focus events

* Use the server's seenAt response to calculate isRead state locally

4 files changed, 27 insertions, 28 deletions

diff --git a/src/state/queries/notifications/feed.ts b/src/state/queries/notifications/feed.ts
index 1610fc0bf..82eda06ea 100644
--- a/src/state/queries/notifications/feed.ts
+++ b/src/state/queries/notifications/feed.ts
@@ -16,7 +16,7 @@
  * 3. Don't call this query's `refetch()` if you're trying to sync latest; call `checkUnread()` instead.
  */
 
-import {useEffect, useRef} from 'react'
+import {useEffect} from 'react'
 import {AppBskyFeedDefs} from '@atproto/api'
 import {
   useInfiniteQuery,
@@ -49,8 +49,6 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
   const threadMutes = useMutedThreads()
   const unreads = useUnreadNotificationsApi()
   const enabled = opts?.enabled !== false
-  // state tracked across page fetches
-  const pageState = useRef({pageNum: 0, hasMarkedRead: false})
 
   const query = useInfiniteQuery<
     FeedPage,
@@ -66,8 +64,6 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
       if (!pageParam) {
         // for the first page, we check the cached page held by the unread-checker first
         page = unreads.getCachedUnreadPage()
-        // reset the page state
-        pageState.current = {pageNum: 0, hasMarkedRead: false}
       }
       if (!page) {
         page = await fetchPage({
@@ -80,27 +76,9 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
         })
       }
 
-      // NOTE
-      // this section checks to see if we need to mark notifs read
-      // we want to wait until we've seen a read notification because
-      // of a timing challenge; marking read on the first page would
-      // cause subsequent pages of unread notifs to incorrectly come
-      // back as "read". we use page 6 as an abort condition, which means
-      // after ~180 notifs we give up on tracking unread state correctly
-      // -prf
-      if (!pageState.current.hasMarkedRead) {
-        let hasMarkedRead = false
-        if (
-          pageState.current.pageNum > 5 ||
-          page.items.some(item => item.notification.isRead)
-        ) {
-          unreads.markAllRead()
-          hasMarkedRead = true
-        }
-        pageState.current = {
-          pageNum: pageState.current.pageNum + 1,
-          hasMarkedRead,
-        }
+      // if the first page has an unread, mark all read
+      if (!pageParam && page.items[0] && !page.items[0].notification.isRead) {
+        unreads.markAllRead()
       }
 
       return page
@@ -108,6 +86,20 @@ export function useNotificationFeedQuery(opts?: {enabled?: boolean}) {
     initialPageParam: undefined,
     getNextPageParam: lastPage => lastPage.cursor,
     enabled,
+    select(data: InfiniteData<FeedPage>) {
+      // override 'isRead' using the first page's returned seenAt
+      // we do this because the `markAllRead()` call above will
+      // mark subsequent pages as read prematurely
+      const seenAt = data.pages[0]?.seenAt || new Date()
+      for (const page of data.pages) {
+        for (const item of page.items) {
+          item.notification.isRead =
+            seenAt > new Date(item.notification.indexedAt)
+        }
+      }
+
+      return data
+    },
   })
 
   useEffect(() => {
diff --git a/src/state/queries/notifications/types.ts b/src/state/queries/notifications/types.ts
index b52341115..86a9bb139 100644
--- a/src/state/queries/notifications/types.ts
+++ b/src/state/queries/notifications/types.ts
@@ -24,6 +24,7 @@ export interface FeedNotification {
 
 export interface FeedPage {
   cursor: string | undefined
+  seenAt: Date
   items: FeedNotification[]
 }
 
diff --git a/src/state/queries/notifications/util.ts b/src/state/queries/notifications/util.ts
index 9fb25867a..cc5943163 100644
--- a/src/state/queries/notifications/util.ts
+++ b/src/state/queries/notifications/util.ts
@@ -68,8 +68,14 @@ export async function fetchPage({
     notif => !isThreadMuted(notif, threadMutes),
   )
 
+  let seenAt = res.data.seenAt ? new Date(res.data.seenAt) : new Date()
+  if (Number.isNaN(seenAt.getTime())) {
+    seenAt = new Date()
+  }
+
   return {
     cursor: res.data.cursor,
+    seenAt,
     items: notifsGrouped,
   }
 }
diff --git a/src/view/screens/Notifications.tsx b/src/view/screens/Notifications.tsx
index fceaa60c2..7ebacc9db 100644
--- a/src/view/screens/Notifications.tsx
+++ b/src/view/screens/Notifications.tsx
@@ -67,8 +67,8 @@ export function NotificationsScreen({}: Props) {
   const onFocusCheckLatest = React.useCallback(() => {
     // on focus, check for latest, but only invalidate if the user
     // isnt scrolled down to avoid moving content underneath them
-    unreadApi.checkUnread({invalidate: !isScrolledDown})
-  }, [unreadApi, isScrolledDown])
+    unreadApi.checkUnread({invalidate: !isScrolledDown && isDesktop})
+  }, [unreadApi, isScrolledDown, isDesktop])
   checkLatestRef.current = onFocusCheckLatest
 
   // on-visible setup