Skip to content

fix(tabs): keep the sort and page of a restored tab you never opened - #2255

Merged
datlechin merged 1 commit into
mainfrom
fix/restored-tab-view-state
Aug 20, 2026
Merged

fix(tabs): keep the sort and page of a restored tab you never opened#2255
datlechin merged 1 commit into
mainfrom
fix/restored-tab-view-state

Conversation

@datlechin

Copy link
Copy Markdown
Member

Two defects in how a restored tab's view state round-trips. Found while investigating #2234; the second is why the first could not ship alone.

A tab you never clicked loses its sort and page

Restore parks a tab's saved sort and page in pendingRestoredSort / restoredPage rather than applying them, because applying needs the schema. Only applyPendingRestoredViewState consumes them, and it is reachable solely through prepareTableTabFirstLoad, whose first guard is tabManager.selectedTabId == tabId. So only the tab in front ever consumes its own state.

Every save maps every tab through toPersistedTab(), which re-derived the sort from sortState.columns and the page from pagination.currentPage. Both are still empty on a tab that has not been activated, so the very next autosave wrote nil over each of them.

Leave five tabs sorted and on page 3, quit, relaunch, and only the selected tab keeps its state. Wait 30 seconds for the periodic save, or just quit again, and the other four have lost it permanently, without the user having touched them.

toPersistedTab() now falls back to the pending values when the live ones are empty, which is exactly what restoredCursorOffset and columnWidths already do. The consumption side is untouched: applyPendingRestoredViewState stays the only consumer.

A restored page index was read in the wrong page size

This is why the first fix could not ship on its own. Restoring the page number makes the page restore actually happen, and the offset is recomputed as (page - 1) * pageSize against AppSettingsManager.shared.dataGrid.defaultPageSize, not against the size the page was counted in. PersistedTab had no page size at all.

Set a tab to 100 rows per page, go to page 12 (rows 1101-1200), quit. It comes back as page 12 of the 1,000 default: offset 11,000, an empty grid, and a pager reading 12 of 2. Fixing the loss alone would have taken this from rare to routine, which is what made it scope rather than a follow-up.

PersistedTab gains restoredPageSize, and restore reads the page index in the size it was counted in.

Two things fall out of that framing, and both are deliberate:

  • The size is persisted only as the unit for a persisted page index, never as "remember the user's page size". A tab on page 1 persists neither.
  • That is also what keeps Show All safe. It puts the whole result on one page, so currentPage is 1, so no page size is written and relaunching never re-fetches a million rows.

It also preserves the existing contract. PersistedTabRoundTripTests.paginationSeedsFromLivePageSize pins that a restored tab seeds its page size from the live default rather than from the persisted query text, and that test has a tab on page 1: no page size is persisted, the default still applies, and it passes unchanged. The new field is an explicit, trustworthy value rather than a parse of the query string, which is what that test was guarding against.

restoredPageSize is decoded with decodeIfPresent, so a tab written by an older build restores its page exactly as before.

Keeping the fallback from outliving the restore

Making the pending values survive a re-save means they now outlive a single activation, and three paths relied on them being transient. Self-review caught all three:

  • A stale page must not outrank live truth. The fallback holds only while execution.lastExecutedAt == nil. Without that, a user who pages back to 1 and quits reopens on page 12 forever, and nothing can correct it: the only code that clears the pending fields is the first-load path.
  • Retargeting a tab clears them. replaceTabContent already resets sort, pagination, filters and layout when a tab is pointed at a different table, which is what clicking a sidebar table during a restored tab's schema load does. The pending fields now reset with them, or orders' page 12 and sort get persisted against customers and applied to it next launch.
  • Only table tabs carry a pending sort. Consumption is gated on tabType == .table, so a query tab would never clear one and a sort the user cleared would reappear in the persisted record permanently.

restoredPageSize is also clamped to SettingsValidationRules.defaultPageSizeRange on decode, so a hand-edited or out-of-range value cannot make a restored tab issue an enormous first fetch.

Verification

Run through verify.sh in an isolated worktree on 752019a6e.

  • generate PASS, build PASS
  • test PASS, 46 of 46: PersistedTabRoundTripTests, DefaultSortInitialQueryTests, TabPersistenceTests, QueryTabManagerTests
  • swiftlint --strict clean over every touched file

The new tests pin the parts that failed silently: an unactivated tab re-emits its sort and its page, three consecutive save/restore cycles keep both, a persisted page carries the size it was counted in, page 1 carries no size (the Show All guard), and a tab written before the field existed still restores its page.

No UI automation. The flow is quit, relaunch and restore across three launches, driven by an autosave timer, which does not run deterministically in TableProUITests. The behaviour is covered at the round-trip boundary instead, which is where both causes live.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 1cb9028 into main Aug 20, 2026
8 checks passed
@datlechin
datlechin deleted the fix/restored-tab-view-state branch August 20, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant