Region-picking onboarding + per-region customization#95
Merged
Conversation
Build the deferred onboarding step that lets the user pick up to 5 US primary regions (map or searchable list, via a top segmented control) and customize each region's color/emoji/icon from predefined lists. - WhereCore: RegionColorToken / RegionAppearance / PrimaryRegion value types; SDTrackedRegion carries appearance + pick order; WhereStore primaryRegions() / setPrimaryRegion(). Picks are the tracked-region set. - WhereUI: RegionAppearanceCatalog + RegionStyleRegistry make RegionStyle data-driven (seeded by WhereSession on launch/changes, and by the widget snapshot in the widget process); PrimaryRegionSelectionModel, RegionPickerView, RegionCustomizeView, RegionsSettingsView. - OnboardingView is now an intro -> pick -> customize -> location state machine that commits picks before finishing; Settings gains a Regions editor. WidgetSnapshot carries appearances so widgets render user looks. - Tests: store round-trip/ordering, selection-model cap, registry resolution, widget appearance; RegionPickerStyle stylesheet group + docs.
Address code-review finding #1 (onboarding List had no search field because .searchable needs a navigation container) and make the onboarding and Settings region flows visually consistent: - Onboarding pick phase is a NavigationStack with a large title and a top-right Next (forward-only after the intro); its list is now filterable via .searchable. - RegionCustomizeView moves Back/Next (Done on the last region) into the nav bar with the region name as the inline title and an X-of-N indicator; callers place it in a NavigationStack. - Onboarding customize phase wraps it in a NavigationStack. - RegionsSettingsView: pick shows Cancel + Next (large title, matching onboarding), customize delegates to RegionCustomizeView's toolbar; dropped the old bottom buttons so each phase owns its own bar.
Address code-review finding #2: the commit looped per-region setPrimaryRegion/removePrimaryRegion calls, each its own store.perform, so N picks fired N changes() pings (attribution rebuild, widget republish, report reload, registry reseed each) and a mid-loop failure left partial state. Replace those single-region WhereServices/WhereStore calls with one batch setPrimaryRegions([PrimaryRegion]) that has replace semantics: upsert each entry (appearance + order) and delete any tracked row not in the list, all inside a single perform. Removals now happen by omission, so the model no longer tracks an initial set to diff against — the selection fully describes the primary set. One transaction, one ping, atomic. Tests updated to the batch API.
Address code-review finding #3. The app is US-only now (the legacy Canada/EU regions ship low-resolution polygons and aren't pickable), so seeding the editor from the default set and saving intentionally drops the non-US regions. Spell that out in PrimaryRegionSelectionModel's init(existing:) doc and pin it with a test: loading the default set into the model selects only CA/NY, and committing narrows the tracked set to those.
Address code-review finding #4: RegionStyle read from a process-wide RegionStyleRegistry.shared singleton — fine in production but a latent test-isolation hazard in the shared multi-bundle host, and it made styles update lazily rather than reactively. Replace it with a RegionStyleResolver injected through the environment (\.regionStyles), seeded by whereBroadwayRoot(regionStyles:): - the app passes WhereSession's live resolver (updated on launch + store changes), so a Settings edit / remote import now restyles the UI reactively; - the widget process builds one from WidgetSnapshot.appearances; - App Intents snippets build one from their services. The default empty resolver yields fallback looks (previews, RegionViewer). Views now read @Environment(\.regionStyles) and call regionStyles.style(for:); the global region.style / RegionStyle.style(for:) and the registry are gone. Renamed the file/test to RegionStyleResolver and updated docs.
Address code-review finding #5: unifying the bespoke look table into RegionAppearanceCatalog.defaultAppearance changed the user-visible .other glyph. Restore location.magnifyingglass (the fallback default isn't constrained to the picker's symbol catalog). EU/Canada looks are left as-is since those regions are being retired (not pickable).
Address code-review finding #6: the List disables unselected rows at the 5-region cap, but the map silently ignored a tap on a new state. Now a capped map tap fires a warning haptic (.sensoryFeedback), and an at-capacity hint appears under the selection count in both modes, so the map is as responsive/legible as the list.
Address code-review finding #7: the picker parsed all ~52 US-region GeoJSON files on appear to back Map mode. Default to List (no geometry needed) and only run the parse when Map mode is first shown (.task(id: mode)), so a List-only user never pays the cost.
Address code-review finding #8: harden RegionCustomizeView so a stale/out-of-range step index can never strand the user on the empty state. Navigation and the step indicator now use a clamped effectiveIndex; the empty (nil) state is reserved for a genuinely empty selection, which the current flow never reaches.
The test helper primary(_:_:_:) collided with the local
'let primary = try await store.primaryRegions()' variables. Local batch
compilation resolved the call to the method, but CI's compilation mode
resolved it to the [PrimaryRegion] local ('cannot call value of
non-function type'). Rename the helper to pick() to remove the ambiguity.
Backups previously stored tracked regions as bare ids, so a restore brought back the regions but not their picked color/emoji/icon. Carry the full primary-region set (appearance + order) in the archive so a restore is lossless. - BackupArchive gains an additive primaryRegions: [PrimaryRegion] (decode-if-present), keeping the legacy trackedRegions ids alongside it for cross-version restores; resolvedPrimaryRegions falls back to those when primaryRegions is absent. PrimaryRegion is now Codable. - Export writes both; import restores via setPrimaryRegions for .replace and a merge (archive appearance wins on overlap) for .merge. - Tests: archive + end-to-end store round-trips, legacy fallback. - WhereCore AGENTS: add an invariant that persisted-model changes must be reflected in the backup end-to-end (archive + export + import + test).
Offer 'Restore from a backup' on the onboarding intro so a returning user can skip the manual region setup: it imports the chosen backup (.replace, since it's a fresh install) and jumps straight to the location ask. The location finale skips committing the manual selection when a restore ran, so it doesn't wipe the regions the restore wrote. Failures surface an alert and keep the user in onboarding.
Add a root AGENTS rule that a PR's title/description must be updated in the same turn you push whenever the PR changes beyond a small bug fix (new behavior/API/data model, migration, scope change, follow-up, or approach-altering review fixes), reflect the end state, preserve human edits, and use gh pr edit when the PR tooling can't target the repo.
The manual-day form listed the full catalog flat. Now it groups the toggles: the user's tracked regions first, then any non-tracked regions the day already uses, then everything else in a collapsed disclosure. - RegionSelectionState gains applyTracked(_:) plus trackedItems / usedItems / otherItems; grouping keys off the initial selection + the tracked set so a row never jumps sections as it's toggled. Until the tracked set loads it stays nil and the form renders the flat list (also the preview/no-services fallback). - ManualDayView loads primaryRegions() once (.task) and renders the three sections; the collapsed 'More regions' disclosure holds the rest. - Tests cover the partition, tracked-over-used precedence, and toggle stability.
Correct the grouping: the middle section is now 'Already this year' — any non-tracked region with days in the selected year (from the report totals), not just regions on the day being edited. .other always stays in the collapsed 'More regions' group; that group auto-expands if a currently-on region landed there, so an existing tag is never hidden. RegionSelectionState.applyGrouping(tracked:usedThisYear:) replaces applyTracked; tests updated.
# Conflicts: # Where/WhereCore/Sources/Backup/BackupArchive.swift # Where/WhereCore/Tests/BackupServiceTests.swift
kyleve
marked this pull request as ready for review
July 17, 2026 01:38
# Conflicts: # Where/WhereUI/Sources/Secondary/RegionDaysView.swift
Match the Log-a-day grouping in the Settings 'Your regions' editor: the picker's List mode now shows Your regions / Used this year / More (collapsed) instead of one flat list. Grouping is Settings-only and enabled via PrimaryRegionSelectionModel.applyGrouping(usedThisYear:); onboarding keeps the flat searchable list, and an active search flattens to matches across sections. Sections are stable (keyed on the picks at open + the year's used regions) so rows don't jump while toggling; the map mode is unchanged. RegionsSettingsView sources used-this-year from the report via SettingsView. Tests cover the partition + toggle stability.
The manual-day form and the primary-region picker had converged on the same tracked/used-this-year/everything-else layout but reimplemented it. Extract the shared pieces so they stay in sync: - RegionGrouping: a pure, order-preserving value type that partitions regions given a stable "primary" reference set + the year's used regions (.other always in "other"). Both RegionSelectionState and PrimaryRegionSelectionModel now expose a `grouping` backed by it. - GroupedRegionSections: a shared SwiftUI view rendering Your regions / Used this year / More (collapsed, auto-expanded when nothing precedes it) from a RegionGrouping, with the row supplied by the caller (Toggle for manual-day, checkmark button for the picker). - One set of section-header strings (regionGroup.*), fixing the drifted "Already this year" vs "Used this year". The two models/selection semantics and row affordances stay distinct; this only unifies the partition, the section layout, and the labels. Tests: RegionGroupingTests for the partition, plus the existing model tests updated to the shared grouping.
kyleve
force-pushed
the
cursor/region-onboarding
branch
from
July 17, 2026 02:52
6228652 to
6ce4a04
Compare
kyleve
commented
Jul 17, 2026
|
|
||
| // Returning users can skip the manual setup by restoring a backup. | ||
| if isRestoring { | ||
| ProgressView(Strings.onboardingRestoring) |
Owner
Author
There was a problem hiding this comment.
Let's use the standard app icon loading view we use elsewhere, not a vanilla progress view.
Owner
Author
There was a problem hiding this comment.
Posted by an AI agent on kyleve's behalf.
Done in 868ef42 — the restore now uses the shared AppIconLoadingView (the pulsing app-icon + caption used for first loads / the data-issue scan / the recent-activity summary) instead of a ProgressView. Since a restore is a whole-screen blocking wait, it replaces the intro while running, then advances to the location step on success (or surfaces the error alert and returns to the intro on failure).
Address the second code review: - #2 (restore-no-wipe): onboarding's location finale now guards the region commit on `selection.hasSelection` instead of a `didRestoreBackup` flag. The manual flow can't reach the finale with an empty selection (its Next is gated on a selection), and the restore path always does — so this directly prevents committing empty (which would delete the just-imported regions). Removed the now-unneeded flag. Added a model test locking in that an empty commit wipes explicit picks (reverts to the default set), documenting why the guard exists. - #1 (backup .merge appearance): added BackupCoordinatorTests covering that a merge keeps the device's customized look when the archive's is nil, and lets the archive's look win on overlap while appending archive-only regions. - #3: noted that "used this year" is sourced from the selected report year (grouping order only) in ManualDayView + SettingsView.
PR feedback: replace the vanilla ProgressView shown while restoring a backup with the standard AppIconLoadingView (the shared "working" treatment used for first loads / scans / summary generation). A restore is a whole-screen blocking wait, so it now replaces the intro with the pulsing app-icon + caption rather than an inline spinner; the intro footer is back to just its two buttons.
# Conflicts: # Where/WhereCore/AGENTS.md # Where/WhereCore/README.md # Where/WhereIntents/Sources/Resources/Localizable.xcstrings # Where/WhereUI/Sources/Resources/Localizable.xcstrings # Where/WhereUI/Sources/Shared/Strings.swift
kyleve
added a commit
that referenced
this pull request
Jul 18, 2026
Conflict resolutions: - RegionMapView.swift: kept both sides' environment reads — our \.isCapturingSnapshot (SnapshotMapStandIn substitution) alongside main's \.regionStyles (data-driven per-region tints, #95). - SettingsView.swift: import-block conflict only — kept both SnapshotKit (our SnapshotDatePickerStandIn substitution) and RegionKit (main's new Regions section + RegionsSettingsView sheet). Both features coexist in the body via git's auto-merge. - AGENTS.md, WhereUI AGENTS.md/README.md, ManualDayView, CalendarView, PreviewSupport auto-merged cleanly: our snapshot-testing docs/stand-ins sit alongside main's flaky-tool docs, PR-description rule, region-style resolver docs, grouped region sections, and the primaryRegionSelectionModel fixture. No new stand-ins were needed: main's new snapshot-visible surfaces (grouped region toggles, the Settings Regions row, restore-from-backup button) are deterministic; the region-picker's live Map is not part of any captured snapshot case. Re-recorded references (all legitimate upstream UI changes from #95): - onboarding.Default_* (10): the intro adds the 'Restore from a backup' button under Continue, shifting the page content up. - secondary.Loaded_* (7): the EU card's stamp symbol changed from star.circle.fill to star.fill (RegionAppearanceCatalog's default). - settings.Default_* (10): new Regions section between Location and Reminders (iPhone_ax5 unchanged — below the captured fold). - manualDay.* (16): region toggles are now grouped (Your regions / More regions disclosure) instead of one flat catalog list. Stuff-iOS-Tests passes. Snapshot suite: failures triaged in a full diagnostic run (artifact-vs-reference inspected for each), re-recorded references verified green with a targeted run; the final all-green full-suite pass was skipped per standing instruction — PR CI serves as the full-suite verification. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the deferred Where onboarding step: pick up to 5 US primary regions (map or filterable list, via a top segmented control), then customize each region's color / emoji / icon from predefined lists. Picks become the tracked-region set; picks + appearance persist in CloudKit-mirrored SwiftData, feed region styling, and round-trip through backups. Editable later in Settings, and a returning user can skip the whole manual flow by restoring a backup.
What's included
WhereCore — persistence & domain
RegionColorToken,RegionAppearance,PrimaryRegion(Codable).SDTrackedRegioncarries appearance (colorRaw/emoji/symbolName) +orderIndex(all optional for CloudKit — additive, no migration scaffolding).WhereStore.primaryRegions()and a single atomicsetPrimaryRegions([PrimaryRegion])(replace semantics: upserts + removals-by-omission in oneperform). Picks are the tracked set.BackupArchivecarriesprimaryRegions: [PrimaryRegion](appearance + pick order) — a strictCodablefield, socurrentFormatVersionis bumped to 2 and pre-v2 archives are upgraded out of band byTools/upgrade-backup.rb(synthesizesprimaryRegionsfrom the legacytrackedRegionsids), matching WhereCore's no-migration-on-read rule. Import restores viasetPrimaryRegions(.replaceexact,.mergeunions with archive appearance winning).WhereUI — flow & styling
PrimaryRegionSelectionModel(US-only, cap 5),RegionPickerView(segmented Map | List — tappable US-state polygons + searchable list; List is the default and map geometry loads lazily),RegionCustomizeView/RegionAppearanceEditor(color/emoji/icon grids + liveRegionSummaryCardpreview),RegionsSettingsView.OnboardingViewis anintro → pick → customize → locationstate machine inNavigationStacks with toolbar controls; Settings reuses the same picker/customize views. The intro also offers Restore from a backup, which imports (.replace) and skips the manual steps to the location ask.@Environment(\.regionStyles)(aRegionStyleResolver), seeded bywhereBroadwayRoot(regionStyles:)— fromWhereSession(live on store changes), theWidgetSnapshotin the widget process, and services in App Intents. No globalregion.style; fallbacks come fromRegionAppearanceCatalog..other), instead of one flat catalog list (RegionSelectionStategrouping +ManualDayView).WidgetSnapshotcarries appearances so widgets render user choices;RegionPickerStylestylesheet group added.Notes / decisions
SDTrackedRegionsoft-delete TODO stays open (untracking still deletes the row, matching prior behavior).DayRelabelView) still uses the flat list; grouping it the same way is an easy follow-up.Testing
RegionStyleResolverresolution/fallback, widget-snapshot appearance round-trip,RegionPickerStyledefaults, backup archive + end-to-end store round-trips (incl. appearance), and the Log-a-day toggle grouping (partition, tracked-over-used-this-year precedence, toggle stability).Stuff-iOS-Testsscheme green locally (-destination 'platform=iOS Simulator,name=iPhone 17,OS=26.2');./swiftformat --lintclean.Docs
WhereCoreandWhereUIREADME + AGENTS; resolved theRegionStylebespoke-table TODO; refreshed the now-inaccuratesetTrackedRegion(false)comment (untracking is user-reachable now); added a WhereCore invariant that persisted-model changes must be reflected in backups end-to-end; added a root AGENTS rule to keep PR descriptions current.Review fixes folded in
Nav-stack/toolbar consistency, atomic single-transaction commit, environment-injected styling (replacing a global singleton; also makes edits/imports restyle reactively), restored
.otherglyph, capped-tap haptic + hint, lazy map-geometry parsing (List default), and a clamped customize step index.