Efficiency & design review fixes (+ conference media)#46
Open
sethlaw wants to merge 18 commits into
Open
Conversation
Add an optional `media` array to the Conference model. Each entry carries dark and/or light variants of (banner_background, banner_logo, square_logo) so a conference can publish branding assets without forcing both appearances. Field names mirror the Firestore document (snake_case via CodingKeys). ConferenceRow now resolves `squareLogo(for: colorScheme)` and renders it via Kingfisher at 44pt before the name/dates. Falls back to the opposite appearance when only one is published, and renders nothing when the conference has no media — existing conferences keep their current row layout. Co-Authored-By: WOZCODE <contact@withwoz.com>
Firestore emits media as a dict ({dark: {...}, light: {...}}) which
failed to decode as [ConferenceMediaEntry]. Flatten to a single
ConferenceMediaEntry? holding optional dark/light variants.
Co-Authored-By: WOZCODE <contact@withwoz.com>
Render the conference's resolved square_logo via Kingfisher under the Home items and above the Contact Us / version footer. Picks the appearance-matching variant from Conference.media; no-ops when the conference has no media configured. Co-Authored-By: WOZCODE <contact@withwoz.com>
Wrap the InfoView square logo in a tap-to-collapse header (chevron + "Show/Hide conference logo"). State is persisted as a JSON-encoded Set<String> of conference codes under @AppStorage so the choice survives both conference switches and cold launches. Co-Authored-By: WOZCODE <contact@withwoz.com>
Restack the row so the conference name spans the full width on top (with the active checkmark trailing), then place the square logo to the left of the dates / timezone / status line below. Co-Authored-By: WOZCODE <contact@withwoz.com>
Move the square logo to the trailing edge of the dates/timezone row so it sits under the active checkmark with the date block expanding to fill the remaining width. Co-Authored-By: WOZCODE <contact@withwoz.com>
- InfoViewModel.removeListeners() had drifted from removeListenersImmediate() and was missing orgListener — every conference switch leaked a live Firestore org listener that could clobber the new conference's orgs with stale data. Delegate to removeListenersImmediate() so there is a single source of truth for the listener list. - downloadFileCompletionHandler never invoked its completion on non-200 responses or failed HTTPURLResponse casts, leaving pendingMapDownloads permanently incremented and the Maps tab spinner stuck for the session. All paths now call completion exactly once. - scheduleNotification omitted .year from its DateComponents, so a notify time already in the past silently scheduled an alert for the same date next year. Include the year and skip scheduling elapsed dates. - Remove dead .swipeActions on EventCell/ContentCell — they only function inside List, and these cells render in ScrollView/LazyVStack since the List removal; the visible bookmark button covers the action. Co-Authored-By: WOZCODE <contact@withwoz.com>
- Downsample the three KFImage sites that decoded full-resolution bitmaps: conference logos in ConferenceRow (56pt) and InfoView (160pt), org media in OrgView (300pt) — matching the Perf E pattern already used by OrgsView/ProductsView. - Rewrite [Event].search(text:speakers:) to use the shared _searchMatches range-based matcher instead of allocating lowercased copies of every title/description per keystroke. - Debounce GlobalSearchView with the same 200ms task(id:) mirror ProductsView uses, so the six-section global search re-filters once per typing pause instead of per keystroke. - ContentListView: compute the filter+group pipeline once per body pass (shared groupedContent property) instead of re-running it for the count and the section list separately. Co-Authored-By: WOZCODE <contact@withwoz.com>
…bookmarks - Replace the ToTop/ToBottom/ToCurrent/ToNext published-Bool objects with a single ScrollCommandBus (PassthroughSubject; ObservableObject with no @published so objectWillChange never fires). Senders call send(.top/.bottom/.current/.next); EventScrollView consumes via .onReceive. A jump tap no longer re-evaluates the schedule body at all — previously it cost two-to-four full pipeline recomputes. - Compute the schedule filter+search+group pipeline once per input change (.task(id:) keyed on everything the pipeline reads) and share the cached result across the scroll view, jump-to-day menus, and the filter sheet's matched count in both iPhone and iPad branches. The jump-to-day keys keep their filtered-but-unsearched semantics. - Hoist EventCell's per-row bookmark @fetchrequest into a BookmarkSnapshot environment value (mirrors noteEventIDs) published by EventsView / GlobalSearchView / InfoView's shared-schedule pane; bookmarkConflicts gains a precomputed-key overload so per-row calls stop re-hashing the bookmark array. - eventDayGroup pre-sorts each day's events; EventData drops its inline per-render sort and calls Date() once per pass. - ContentView: inline ScheduleView, delete dead view-in-@State and never-mutated .id(UUID) tab identities. Co-Authored-By: WOZCODE <contact@withwoz.com>
- fetchContent and fetchSpeakers now capture the snapshot docs, decode and rebuild events in a Task.detached, and hop back to the MainActor only for the final whole-array assignment. DEF CON-sized content (1000+ docs) no longer stalls the main thread on every snapshot tick (including the double cache+server tick at launch). - Per-fetcher generation counters discard stale decodes so a slow cache-tick result can't land after (and clobber) a newer server tick. - QueryDocumentSnapshot isn't Sendable until firebase-ios-sdk 11.3.0 and we pin 10.29.0, so the array crosses the detach boundary in a documented UncheckedSendableBox (snapshots are immutable and data(as:) is thread-safe); remove the box when the SDK is bumped. - Speakers and orgs are sorted before assignment, halving the didSet index rebuilds and observation invalidations per tick. - All twelve snapshot listener closures now capture [weak self], breaking the listener->closure->self retain cycle that made the deinit cleanup unreachable. Co-Authored-By: WOZCODE <contact@withwoz.com>
The inline search bar, search toggle button, and floating Top/Bottom jump menu were copy-pasted across nine list screens and had started to drift. New Views/ListChrome.swift holds InlineSearchBar, SearchToggleButton, and JumpMenu/JumpMenuOverlay; Conferences, Merch, Orgs, News, FAQs migrate fully, Speakers / All Content / Events keep their structurally different jump menus (alphabet groups, day list + command bus) but share the search chrome, SharedScheduleView shares the jump overlay. MapView's live in-document search is intentionally untouched. Net -442 lines. Co-Authored-By: WOZCODE <contact@withwoz.com>
- New `success` palette slot on every theme (green for Default/Hacker Green, the theme accent for Synthwave/DEF CON Red/Cyberpunk/Vegas) with a ThemeManager accessor; ConferenceRow and ThemePickerView's active checkmark + border stop hardcoding Color.green. LocationView's open/closed colors and CountdownView's ThemeColors.green are data-driven and intentionally untouched. - MerchInfo banner drops the danger background for cardSurface — it's informational, and danger red broke contrast in light mode. - AboutView adopts theme tokens: calloutFont/footnoteFont accessors on ThemeManager, themed accent button fills, cardSurface instead of systemGray5. Commit-hash monospace stays. - ThemeMetrics.cardRadius (10pt) replaces the scattered literal on the card sites that already used 10 (settingsCard, ConferenceRow, ThemePickerView, SpeakerRow, EventCell, ContentCell); InfoView's 15s and small-button 5s are documented as intentional outliers. Co-Authored-By: WOZCODE <contact@withwoz.com>
The app carried two theme systems: the legacy `Theme` ObservableObject (a colorScheme flag + an impure carousel() color cycler that mutated a shared index, so card tints depended on render order) alongside the @observable ThemeManager. Fold everything into ThemeManager: - colorScheme -> themeManager.preferredColorScheme (driven off the same "lightMode" AppStorage key). - carousel() -> pure carouselColor(index:) keyed on a stable per-card value (model id in lists, link url / fixed index for one-off header cards), plus a carouselColor(forKey:) overload for String-keyed models like Organization (@documentid). Colors are now stable per card instead of order-dependent. - Drop the `@EnvironmentObject var theme: Theme` from all 13 remaining views and the theme.index reset; the Theme class and its injection are gone. Co-Authored-By: WOZCODE <contact@withwoz.com>
The parsed-PDF cache was an unbounded process-lifetime dictionary that eagerly retained every map of every conference visited (maps prewarm adjacent pages), growing memory across a multi-conference session. Replace with a 12-entry LRU (reorder-on-read, evict-oldest-on-insert) — enough for a conference's maps plus prewarmed neighbors across a few conferences. Public subscript API unchanged; still @mainactor. Co-Authored-By: WOZCODE <contact@withwoz.com>
- summary(for:) is now a plain dictionary read; the SHA256 staleness hash moved into warm(), which runs once per cell materialization instead of on every re-render. Invalidation on a changed description is preserved (a stale summary can show for at most one warm cycle). - Persistence is debounced: writes mark the cache dirty and coalesce onto a 200ms task (matching the app's other debounce sites) with an explicit flush() for guaranteed writes, instead of re-encoding the whole cache to UserDefaults after every single summary. - The pending work queue is bounded at 64 (evict-oldest) so it can't grow without limit or retain stale description strings; the existing de-dup guard is unchanged. Co-Authored-By: WOZCODE <contact@withwoz.com>
23 @AppStorage keys were raw string literals across 27 files (colorMode alone in 15) with no compile-time protection — a typo silently forked a setting. Add Utils/AppStorageKeys.swift with a static constant per key (values byte-identical to the old literals so persisted settings are preserved) and migrate all 79 call sites to reference it. Three raw UserDefaults reads of the same keys (ThemeManager lightMode, DateFormatterUtility showLocaltime, CustomEventUtility notifyAt) now point at the registry too. Separate key namespaces (Filters store, Crashlytics, TalkSummaryCache) left alone. Co-Authored-By: WOZCODE <contact@withwoz.com>
Contributor
Author
Follow-up: maintainability findings from the review (phases 7–10)Pushed four more commits addressing the remaining verified findings, largest first:
Each built clean (iPhone 16 / iOS 18.2 sim) and swiftlint reported no new violations. Extra device-QA items for these: theme switching across all six themes (colorful-mode card tints now stable, not order-based), AI summary rendering + regeneration on edited descriptions, map navigation across multiple conferences, and a spot-check that persisted settings (light mode, notify time, filters) survive an upgrade install. |
Raise the SwiftPM requirement from 10.x to upToNextMajor 11.3.0 (resolves to 11.15.0). Firebase 11 removed the separate *Swift add-on products — their Swift/Codable APIs merged into the base modules: - FirebaseFirestoreSwift -> FirebaseFirestore (Codable data(as:) is now in the base module) - FirebaseInAppMessagingSwift-Beta -> FirebaseInAppMessaging-Beta - FirebaseAnalyticsSwift -> removed (the app links FirebaseAnalyticsWithoutAdIdSupport, which provides the same module) - FirebaseDatabaseSwift -> removed (unused; nothing imports it) With 11.3+ declaring QueryDocumentSnapshot Sendable, the Phase 3 UncheckedSendableBox workaround is gone — the snapshot docs array now crosses the Task.detached boundary directly. Co-Authored-By: WOZCODE <contact@withwoz.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
1 participant
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.
Summary
Fixes from a full six-lens efficiency/design review of the app (42 review+verify agents; every finding adversarially verified against the code before being acted on). Also carries the
conference-mediafeature branch this was cut from: optionalConference.media(dark/light banner + square logos), the square logo in the conference picker and Info screen (collapsible, per-conference memory), and the 6.2 version bump.Correctness fixes (016cc31)
removeListeners()had drifted fromremoveListenersImmediate()and skippedorgListener— every conference switch leaked a live Firestore listener that could clobber the new conference's orgs with stale data. Now delegates to the single source of truth.mapsLoadingon for the rest of the session. All paths now complete exactly once..year, so a past-dated notify time silently scheduled for the same date next year. Year included; elapsed dates skipped.List, which the app no longer uses).Performance
ScrollCommandBus(PassthroughSubject, zero invalidation) and a single cached pipeline recompute keyed on exactly the inputs it reads. Per-cell bookmark@FetchRequests hoisted into oneBookmarkSnapshotenvironment value; day groups pre-sorted once.[weak self].KFImagesites, allocation-free event search, debounced global search, deduplicated content grouping.Design / maintainability
Views/ListChrome.swift(net −442 lines).successpalette slot (ends hardcodedColor.greenon active states), MerchInfo banner de-danger-ed (fixes light-mode contrast), AboutView on theme typography/color tokens,ThemeMetrics.cardRadius.Conference media (186ed35…84e7d30)
Conference.mediadecodes the Firestore dict shape ({dark:{...}, light:{...}}) withbanner_background/banner_logo/square_logoper appearance and a fallback resolver.Test plan
🤖 Generated with Claude Code