refactor(settings): unify sidebar, data grid, and filter settings storage and UI#1847
refactor(settings): unify sidebar, data grid, and filter settings storage and UI#1847datlechin wants to merge 19 commits into
Conversation
…older stores with it
…pace the selected-pane key
…er-table layouts and filters
…rState as the single connection-scoped state
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b5326894
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cache[key.connectionId] = entries | ||
| writeEntries(entries, for: key.connectionId) | ||
| } | ||
| syncTracker.markDeleted(.settings, id: Self.syncCategory(for: key.storageKey)) |
There was a problem hiding this comment.
When a user resets a synced table layout or clears a hidden-only customization, this records a .settings tombstone, but performPush only serializes dirty settings records and never turns settings tombstones into recordIDsToDelete; applyRemoteChanges also ignores deleted Settings_ IDs. In that scenario the old CloudKit layout record is not removed, so another device or a later pull can re-apply the column widths/order/hidden columns the user just reset.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| static func syncCategory(for storageKey: String) -> String { | ||
| syncCategoryPrefix + storageKey |
There was a problem hiding this comment.
For tables with long database/schema/table identifiers, this embeds the full percent-encoded storageKey in the settings category, which SyncRecordMapper.recordID later uses as Settings_<category>. The CKRecord.ID docs document a 255-character ASCII limit for record names; a 63-character database, schema, and table name already exceeds that after the Settings_columnLayout. prefix and encoded UUID, so CloudKit can reject layout sync for those tables. Use a bounded stable ID, such as a hash, and keep the full scope in the payload.
Useful? React with 👍 / 👎.
1 participant
What and why
Settings for the sidebar, data grid, and filters were scattered across UserDefaults keys, JSON files, and ad-hoc storage, with no shared scoping and no single place to review or reset them. This branch unifies the storage foundation, reorganizes the Settings window, and fixes several bugs that fell out of the old layout. Native only (AppKit/SwiftUI, HIG), no compatibility shims, no PluginKit ABI changes. Old keys are migrated (read-old / write-new), so no saved user settings are lost.
User-facing changes
Added
Fixed (root-cause)
Internal refactors
DefaultsKey/PreferenceKeys/KeyValueStore), backing linked-folder and other stores.TableScopecomposite key with reversible percent-encoded storage components;CompositeStorageKeydelegates to it.ColumnLayoutPersisting), replacing the split visibility persister.SyncScopegate onSyncChangeTracker.markDirty; additive per-table cases inSyncCoordinator.ConnectionStorage/AIKeyStorage/LicenseStorage/SSHProfileStorage.SQLBoundaryValidator.SidebarViewModelsearch text and Redis tree forwarded into the connection-scopedSharedSidebarState(single source of truth).accentColortheme field and a duplicate accessibility observer.Testing
TableScopedecode, column-layout sync round-trip, filter match-mode persistence, saved-customizations service).--strict).main(KeychainHelper overwrite, DataChangeManager reload version, 2 TableQueryBuilderFilteredQuery). They are not regressions from this work.Verify before release
These surfaces are correct by unit test and build, but should get a pass in a running app before shipping: