From e188048f97e9e97aab497b5aa65b8181815d65d2 Mon Sep 17 00:00:00 2001 From: jelveh Date: Sat, 18 Jul 2026 18:07:55 -0700 Subject: [PATCH] fix: rank-preserving saved-order merge; keep the pager put after uninstall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The saved-order carryover appended missing apps' names at the tail, permanently demoting their saved grid positions — the very thing it claimed to protect. Replace it with mergeSavedOrder in appOrder.js (beside its tested siblings, with unit tests): each missing name keeps its rank among surviving names, so a drag during a partial-load session neither drops hidden apps' positions nor teleports them behind the dragged tile. - The post-uninstall convergence refetch rendered without preservePage, snapping the pager back to page 1 and replaying the load fade; thread render options through loadApps so the background sync keeps the user's page. --- src/gui/src/UI/Dashboard/TabApps.js | 30 ++++++------ src/gui/src/UI/Dashboard/appOrder.js | 55 ++++++++++++++++++++++ src/gui/src/UI/Dashboard/appOrder.test.js | 57 ++++++++++++++++++++++- 3 files changed, 124 insertions(+), 18 deletions(-) diff --git a/src/gui/src/UI/Dashboard/TabApps.js b/src/gui/src/UI/Dashboard/TabApps.js index f35b07e27..258870469 100644 --- a/src/gui/src/UI/Dashboard/TabApps.js +++ b/src/gui/src/UI/Dashboard/TabApps.js @@ -1,6 +1,6 @@ import UIContextMenu from '../UIContextMenu.js'; import { isTouchPrimaryDevice } from './ContextMenu/ContextMenu.js'; -import { reconcileAppOrder, serializeAppOrder, APPS_ORDER_KV_KEY } from './appOrder.js'; +import { reconcileAppOrder, serializeAppOrder, mergeSavedOrder, APPS_ORDER_KV_KEY } from './appOrder.js'; /** Lowercase app names that must not offer Uninstall in the My Apps tile context menu. */ const APP_NAMES_NO_UNINSTALL = new Set([ @@ -185,7 +185,9 @@ function showUninstallModal ({ appName, appTitle, appUid, removesTile, self, $el self._apps = self._apps.filter(a => a.name !== appName); self.renderApps($el_window, { preservePage: true }); } - self.loadApps($el_window); + // Keep the user's page and skip the load fade — this is a + // background sync, not a fresh visit. + self.loadApps($el_window, { preservePage: true, instant: true }); } catch ( err ) { console.error('Failed to uninstall app:', err); } @@ -946,18 +948,12 @@ const TabApps = { saveOrder () { this._hasCustomOrder = true; - const names = serializeAppOrder(this._apps); - // Carry over saved names that aren't in the current list (e.g. apps - // whose installedApps page failed to load this session): the saved - // order is the only record of their positions, so dropping them would - // lose those for good, while stale names are harmless — + // Merge with the previously saved order so names absent from the + // current list (e.g. apps whose installedApps page failed to load + // this session) keep their saved positions — the saved order is the + // only record of them, and stale names are harmless because // reconcileAppOrder ignores them. - if ( Array.isArray(this._savedOrderNames) ) { - const have = new Set(names); - for ( const name of this._savedOrderNames ) { - if ( ! have.has(name) ) names.push(name); - } - } + const names = mergeSavedOrder(serializeAppOrder(this._apps), this._savedOrderNames); this._savedOrderNames = names; try { const p = puter.kv.set(APPS_ORDER_KV_KEY, JSON.stringify(names)); @@ -969,7 +965,7 @@ const TabApps = { } }, - loadApps ($el_window) { + loadApps ($el_window, renderOpts) { if ( this._drag ) { // Don't fetch/re-render on top of a live drag; cancel a pending // (not-yet-started) pickup so a rebuild can't strand it. @@ -979,14 +975,14 @@ const TabApps = { // init and the initial-route onActivate both fire on open; join the // in-flight load instead of issuing a duplicate request trio. if ( this._loadPromise ) return this._loadPromise; - const p = this._fetchAndRenderApps($el_window).finally(() => { + const p = this._fetchAndRenderApps($el_window, renderOpts).finally(() => { if ( this._loadPromise === p ) this._loadPromise = null; }); this._loadPromise = p; return p; }, - async _fetchAndRenderApps ($el_window) { + async _fetchAndRenderApps ($el_window, renderOpts = {}) { // Give each load a monotonically increasing id. An older/slower // response must not clobber a newer one that already applied — or a // reorder the user saved while a stale fetch was in flight. We gate on @@ -1153,7 +1149,7 @@ const TabApps = { this._hasCustomOrder = Array.isArray(orderedNames) && orderedNames.length > 0; this._apps = reconcileAppOrder(merged, orderedNames); - this.renderApps($el_window); + this.renderApps($el_window, renderOpts); } catch (e) { console.error('Failed to load installed apps:', e); // Only show the failure placeholder when nothing has loaded yet; a diff --git a/src/gui/src/UI/Dashboard/appOrder.js b/src/gui/src/UI/Dashboard/appOrder.js index 4f2c9cf52..42d94dfa8 100644 --- a/src/gui/src/UI/Dashboard/appOrder.js +++ b/src/gui/src/UI/Dashboard/appOrder.js @@ -71,3 +71,58 @@ export function serializeAppOrder (apps) { .map(app => app && app.name) .filter(name => typeof name === 'string' && name.length > 0); } + +/** + * Merge the order being saved with the previously saved one so that names + * absent from `currentNames` (e.g. apps on an installedApps page that failed + * to load this session) keep their saved positions instead of being dropped + * or demoted to the tail. Each missing name keeps its RANK: if k survivors + * (names in both lists) preceded it in the saved order, it is re-inserted + * after the k-th survivor of the new order — so a drag of some visible tile + * neither drags hidden apps along nor pushes them off their slots. Present + * names appear exactly in `currentNames` order. Kept beside + * {@link serializeAppOrder} because it produces the same persisted shape. + * + * @param {string[]} currentNames - the on-screen order being saved + * @param {string[]|null|undefined} previousNames - the last saved order + * @returns {string[]} + */ +export function mergeSavedOrder (currentNames, previousNames) { + const result = Array.isArray(currentNames) ? currentNames.slice() : []; + if ( ! Array.isArray(previousNames) || previousNames.length === 0 ) return result; + + const currentSet = new Set(result); + const prevSet = new Set(previousNames); + // A survivor is a name present in both lists; re-inserted missing names + // and brand-new names never count when locating the k-th survivor. + const isSurvivor = name => currentSet.has(name) && prevSet.has(name); + + const seen = new Set(); + let rank = 0; // survivors encountered so far in the saved order + let lastInsert = -1; // keeps runs of missing names in their saved order + for ( const name of previousNames ) { + if ( typeof name !== 'string' || name.length === 0 ) continue; + if ( seen.has(name) ) continue; + seen.add(name); + if ( currentSet.has(name) ) { + rank++; + lastInsert = -1; + continue; + } + // Find the index just past the rank-th survivor in the result. + let at = 0; + if ( rank > 0 ) { + let survivors = 0; + for ( let i = 0; i < result.length; i++ ) { + if ( isSurvivor(result[i]) && ++survivors === rank ) { + at = i + 1; + break; + } + } + } + if ( lastInsert >= at ) at = lastInsert + 1; + result.splice(at, 0, name); + lastInsert = at; + } + return result; +} diff --git a/src/gui/src/UI/Dashboard/appOrder.test.js b/src/gui/src/UI/Dashboard/appOrder.test.js index d3fc667b8..3e710cb4e 100644 --- a/src/gui/src/UI/Dashboard/appOrder.test.js +++ b/src/gui/src/UI/Dashboard/appOrder.test.js @@ -18,7 +18,7 @@ */ import { describe, it, expect } from 'vitest'; -import { reconcileAppOrder, serializeAppOrder } from './appOrder.js'; +import { reconcileAppOrder, serializeAppOrder, mergeSavedOrder } from './appOrder.js'; const names = apps => apps.map(a => a.name); const mk = (...ns) => ns.map(n => ({ name: n })); @@ -86,3 +86,58 @@ describe('serializeAppOrder', () => { expect(names(reconcileAppOrder(apps, saved))).toEqual(['d', 'c', 'b', 'a']); }); }); + +describe('mergeSavedOrder', () => { + it('returns the current order when nothing was saved before', () => { + expect(mergeSavedOrder(['a', 'b'], null)).toEqual(['a', 'b']); + expect(mergeSavedOrder(['a', 'b'], [])).toEqual(['a', 'b']); + }); + + it('keeps present names exactly in the current order', () => { + expect(mergeSavedOrder(['c', 'a', 'b'], ['a', 'b', 'c'])).toEqual(['c', 'a', 'b']); + }); + + it('re-inserts a missing name after its surviving predecessor', () => { + // 'm' sat between 'b' and 'c' in the saved order; it must return to + // that slot, not be demoted to the tail. + expect(mergeSavedOrder(['a', 'b', 'c'], ['a', 'b', 'm', 'c'])).toEqual(['a', 'b', 'm', 'c']); + }); + + it('keeps a missing name at the front when it led the saved order', () => { + expect(mergeSavedOrder(['a', 'b'], ['m', 'a', 'b'])).toEqual(['m', 'a', 'b']); + }); + + it('keeps runs of missing names in their saved order', () => { + expect(mergeSavedOrder(['a', 'b'], ['a', 'm1', 'm2', 'b'])).toEqual(['a', 'm1', 'm2', 'b']); + }); + + it('keeps a missing name at its rank when visible tiles are rearranged', () => { + // 'm' was third; the user swapped 'a' and 'b'. 'm' stays third — it + // neither follows 'b' to the front nor gets demoted. + expect(mergeSavedOrder(['b', 'a', 'c'], ['a', 'b', 'm', 'c'])).toEqual(['b', 'a', 'm', 'c']); + }); + + it('preserves a truncated tail after a partial load and a drag', () => { + // Saved order covers 6 apps; only the first 4 loaded (page 2 failed) + // and the user dragged 'd' to the front. The unloaded tail must keep + // its saved position after the surviving 'c', not vanish. + const merged = mergeSavedOrder(['d', 'a', 'b', 'c'], ['a', 'b', 'c', 'd', 'e', 'f']); + expect(merged).toEqual(['d', 'a', 'b', 'c', 'e', 'f']); + // Round-trip: when the full list loads again, 'e' and 'f' come back + // in their saved slots. + expect(names(reconcileAppOrder(mk('a', 'b', 'c', 'd', 'e', 'f'), merged))) + .toEqual(['d', 'a', 'b', 'c', 'e', 'f']); + }); + + it('ignores unusable saved entries and duplicates', () => { + expect(mergeSavedOrder(['a'], ['', null, 'a', 'a', 'm'])).toEqual(['a', 'm']); + }); + + it('does not mutate its inputs', () => { + const current = ['a', 'b']; + const previous = ['b', 'm', 'a']; + mergeSavedOrder(current, previous); + expect(current).toEqual(['a', 'b']); + expect(previous).toEqual(['b', 'm', 'a']); + }); +});