mirror of
https://github.com/HeyPuter/puter.git
synced 2026-08-28 17:07:16 +00:00
fix: rank-preserving saved-order merge; keep the pager put after uninstall
- 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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user