Feed and notifs improvements (#498)

* Reduce frequency of the notifications sync

* Reduce frequency of home feed polling

* Ensure loading spinner is visible in notifications

* Render notifications loading spinner in the flatlist

* Fixes and performance improvements to notifications

* Render 30+ on notifications if at max

* Fix issue with repeating posts in home feed

* Dont check for unread notifs if we're already at max
zio/stable
Paul Frazee 2023-04-19 20:11:10 -05:00 committed by GitHub
parent b24ba3adc9
commit 04e0ebe8fc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 130 additions and 150 deletions

View File

@ -21,7 +21,7 @@ const MS_2DAY = MS_1HR * 48
let _idCounter = 0 let _idCounter = 0
type CondFn = (notif: ListNotifications.Notification) => boolean export const MAX_VISIBLE_NOTIFS = 30
export interface GroupedNotification extends ListNotifications.Notification { export interface GroupedNotification extends ListNotifications.Notification {
additional?: ListNotifications.Notification[] additional?: ListNotifications.Notification[]
@ -220,6 +220,7 @@ export class NotificationsFeedModel {
loadMoreError = '' loadMoreError = ''
hasMore = true hasMore = true
loadMoreCursor?: string loadMoreCursor?: string
lastSync?: Date
// used to linearize async modifications to state // used to linearize async modifications to state
lock = new AwaitLock() lock = new AwaitLock()
@ -259,6 +260,17 @@ export class NotificationsFeedModel {
return this.queuedNotifications && this.queuedNotifications?.length > 0 return this.queuedNotifications && this.queuedNotifications?.length > 0
} }
get unreadCountLabel(): string {
const count = this.unreadCount + this.rootStore.invitedUsers.numNotifs
if (count >= MAX_VISIBLE_NOTIFS) {
return `${MAX_VISIBLE_NOTIFS}+`
}
if (count === 0) {
return ''
}
return String(count)
}
// public api // public api
// = // =
@ -288,10 +300,13 @@ export class NotificationsFeedModel {
try { try {
this._xLoading(isRefreshing) this._xLoading(isRefreshing)
try { try {
const res = await this._fetchUntil(notif => notif.isRead, { const res = await this.rootStore.agent.listNotifications({
breakAt: 'page', limit: PAGE_SIZE,
}) })
await this._replaceAll(res) await this._replaceAll(res)
runInAction(() => {
this.lastSync = new Date()
})
this._setQueued(undefined) this._setQueued(undefined)
this._countUnread() this._countUnread()
this._xIdle() this._xIdle()
@ -313,20 +328,33 @@ export class NotificationsFeedModel {
/** /**
* Sync the next set of notifications to show * Sync the next set of notifications to show
* returns true if the number changed
*/ */
syncQueue = bundleAsync(async () => { syncQueue = bundleAsync(async () => {
this.rootStore.log.debug('NotificationsModel:syncQueue') this.rootStore.log.debug('NotificationsModel:syncQueue')
if (this.unreadCount >= MAX_VISIBLE_NOTIFS) {
return // no need to check
}
await this.lock.acquireAsync() await this.lock.acquireAsync()
try { try {
const res = await this._fetchUntil( const res = await this.rootStore.agent.listNotifications({
notif => limit: PAGE_SIZE,
this.notifications.length })
? isEq(notif, this.notifications[0])
: notif.isRead, const queue = []
{breakAt: 'record'}, for (const notif of res.data.notifications) {
) if (this.notifications.length) {
this._setQueued(res.data.notifications) if (isEq(notif, this.notifications[0])) {
break
}
} else {
if (!notif.isRead) {
break
}
}
queue.push(notif)
}
this._setQueued(this._filterNotifications(queue))
this._countUnread() this._countUnread()
} catch (e) { } catch (e) {
this.rootStore.log.error('NotificationsModel:syncQueue failed', {e}) this.rootStore.log.error('NotificationsModel:syncQueue failed', {e})
@ -335,37 +363,6 @@ export class NotificationsFeedModel {
} }
}) })
/**
*
*/
processQueue = bundleAsync(async () => {
this.rootStore.log.debug('NotificationsModel:processQueue')
if (!this.queuedNotifications) {
return
}
this.isRefreshing = true
await this.lock.acquireAsync()
try {
runInAction(() => {
this.mostRecentNotificationUri = this.queuedNotifications?.[0].uri
})
const itemModels = await this._processNotifications(
this.queuedNotifications,
)
this._setQueued(undefined)
runInAction(() => {
this.notifications = itemModels.concat(this.notifications)
})
} catch (e) {
this.rootStore.log.error('NotificationsModel:processQueue failed', {e})
} finally {
runInAction(() => {
this.isRefreshing = false
})
this.lock.release()
}
})
/** /**
* Load more posts to the end of the notifications * Load more posts to the end of the notifications
*/ */
@ -423,22 +420,23 @@ export class NotificationsFeedModel {
/** /**
* Update read/unread state * Update read/unread state
*/ */
async markAllUnqueuedRead() { async markAllRead() {
try { try {
for (const notif of this.notifications) { for (const notif of this.notifications) {
notif.markGroupRead() notif.markGroupRead()
} }
this._countUnread() this._countUnread()
if (this.notifications[0]) { await this.rootStore.agent.updateSeenNotifications(
await this.rootStore.agent.updateSeenNotifications( this.lastSync ? this.lastSync.toISOString() : undefined,
this.notifications[0].indexedAt, )
)
}
} catch (e: any) { } catch (e: any) {
this.rootStore.log.warn('Failed to update notifications read state', e) this.rootStore.log.warn('Failed to update notifications read state', e)
} }
} }
/**
* Used in background fetch to trigger notifications
*/
async getNewMostRecent(): Promise<NotificationsFeedItemModel | undefined> { async getNewMostRecent(): Promise<NotificationsFeedItemModel | undefined> {
let old = this.mostRecentNotificationUri let old = this.mostRecentNotificationUri
const res = await this.rootStore.agent.listNotifications({ const res = await this.rootStore.agent.listNotifications({
@ -486,40 +484,6 @@ export class NotificationsFeedModel {
// helper functions // helper functions
// = // =
async _fetchUntil(
condFn: CondFn,
{breakAt}: {breakAt: 'page' | 'record'},
): Promise<ListNotifications.Response> {
const accRes: ListNotifications.Response = {
success: true,
headers: {},
data: {cursor: undefined, notifications: []},
}
for (let i = 0; i <= 10; i++) {
const res = await this.rootStore.agent.listNotifications({
limit: PAGE_SIZE,
cursor: accRes.data.cursor,
})
accRes.data.cursor = res.data.cursor
let pageIsDone = false
for (const notif of res.data.notifications) {
if (condFn(notif)) {
if (breakAt === 'record') {
return accRes
} else {
pageIsDone = true
}
}
accRes.data.notifications.push(notif)
}
if (pageIsDone || res.data.notifications.length < PAGE_SIZE) {
return accRes
}
}
return accRes
}
async _replaceAll(res: ListNotifications.Response) { async _replaceAll(res: ListNotifications.Response) {
if (res.data.notifications[0]) { if (res.data.notifications[0]) {
this.mostRecentNotificationUri = res.data.notifications[0].uri this.mostRecentNotificationUri = res.data.notifications[0].uri
@ -540,17 +504,23 @@ export class NotificationsFeedModel {
}) })
} }
async _processNotifications( _filterNotifications(
items: ListNotifications.Notification[], items: ListNotifications.Notification[],
): Promise<NotificationsFeedItemModel[]> { ): ListNotifications.Notification[] {
const promises = [] return items.filter(item => {
const itemModels: NotificationsFeedItemModel[] = []
items = items.filter(item => {
return ( return (
this.rootStore.preferences.getLabelPreference(item.labels).pref !== this.rootStore.preferences.getLabelPreference(item.labels).pref !==
'hide' 'hide'
) )
}) })
}
async _processNotifications(
items: ListNotifications.Notification[],
): Promise<NotificationsFeedItemModel[]> {
const promises = []
const itemModels: NotificationsFeedItemModel[] = []
items = this._filterNotifications(items)
for (const item of groupNotifications(items)) { for (const item of groupNotifications(items)) {
const itemModel = new NotificationsFeedItemModel( const itemModel = new NotificationsFeedItemModel(
this.rootStore, this.rootStore,
@ -581,7 +551,7 @@ export class NotificationsFeedModel {
unread += notif.numUnreadInGroup unread += notif.numUnreadInGroup
} }
if (this.queuedNotifications) { if (this.queuedNotifications) {
unread += this.queuedNotifications.length unread += this.queuedNotifications.filter(notif => !notif.isRead).length
} }
this.unreadCount = unread this.unreadCount = unread
this.rootStore.emitUnreadNotifications(unread) this.rootStore.emitUnreadNotifications(unread)

View File

@ -237,7 +237,6 @@ export class PostsFeedModel {
// data // data
slices: PostsFeedSliceModel[] = [] slices: PostsFeedSliceModel[] = []
nextSlices: PostsFeedSliceModel[] = []
constructor( constructor(
public rootStore: RootStoreModel, public rootStore: RootStoreModel,
@ -309,7 +308,6 @@ export class PostsFeedModel {
this.loadMoreCursor = undefined this.loadMoreCursor = undefined
this.pollCursor = undefined this.pollCursor = undefined
this.slices = [] this.slices = []
this.nextSlices = []
this.tuner.reset() this.tuner.reset()
} }
@ -461,46 +459,33 @@ export class PostsFeedModel {
} }
const res = await this._getFeed({limit: PAGE_SIZE}) const res = await this._getFeed({limit: PAGE_SIZE})
const tuner = new FeedTuner() const tuner = new FeedTuner()
const nextSlices = tuner.tune(res.data.feed, this.feedTuners) const slices = tuner.tune(res.data.feed, this.feedTuners)
if (nextSlices[0]?.uri !== this.slices[0]?.uri) { if (slices[0]?.uri !== this.slices[0]?.uri) {
const nextSlicesModels = nextSlices.map( if (!autoPrepend) {
slice => this.setHasNewLatest(true)
new PostsFeedSliceModel( } else {
this.rootStore, this.setHasNewLatest(false)
`item-${_idCounter++}`,
slice,
),
)
if (autoPrepend) {
runInAction(() => { runInAction(() => {
this.slices = nextSlicesModels.concat( const slicesModels = slices.map(
slice =>
new PostsFeedSliceModel(
this.rootStore,
`item-${_idCounter++}`,
slice,
),
)
this.slices = slicesModels.concat(
this.slices.filter(slice1 => this.slices.filter(slice1 =>
nextSlicesModels.find(slice2 => slice1.uri === slice2.uri), slicesModels.find(slice2 => slice1.uri === slice2.uri),
), ),
) )
this.setHasNewLatest(false)
}) })
} else {
runInAction(() => {
this.nextSlices = nextSlicesModels
})
this.setHasNewLatest(true)
} }
} else { } else {
this.setHasNewLatest(false) this.setHasNewLatest(false)
} }
} }
/**
* Sets the current slices to the "next slices" loaded by checkForLatest
*/
resetToLatest() {
if (this.nextSlices.length) {
this.slices = this.nextSlices
}
this.setHasNewLatest(false)
}
/** /**
* Removes posts from the feed upon deletion. * Removes posts from the feed upon deletion.
*/ */

View File

@ -7,6 +7,7 @@ import {MyFollowsCache} from './cache/my-follows'
import {isObj, hasProp} from 'lib/type-guards' import {isObj, hasProp} from 'lib/type-guards'
const PROFILE_UPDATE_INTERVAL = 10 * 60 * 1e3 // 10min const PROFILE_UPDATE_INTERVAL = 10 * 60 * 1e3 // 10min
const NOTIFS_UPDATE_INTERVAL = 30 * 1e3 // 30sec
export class MeModel { export class MeModel {
did: string = '' did: string = ''
@ -21,6 +22,7 @@ export class MeModel {
follows: MyFollowsCache follows: MyFollowsCache
invites: ComAtprotoServerDefs.InviteCode[] = [] invites: ComAtprotoServerDefs.InviteCode[] = []
lastProfileStateUpdate = Date.now() lastProfileStateUpdate = Date.now()
lastNotifsUpdate = Date.now()
get invitesAvailable() { get invitesAvailable() {
return this.invites.filter(isInviteAvailable).length return this.invites.filter(isInviteAvailable).length
@ -119,7 +121,10 @@ export class MeModel {
await this.fetchProfile() await this.fetchProfile()
await this.fetchInviteCodes() await this.fetchInviteCodes()
} }
await this.notifications.syncQueue() if (Date.now() - this.lastNotifsUpdate > NOTIFS_UPDATE_INTERVAL) {
this.lastNotifsUpdate = Date.now()
await this.notifications.syncQueue()
}
} }
async fetchProfile() { async fetchProfile() {

View File

@ -14,6 +14,7 @@ import {usePalette} from 'lib/hooks/usePalette'
const EMPTY_FEED_ITEM = {_reactKey: '__empty__'} const EMPTY_FEED_ITEM = {_reactKey: '__empty__'}
const LOAD_MORE_ERROR_ITEM = {_reactKey: '__load_more_error__'} const LOAD_MORE_ERROR_ITEM = {_reactKey: '__load_more_error__'}
const LOADING_SPINNER = {_reactKey: '__loading_spinner__'}
export const Feed = observer(function Feed({ export const Feed = observer(function Feed({
view, view,
@ -27,28 +28,42 @@ export const Feed = observer(function Feed({
onScroll?: OnScrollCb onScroll?: OnScrollCb
}) { }) {
const pal = usePalette('default') const pal = usePalette('default')
const [isPTRing, setIsPTRing] = React.useState(false)
const data = React.useMemo(() => { const data = React.useMemo(() => {
let feedItems let feedItems: any[] = []
if (view.isRefreshing && !isPTRing) {
feedItems = [LOADING_SPINNER]
}
if (view.hasLoaded) { if (view.hasLoaded) {
if (view.isEmpty) { if (view.isEmpty) {
feedItems = [EMPTY_FEED_ITEM] feedItems = feedItems.concat([EMPTY_FEED_ITEM])
} else { } else {
feedItems = view.notifications feedItems = feedItems.concat(view.notifications)
} }
} }
if (view.loadMoreError) { if (view.loadMoreError) {
feedItems = (feedItems || []).concat([LOAD_MORE_ERROR_ITEM]) feedItems = (feedItems || []).concat([LOAD_MORE_ERROR_ITEM])
} }
return feedItems return feedItems
}, [view.hasLoaded, view.isEmpty, view.notifications, view.loadMoreError]) }, [
view.hasLoaded,
view.isEmpty,
view.notifications,
view.loadMoreError,
view.isRefreshing,
isPTRing,
])
const onRefresh = React.useCallback(async () => { const onRefresh = React.useCallback(async () => {
try { try {
setIsPTRing(true)
await view.refresh() await view.refresh()
} catch (err) { } catch (err) {
view.rootStore.log.error('Failed to refresh notifications feed', err) view.rootStore.log.error('Failed to refresh notifications feed', err)
} finally {
setIsPTRing(false)
} }
}, [view]) }, [view, setIsPTRing])
const onEndReached = React.useCallback(async () => { const onEndReached = React.useCallback(async () => {
try { try {
@ -83,6 +98,12 @@ export const Feed = observer(function Feed({
onPress={onPressRetryLoadMore} onPress={onPressRetryLoadMore}
/> />
) )
} else if (item === LOADING_SPINNER) {
return (
<View style={styles.loading}>
<ActivityIndicator size="small" />
</View>
)
} }
return <FeedItem item={item} /> return <FeedItem item={item} />
}, },
@ -104,7 +125,9 @@ export const Feed = observer(function Feed({
return ( return (
<View style={s.hContentRegion}> <View style={s.hContentRegion}>
<CenteredView> <CenteredView>
{view.isLoading && !data && <NotificationFeedLoadingPlaceholder />} {view.isLoading && !data.length && (
<NotificationFeedLoadingPlaceholder />
)}
{view.hasError && ( {view.hasError && (
<ErrorMessage <ErrorMessage
message={view.error} message={view.error}
@ -112,7 +135,7 @@ export const Feed = observer(function Feed({
/> />
)} )}
</CenteredView> </CenteredView>
{data && ( {data.length && (
<FlatList <FlatList
ref={scrollElRef} ref={scrollElRef}
data={data} data={data}
@ -121,7 +144,7 @@ export const Feed = observer(function Feed({
ListFooterComponent={FeedFooter} ListFooterComponent={FeedFooter}
refreshControl={ refreshControl={
<RefreshControl <RefreshControl
refreshing={view.isRefreshing} refreshing={isPTRing}
onRefresh={onRefresh} onRefresh={onRefresh}
tintColor={pal.colors.text} tintColor={pal.colors.text}
titleColor={pal.colors.text} titleColor={pal.colors.text}
@ -138,6 +161,9 @@ export const Feed = observer(function Feed({
}) })
const styles = StyleSheet.create({ const styles = StyleSheet.create({
loading: {
paddingVertical: 20,
},
feedFooter: {paddingTop: 20}, feedFooter: {paddingTop: 20},
emptyState: {paddingVertical: 40}, emptyState: {paddingVertical: 40},
}) })

View File

@ -20,6 +20,7 @@ import {ComposeIcon2} from 'lib/icons'
import {isDesktopWeb} from 'platform/detection' import {isDesktopWeb} from 'platform/detection'
const HEADER_OFFSET = isDesktopWeb ? 50 : 40 const HEADER_OFFSET = isDesktopWeb ? 50 : 40
const POLL_FREQ = 30e3 // 30sec
type Props = NativeStackScreenProps<HomeTabNavigatorParams, 'Home'> type Props = NativeStackScreenProps<HomeTabNavigatorParams, 'Home'>
export const HomeScreen = withAuthRequired((_opts: Props) => { export const HomeScreen = withAuthRequired((_opts: Props) => {
@ -150,7 +151,7 @@ const FeedPage = observer(
React.useCallback(() => { React.useCallback(() => {
const softResetSub = store.onScreenSoftReset(onSoftReset) const softResetSub = store.onScreenSoftReset(onSoftReset)
const feedCleanup = feed.registerListeners() const feedCleanup = feed.registerListeners()
const pollInterval = setInterval(doPoll, 15e3) const pollInterval = setInterval(doPoll, POLL_FREQ)
screen('Feed') screen('Feed')
store.log.debug('HomeScreen: Updating feed') store.log.debug('HomeScreen: Updating feed')
@ -176,8 +177,8 @@ const FeedPage = observer(
}, [feed]) }, [feed])
const onPressLoadLatest = React.useCallback(() => { const onPressLoadLatest = React.useCallback(() => {
feed.resetToLatest()
scrollToTop() scrollToTop()
feed.refresh()
}, [feed, scrollToTop]) }, [feed, scrollToTop])
return ( return (

View File

@ -38,8 +38,8 @@ export const NotificationsScreen = withAuthRequired(
}, [scrollElRef]) }, [scrollElRef])
const onPressLoadLatest = React.useCallback(() => { const onPressLoadLatest = React.useCallback(() => {
store.me.notifications.processQueue()
scrollToTop() scrollToTop()
store.me.notifications.refresh()
}, [store, scrollToTop]) }, [store, scrollToTop])
// on-visible setup // on-visible setup
@ -49,13 +49,12 @@ export const NotificationsScreen = withAuthRequired(
store.shell.setMinimalShellMode(false) store.shell.setMinimalShellMode(false)
store.log.debug('NotificationsScreen: Updating feed') store.log.debug('NotificationsScreen: Updating feed')
const softResetSub = store.onScreenSoftReset(onPressLoadLatest) const softResetSub = store.onScreenSoftReset(onPressLoadLatest)
store.me.notifications.syncQueue()
store.me.notifications.update() store.me.notifications.update()
screen('Notifications') screen('Notifications')
return () => { return () => {
softResetSub.remove() softResetSub.remove()
store.me.notifications.markAllUnqueuedRead() store.me.notifications.markAllRead()
} }
}, [store, screen, onPressLoadLatest]), }, [store, screen, onPressLoadLatest]),
) )

View File

@ -203,9 +203,7 @@ export const DrawerContent = observer(() => {
) )
} }
label="Notifications" label="Notifications"
count={ count={store.me.notifications.unreadCountLabel}
store.me.notifications.unreadCount + store.invitedUsers.numNotifs
}
bold={isAtNotifications} bold={isAtNotifications}
onPress={onPressNotifications} onPress={onPressNotifications}
/> />
@ -291,7 +289,7 @@ function MenuItem({
}: { }: {
icon: JSX.Element icon: JSX.Element
label: string label: string
count?: number count?: string
bold?: boolean bold?: boolean
onPress: () => void onPress: () => void
}) { }) {
@ -307,14 +305,14 @@ function MenuItem({
<View <View
style={[ style={[
styles.menuItemCount, styles.menuItemCount,
count > 99 count.length > 2
? styles.menuItemCountHundreds ? styles.menuItemCountHundreds
: count > 9 : count.length > 1
? styles.menuItemCountTens ? styles.menuItemCountTens
: undefined, : undefined,
]}> ]}>
<Text style={styles.menuItemCountLabel} numberOfLines={1}> <Text style={styles.menuItemCountLabel} numberOfLines={1}>
{count > 999 ? `${Math.round(count / 1000)}k` : count} {count}
</Text> </Text>
</View> </View>
) : undefined} ) : undefined}

View File

@ -132,9 +132,7 @@ export const BottomBar = observer(({navigation}: BottomTabBarProps) => {
) )
} }
onPress={onPressNotifications} onPress={onPressNotifications}
notificationCount={ notificationCount={store.me.notifications.unreadCountLabel}
store.me.notifications.unreadCount + store.invitedUsers.numNotifs
}
/> />
<Btn <Btn
testID="bottomBarProfileBtn" testID="bottomBarProfileBtn"
@ -170,7 +168,7 @@ function Btn({
}: { }: {
testID?: string testID?: string
icon: JSX.Element icon: JSX.Element
notificationCount?: number notificationCount?: string
onPress?: (event: GestureResponderEvent) => void onPress?: (event: GestureResponderEvent) => void
onLongPress?: (event: GestureResponderEvent) => void onLongPress?: (event: GestureResponderEvent) => void
}) { }) {

View File

@ -70,7 +70,7 @@ function BackBtn() {
} }
interface NavItemProps { interface NavItemProps {
count?: number count?: string
href: string href: string
icon: JSX.Element icon: JSX.Element
iconFilled: JSX.Element iconFilled: JSX.Element
@ -95,7 +95,7 @@ const NavItem = observer(
<Link href={href} style={styles.navItem}> <Link href={href} style={styles.navItem}>
<View style={[styles.navItemIconWrapper]}> <View style={[styles.navItemIconWrapper]}>
{isCurrent ? iconFilled : icon} {isCurrent ? iconFilled : icon}
{typeof count === 'number' && count > 0 && ( {typeof count === 'string' && count && (
<Text type="button" style={styles.navItemCount}> <Text type="button" style={styles.navItemCount}>
{count} {count}
</Text> </Text>
@ -162,9 +162,7 @@ export const DesktopLeftNav = observer(function DesktopLeftNav() {
/> />
<NavItem <NavItem
href="/notifications" href="/notifications"
count={ count={store.me.notifications.unreadCountLabel}
store.me.notifications.unreadCount + store.invitedUsers.numNotifs
}
icon={<BellIcon strokeWidth={2} size={24} style={pal.text} />} icon={<BellIcon strokeWidth={2} size={24} style={pal.text} />}
iconFilled={ iconFilled={
<BellIconSolid strokeWidth={1.5} size={24} style={pal.text} /> <BellIconSolid strokeWidth={1.5} size={24} style={pal.text} />