fix(app): stop the quick switcher list rebuilding while you navigate (#1881)

fluxer_app/src/features/search/state/QuickSwitcher.tsx

@@ -6,7 +6,6 @@ import {onLocaleChange} from '@app/features/i18n/utils/LocaleChangeListener';
 import type {GuildMember} from '@app/features/member/models/GuildMember';
 import GuildMembers from '@app/features/member/state/GuildMembers';
 import MemberSearch, {type SearchContext, type TransformedMember} from '@app/features/member/state/MemberSearch';
-import Navigation from '@app/features/navigation/state/Navigation';
 import SelectedChannel from '@app/features/navigation/state/SelectedChannel';
 import {parseChannelUrl} from '@app/features/navigation/utils/DeepLinkUtils';
 import {Logger} from '@app/features/platform/utils/AppLogger';
@@ -19,7 +18,7 @@ import {
 	generateQueryModeResults,
 	resolveTransformedMember,
 } from '@app/features/search/state/QuickSwitcherResultGenerators';
-import {resolveRecomputedSelectedIndex} from '@app/features/search/state/QuickSwitcherSelection';
+import {hasSameResultIdentity, resolveRecomputedSelectedIndex} from '@app/features/search/state/QuickSwitcherSelection';
 import type {
 	CandidateSets,
 	ComputeResultsForQueryResult,
@@ -35,7 +34,7 @@ import MobileLayout from '@app/features/ui/state/MobileLayout';
 import {QuickSwitcherResultTypes} from '@fluxer/constants/src/QuickSwitcherConstants';
 import type {I18n} from '@lingui/core';
 import {msg} from '@lingui/core/macro';
-import {action, makeAutoObservable, reaction, runInAction} from 'mobx';
+import {action, makeAutoObservable, runInAction} from 'mobx';
 
 const GO_TO_MESSAGE_DESCRIPTOR = msg({
 	message: 'Go to message',
@@ -49,6 +48,7 @@ class QuickSwitcher {
 	private logger = new Logger('QuickSwitcher');
 	private candidateSets: CandidateSets | null = null;
 	private candidateWarmupCancel: (() => void) | null = null;
+	private defaultResults: Array<QuickSwitcherResult> | null = null;
 	private modalPreloadPromise: Promise<QuickSwitcherModalModule> | null = null;
 	private modalPreloadCancel: (() => void) | null = null;
 	isOpen = false;
@@ -65,34 +65,24 @@ class QuickSwitcher {
 	constructor() {
 		makeAutoObservable<
 			this,
-			'candidateSets' | 'candidateWarmupCancel' | 'logger' | 'modalPreloadCancel' | 'modalPreloadPromise'
+			| 'candidateSets'
+			| 'candidateWarmupCancel'
+			| 'defaultResults'
+			| 'logger'
+			| 'modalPreloadCancel'
+			| 'modalPreloadPromise'
 		>(
 			this,
 			{
 				candidateSets: false,
 				candidateWarmupCancel: false,
+				defaultResults: false,
 				logger: false,
 				modalPreloadCancel: false,
 				modalPreloadPromise: false,
 			},
 			{autoBind: true},
 		);
-		reaction(
-			() => SelectedChannel.recentChannelVisits,
-			() => {
-				if (this.isOpen) {
-					this.recomputeIfOpen();
-				}
-			},
-		);
-		reaction(
-			() => [Navigation.guildId, Navigation.channelId],
-			() => {
-				if (this.isOpen) {
-					this.recomputeIfOpen();
-				}
-			},
-		);
 	}
 
 	setI18n(i18n: I18n): void {
@@ -102,7 +92,8 @@ class QuickSwitcher {
 
 	private handleLocaleChange(): void {
 		this.invalidateCandidateSets();
-		this.recomputeIfOpen({invalidateCandidates: false});
+		this.defaultResults = null;
+		this.recomputeIfOpen({force: true, invalidateCandidates: false});
 	}
 
 	preloadModal(): void {
@@ -171,6 +162,7 @@ class QuickSwitcher {
 		if (this.isOpen) return;
 		this.cancelCandidateWarmup();
 		this.candidateSets = null;
+		this.defaultResults = null;
 		this.isOpen = true;
 		this.query = '';
 		this.queryMode = null;
@@ -231,6 +223,7 @@ class QuickSwitcher {
 		this.cancelCandidateWarmup();
 		this.isOpen = false;
 		this.candidateSets = null;
+		this.defaultResults = null;
 		this.query = '';
 		this.queryMode = null;
 		this.results = [];
@@ -303,6 +296,7 @@ class QuickSwitcher {
 		if (!this.isOpen && query.length === 0) {
 			return;
 		}
+		this.defaultResults = null;
 		const {queryMode, results, selectedIndex} = this.computeResultsForQuery(query);
 		this.query = query;
 		this.queryMode = queryMode;
@@ -398,7 +392,7 @@ class QuickSwitcher {
 		this.selectedIndex = selectedIndex;
 	}
 
-	recomputeIfOpen(options: {invalidateCandidates?: boolean} = {}): void {
+	recomputeIfOpen(options: {force?: boolean; invalidateCandidates?: boolean} = {}): void {
 		if (!this.isOpen) {
 			return;
 		}
@@ -407,11 +401,21 @@ class QuickSwitcher {
 		}
 		const previous = this.results[this.selectedIndex];
 		const {queryMode, results, selectedIndex} = this.computeResultsForQuery(this.query);
+		if (!options.force && this.queryMode === queryMode && hasSameResultIdentity(this.results, results)) {
+			return;
+		}
 		this.queryMode = queryMode;
 		this.results = results;
 		this.selectedIndex = resolveRecomputedSelectedIndex(previous, results, selectedIndex);
 	}
 
+	private getDefaultResults(i18n: I18n): Array<QuickSwitcherResult> {
+		if (this.defaultResults == null || this.defaultResults.length === 0) {
+			this.defaultResults = generateDefaultResults(i18n);
+		}
+		return this.defaultResults;
+	}
+
 	private computeResultsForQuery(query: string): ComputeResultsForQueryResult {
 		const i18n = this.getI18n();
 		const channelPath = parseChannelUrl(query);
@@ -430,11 +434,11 @@ class QuickSwitcher {
 			};
 		}
 		if (query['trim']().length === 0) {
-			const results = generateDefaultResults(i18n);
+			const defaultResults = this.getDefaultResults(i18n);
 			return {
 				queryMode: null,
-				results,
+				results: defaultResults,
-				selectedIndex: getFirstSelectableIndex(results),
+				selectedIndex: getFirstSelectableIndex(defaultResults),
 			};
 		}
 		const queryMode = this.getQueryMode(query);
@@ -445,7 +449,7 @@ class QuickSwitcher {
 			const sets = this.getCandidateSets(i18n);
 			results = generateQueryModeResults(queryMode, trimmedSearch, sets, i18n, this.memberSearchResults);
 		} else if (trimmedSearch.length === 0) {
-			results = generateDefaultResults(i18n);
+			results = this.getDefaultResults(i18n);
 		} else {
 			const sets = this.getCandidateSets(i18n);
 			results = generateGeneralResults(trimmedSearch, sets, i18n);

fluxer_app/src/features/search/state/QuickSwitcherSelection.test.ts

@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: AGPL-3.0-or-later
 
-import {resolveRecomputedSelectedIndex} from '@app/features/search/state/QuickSwitcherSelection';
+import {hasSameResultIdentity, resolveRecomputedSelectedIndex} from '@app/features/search/state/QuickSwitcherSelection';
 import type {QuickSwitcherResult} from '@app/features/search/state/QuickSwitcherTypes';
 import {QuickSwitcherResultTypes} from '@fluxer/constants/src/QuickSwitcherConstants';
 import {describe, expect, it} from 'vitest';
 
 const result = (type: QuickSwitcherResult['type'], id: string) => ({type, id}) as QuickSwitcherResult;
 
+const labelled = (type: QuickSwitcherResult['type'], id: string, title: string, subtitle?: string) =>
+	({type, id, title, subtitle}) as QuickSwitcherResult;
+
 describe('resolveRecomputedSelectedIndex', () => {
 	it('follows the focused result when the list is reordered', () => {
 		const previous = result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c3');
@@ -44,3 +47,86 @@ describe('resolveRecomputedSelectedIndex', () => {
 		expect(resolveRecomputedSelectedIndex(undefined, [result(QuickSwitcherResultTypes.GUILD, 'g')], 0)).toBe(0);
 	});
 });
+
+describe('hasSameResultIdentity', () => {
+	it('treats a rebuilt list of the same rows in the same order as unchanged', () => {
+		const previous = [
+			result(QuickSwitcherResultTypes.HEADER, 'people'),
+			result(QuickSwitcherResultTypes.USER, 'u1'),
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1'),
+		];
+		const next = [
+			result(QuickSwitcherResultTypes.HEADER, 'people'),
+			result(QuickSwitcherResultTypes.USER, 'u1'),
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1'),
+		];
+		expect(hasSameResultIdentity(previous, next)).toBe(true);
+	});
+
+	it('detects a reorder', () => {
+		const previous = [
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1'),
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c2'),
+		];
+		const next = [
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c2'),
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1'),
+		];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('detects an insertion', () => {
+		const previous = [result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1')];
+		const next = [
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1'),
+			result(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c2'),
+		];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('detects a row whose type changed under the same id', () => {
+		const previous = [result(QuickSwitcherResultTypes.TEXT_CHANNEL, '42')];
+		const next = [result(QuickSwitcherResultTypes.VOICE_CHANNEL, '42')];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('treats two empty lists as unchanged', () => {
+		expect(hasSameResultIdentity([], [])).toBe(true);
+	});
+
+	it('detects a renamed row so the open list repaints', () => {
+		const previous = [labelled(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1', 'general')];
+		const next = [labelled(QuickSwitcherResultTypes.TEXT_CHANNEL, 'c1', 'general-chat')];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('detects a changed subtitle under an unchanged title', () => {
+		const previous = [labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada', 'ada#0001')];
+		const next = [labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada', 'ada#0002')];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('detects a subtitle that appeared where there was none', () => {
+		const previous = [labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada')];
+		const next = [labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada', 'ada#0001')];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('detects a renamed section header', () => {
+		const previous = [labelled(QuickSwitcherResultTypes.HEADER, 'people', 'People')];
+		const next = [labelled(QuickSwitcherResultTypes.HEADER, 'people', 'Personnes')];
+		expect(hasSameResultIdentity(previous, next)).toBe(false);
+	});
+
+	it('still treats an identically labelled rebuild as unchanged', () => {
+		const previous = [
+			labelled(QuickSwitcherResultTypes.HEADER, 'people', 'People'),
+			labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada', 'ada#0001'),
+		];
+		const next = [
+			labelled(QuickSwitcherResultTypes.HEADER, 'people', 'People'),
+			labelled(QuickSwitcherResultTypes.USER, 'u1', 'ada', 'ada#0001'),
+		];
+		expect(hasSameResultIdentity(previous, next)).toBe(true);
+	});
+});

fluxer_app/src/features/search/state/QuickSwitcherSelection.ts

@@ -2,6 +2,31 @@
 
 import type {QuickSwitcherResult} from '@app/features/search/state/QuickSwitcherTypes';
 
+function getResultSubtitle(result: QuickSwitcherResult): string | undefined {
+	return 'subtitle' in result ? result.subtitle : undefined;
+}
+
+export function hasSameResultIdentity(
+	previous: ReadonlyArray<QuickSwitcherResult>,
+	next: ReadonlyArray<QuickSwitcherResult>,
+): boolean {
+	if (previous.length !== next.length) {
+		return false;
+	}
+	for (let i = 0; i < previous.length; i += 1) {
+		if (previous[i].type !== next[i].type || previous[i].id !== next[i].id) {
+			return false;
+		}
+		if (previous[i].title !== next[i].title) {
+			return false;
+		}
+		if (getResultSubtitle(previous[i]) !== getResultSubtitle(next[i])) {
+			return false;
+		}
+	}
+	return true;
+}
+
 export function resolveRecomputedSelectedIndex(
 	previous: QuickSwitcherResult | undefined,
 	results: ReadonlyArray<QuickSwitcherResult>,