Performance improvements and ordering rework

CHANGELOG.md

@@ -7,8 +7,13 @@ You can also check [on GitHub](https://github.com/nextcloud/news/releases), the
 # Unreleased
 ## [25.x.x]
 ### Changed
+- Performance improvements on item list
+- Rework feed and global sort ordering
 
 ### Fixed
+- starred items in a feed can prevent further scrolling
+- j shortcut doesn't load more items in infinite scroll (#2847)
+- Feed ordering uses wrong values (#2846)
 
 # Releases
 ## [25.0.0-alpha12] - 2024-10-23

src/components/Sidebar.vue

@@ -282,8 +282,8 @@ export default Vue.extend({
 
 			},
 			set(newValue) {
+				this.$store.dispatch(ACTIONS.RESET_LAST_ITEM_LOADED)
 				this.saveSetting('oldestFirst', newValue)
-				this.$store.dispatch(ACTIONS.RESET_ITEMS)
 			},
 		},
 		preventReadOnScroll: {

src/components/feed-display/FeedItemDisplayList.vue

@@ -53,7 +53,7 @@
 				:fetch-key="fetchKey"
 				@load-more="fetchMore()">
 				<template v-if="items && items.length > 0">
-					<template v-for="item in filterSortedItems()">
+					<template v-for="item in filteredItemcache">
 						<FeedItemRow :key="item.id"
 							:ref="'feedItemRow' + item.id"
 							:item="item"
@@ -133,27 +133,16 @@ export default Vue.extend({
 
 			// Determine the sorting order
 			sort: (a: FeedItem, b: FeedItem) => {
-				let oldestFirst
-				switch (this.config.ordering) {
-				case FEED_ORDER.OLDEST:
-					oldestFirst = true
-					break
-				case FEED_ORDER.NEWEST:
-					oldestFirst = false
-					break
-				case FEED_ORDER.DEFAULT:
-				default:
-					oldestFirst = this.$store.getters.oldestFirst
-				}
-
 				if (a.id > b.id) {
-					return oldestFirst ? 1 : -1
+					return this.listOrdering ? 1 : -1
 				} else {
-					return oldestFirst ? -1 : 1
+					return this.listOrdering ? -1 : 1
 				}
 			},
 			cache: [] as FeedItem[] | undefined,
+			filteredItemcache: [] as FeedItem,
 			selectedItem: undefined as FeedItem | undefined,
+			debouncedClickItem: null,
 		}
 	},
 	computed: {
@@ -166,22 +155,90 @@ export default Vue.extend({
 		getSelectedItem() {
 			return this.$store.getters.selected
 		},
+		changedFeedOrdering() {
+			if (this.fetchKey.startsWith('feed-')) {
+				return this.$store.state.feeds.ordering[this.fetchKey]
+			}
+			return 0
+		},
+		changedGlobalOrdering() {
+			return this.$store.getters.oldestFirst
+		},
+		changedOrdering() {
+			return {
+			        feedOrdering: this.changedFeedOrdering,
+			        globalOrdering: this.changedGlobalOrdering,
+		      }
+		},
+		changedShowAll() {
+			return this.$store.getters.showAll
+		},
 	},
 	watch: {
 		getSelectedItem(newVal) {
 			this.selectedItem = newVal
 		},
 		fetchKey() {
+			this.listOrdering = this.getListOrdering()
+			this.cache = undefined
+		},
+		// rebuild filtered item list only when items has changed
+		items: {
+			handler() {
+				this.refreshItemList()
+			},
+			immediate: true,
+			deep: false,
+		},
+		// ordering has changed rebuild item list
+		changedOrdering() {
+			this.listOrdering = this.getListOrdering()
+			// make sure the first items from this ordering are loaded
+			this.fetchMore()
+			this.cache = undefined
+			// refresh the list with the new ordering
+			this.refreshItemList()
+			this.$refs.virtualScroll.scrollTop = 0
+		},
+		// showAll has changed rebuild item list
+		changedShowAll() {
 			this.cache = undefined
+			this.refreshItemList()
 		},
 	},
 	created() {
+		this.listOrdering = this.getListOrdering()
 		this.loadFilter()
 	},
 	mounted() {
 		this.mounted = true
+		this.setupDebouncedClick()
 	},
 	methods: {
+		refreshItemList() {
+			if (this.items.length > 0) {
+				this.filteredItemcache = this.filterSortedItems()
+			}
+		},
+		getListOrdering(): boolean {
+			if (!this.fetchKey.startsWith('feed-')) {
+				return this.$store.getters.oldestFirst
+			}
+			// this.config.ordering is only defined in feed route
+			let oldestFirst
+			switch (this.$store.state.feeds.ordering[this.fetchKey]) {
+			case FEED_ORDER.OLDEST:
+				oldestFirst = true
+				break
+			case FEED_ORDER.NEWEST:
+				oldestFirst = false
+				break
+			case FEED_ORDER.DEFAULT:
+			default:
+				oldestFirst = this.$store.getters.oldestFirst
+			}
+			return oldestFirst
+		},
 		storeFilter() {
 			try {
 				let filterString = 'noFilter'
@@ -231,7 +288,7 @@ export default Vue.extend({
 		},
 		outOfScopeFilter(item: FeedItem): boolean {
 			const lastItemLoaded = this.$store.state.items.lastItemLoaded[this.fetchKey]
-			return (this.$store.getters.oldestFirst ? lastItemLoaded >= item.id : lastItemLoaded <= item.id)
+			return (this.listOrdering ? lastItemLoaded >= item.id : lastItemLoaded <= item.id)
 		},
 		toggleFilter(filter: (item: FeedItem) => boolean) {
 			if (this.filter === filter) {
@@ -244,6 +301,7 @@ export default Vue.extend({
 			}
 
 			this.storeFilter()
+			this.refreshItemList()
 		},
 		filterSortedItems(): FeedItem[] {
 			let response = [...this.items] as FeedItem[]
@@ -274,13 +332,19 @@ export default Vue.extend({
 				response = response.filter(this.filter)
 			}
 
-			// filter items that are already loaded but do not yet match the current view (unread, all, folder...)
-			if (!this.fetchKey.startsWith('feed-') && this.$store.state.items.lastItemLoaded[this.fetchKey] > 0) {
+			// filter items that are already loaded but do not yet match the current view
+			if (this.$store.state.items.lastItemLoaded[this.fetchKey] > 0) {
 				response = response.filter(this.outOfScopeFilter)
 			}
 
 			return response.sort(this.sort)
 		},
+		// debounce clicks to prevent multiple api calls when on the end of the actual loaded list
+		setupDebouncedClick() {
+			this.debouncedClickItem = _.debounce((Item) => {
+				this.clickItem(Item)
+			}, 20, { leading: true })
+		},
 		// Trigger the click event programmatically to benefit from the item handling inside the FeedItemRow component
 		clickItem(item: FeedItem) {
 			if (!item) {
@@ -304,7 +368,7 @@ export default Vue.extend({
 			return this.selectedItem ? items.findIndex((item: FeedItem) => item.id === this.selectedItem.id) || 0 : -1
 		},
 		jumpToPreviousItem() {
-			const items = this.filterSortedItems()
+			const items = this.filteredItemcache
 			let currentIndex = this.currentIndex(items)
 			// Prepare to jump to the first item, if none was selected
 			if (currentIndex === -1) {
@@ -313,16 +377,18 @@ export default Vue.extend({
 			// Jump to the previous item
 			if (currentIndex > 0) {
 				const previousItem = items[currentIndex - 1]
-				this.clickItem(previousItem)
+				this.debouncedClickItem(previousItem)
+
 			}
 		},
+
 		jumpToNextItem() {
-			const items = this.filterSortedItems()
+			const items = this.filteredItemcache
 			const currentIndex = this.currentIndex(items)
 			// Jump to the first item, if none was selected, otherwise jump to the next item
 			if (currentIndex === -1 || (currentIndex < items.length - 1)) {
 				const nextItem = items[currentIndex + 1]
-				this.clickItem(nextItem)
+				this.debouncedClickItem(nextItem)
 			}
 		},
 	},

src/components/feed-display/FeedItemRow.vue

@@ -141,7 +141,7 @@ export default Vue.extend({
 			return this.$store.getters.feeds.find((feed: Feed) => feed.id === id) || {}
 		},
 		markRead(item: FeedItem): void {
-			if (!this.keepUnread) {
+			if (!this.keepUnread && item.unread) {
 				this.$store.dispatch(ACTIONS.MARK_READ, { item })
 			}
 		},

src/components/routes/Feed.vue

@@ -1,6 +1,6 @@
 <template>
-	<ContentTemplate :items="items"
-		:config="{ ordering: feed?.ordering || FEED_ORDER.NEWEST }"
+	<ContentTemplate v-if="!loading"
+		:items="items"
 		:fetch-key="'feed-' + feedId"
 		@load-more="fetchMore()">
 		<template #header>
@@ -23,7 +23,6 @@ import ContentTemplate from '../ContentTemplate.vue'
 import { FeedItem } from '../../types/FeedItem'
 import { ACTIONS, MUTATIONS } from '../../store'
 import { Feed } from '../../types/Feed'
-import { FEED_ORDER } from '../../dataservices/feed.service'
 
 export default Vue.extend({
 	components: {
@@ -37,13 +36,10 @@ export default Vue.extend({
 		},
 	},
 	computed: {
-		FEED_ORDER() {
-			return FEED_ORDER
-		},
 		...mapState(['items', 'feeds']),
-		feed(): Feed | null {
+		feed(): Feed {
 			const feeds = this.$store.getters.feeds
-			return feeds ? feeds.find((feed: Feed) => feed.id === this.id) : null
+			return feeds.find((feed: Feed) => feed.id === this.id)
 		},
 		items(): FeedItem[] {
 			return this.$store.state.items.allItems.filter((item: FeedItem) => {
@@ -53,6 +49,9 @@ export default Vue.extend({
 		id(): number {
 			return Number(this.feedId)
 		},
+		loading() {
+			return this.$store.getters.loading
+		},
 	},
 	created() {
 		this.$store.commit(MUTATIONS.SET_SELECTED_ITEM, { id: undefined })
@@ -61,8 +60,8 @@ export default Vue.extend({
 	},
 	methods: {
 		async fetchMore() {
-			if (!this.$store.state.items.fetchingItems['feed-' + this.feedId]) {
-			  this.$store.dispatch(ACTIONS.FETCH_FEED_ITEMS, { feedId: this.id, ordering: this.feed?.ordering || 0 })
+			if (!this.loading && !this.$store.state.items.fetchingItems['feed-' + this.feedId]) {
+			  this.$store.dispatch(ACTIONS.FETCH_FEED_ITEMS, { feedId: this.id })
 			}
 		},
 	},

src/dataservices/feed.service.ts

@@ -4,9 +4,9 @@ import axios from '@nextcloud/axios'
 import { API_ROUTES } from '../types/ApiRoutes'
 
 export enum FEED_ORDER {
+	DEFAULT = 0,
 	OLDEST = 1,
-	NEWEST = 0,
-	DEFAULT = 2,
+	NEWEST = 2,
 }
 
 export enum FEED_UPDATE_MODE {

src/dataservices/item.service.ts

@@ -2,6 +2,7 @@ import _ from 'lodash'
 import { AxiosResponse } from 'axios'
 import axios from '@nextcloud/axios'
 import store from './../store/app'
+import feedstore from './../store/feed'
 
 import { API_ROUTES } from '../types/ApiRoutes'
 import { FeedItem } from '../types/FeedItem'
@@ -85,12 +86,11 @@ export class ItemService {
 	 *
 	 * @param feedId id number of feed to retrieve items for
 	 * @param start (id of last unread item loaded)
-	 * @param ordering (0 = NEWEST, 1 = OLDEST, 2 = DEFAULT)
 	 * @return {AxiosResponse} response object containing backend request response
 	 */
-	static async fetchFeedItems(feedId: number, start?: number, ordering?: number): Promise<AxiosResponse> {
+	static async fetchFeedItems(feedId: number, start?: number): Promise<AxiosResponse> {
 		let oldestFirst
-		switch (ordering) {
+		switch (feedstore.state.ordering['feed-' + feedId]) {
 		case FEED_ORDER.OLDEST:
 			oldestFirst = true
 			break

src/store/feed.ts

@@ -27,11 +27,13 @@
 export type FeedState = {
 	feeds: Feed[];
 	unreadCount: number;
+	ordering: { [key: string]: number };
 }
 
 const state: FeedState = {
 	feeds: [],
 	unreadCount: 0,
+	ordering: {},
 }
 
 const getters = {
@@ -87,17 +89,17 @@
 			commit(FEED_MUTATION_TYPES.ADD_FEED, response.data.feeds[0])
 		} catch (e) {
 			// TODO: show error to user if failure
 			console.log(e)
 		}
 	},
 
 	async [FEED_ACTION_TYPES.MOVE_FEED](
 		{ commit }: ActionParams<FeedState>,
 		{ feedId, folderId }: { feedId: number, folderId: number },
 	) {
 		// Check that url is resolvable
 		try {
 			const response = await FeedService.moveFeed({
 				feedId,
 				folderId,
 			})
@@ -107,7 +109,7 @@
 			// commit(FEED_MUTATION_TYPES.UPDATE_FEED, { id: feedId, folderId })
 		} catch (e) {
 			// TODO: show error to user if failure
 			console.log(e)
 		}
 	},
 
@@ -140,6 +142,8 @@
 		{ commit }: ActionParams<FeedState>,
 		{ feed, ordering }: { feed: Feed, ordering: FEED_ORDER },
 	) {
+		state.ordering = { ...state.ordering, ['feed-' + feed.id]: ordering }
+		commit(FEED_ITEM_MUTATION_TYPES.SET_LAST_ITEM_LOADED, { key: 'feed-' + feed.id, lastItem: undefined })
 		await FeedService.updateFeed({ feedId: feed.id as number, ordering })
 
 		commit(FEED_MUTATION_TYPES.UPDATE_FEED, { id: feed.id, ordering })
@@ -205,6 +209,9 @@
 	) {
 		feeds.forEach(it => {
 			state.feeds.push(it)
+			if (it.ordering) {
+				state.ordering['feed-' + it.id] = it.ordering
+			}
 		})
 	},