Feat/a11y wcag2 audit fixes - #11
Merged
Merged
Conversation
These were the last full-collection .get() calls without a limit; add a defense-in-depth .limit(1000) matching the cap used elsewhere. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
These filters are applied in-memory by CachedRepository.search() (load-all-then-filter-then-paginate), so cursor pagination works fine alongside them. Only the startDate/endDate path uses a separate Firestore query that can't accept a resumption cursor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
apiRequest() previously let a failed fetch (offline, API down) throw an unlabeled error; wrap it into a typed ApiError. The auth listener now retries getMe() with backoff before giving up (handles the brief window right after sign-in where the API may not be reachable yet), and only logs the user out on a genuine 401/403 rather than any error, surfacing the real message via authStore.setError(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
handleSignOut() called authStore.logout() alone, which only cleared local state without calling Firebase signOut() or navigating away, leaving the user on a protected page with a stale Firebase session. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ing costs Adds BillingCycle.rate_ema, an exponential moving average of billing_rate per meter group (gamma tuned per utility type via backtest), recomputed on cycle create/update over a causally-bounded range instead of the group's full history. Propagates into Billing.estimated_cost for still-uncycled billings so a rough cost is available before the official rate lands. Supporting changes: - CachedRepository.searchDirect() + Repository $arrayContains/ $arrayContainsAny filters, for correctness-critical bounded reads that bypass the in-memory list cache. - Property.main_meter_group_ids denormalized field, replacing full-collection scans for main-meter uniqueness/lookup with targeted array-contains queries. - Batched list-cache read-modify-write (listAppendMany/listUpdateMany/ listRemoveMany) to fix a race where concurrent single-item calls on the same cache key could drop entries. - New composite Firestore indexes and shared fetchSeedAndAffected() bounded-read helper backing the above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a "Pending Estimates" panel listing uncycled billings that already have a known consumption but no official cycle/rate yet, using the new rate_ema-derived Billing.estimated_cost. An optional toggle also surfaces the latest derived (main-meter) estimate per meter group. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cover the new rate_ema/estimated_cost flow across the API and UI CLAUDE guides, and bring README's feature-status tables up to date (several items marked partial/stub had since shipped). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keeps one-off migration/backfill scripts (e.g. backfill-rate-ema.ts) and their generated artifacts (billing_export.csv) out of git status noise without deleting them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ucture Adds screen-reader-friendly confirm dialogs, toast notifications, and session-expired detection to replace native window.confirm()/alert() calls that don't work well with assistive tech.
Replaces native confirm()/alert() with the new ConfirmSheet/Toast components, adds aria-live route announcements, focus-on-navigate for screen readers, and session-expired notices across all screens.
…tructure Adds a Promise-based confirmAsync() dialog and toast notifications to replace native window.confirm() calls, plus a nav-counts store for accessible badge announcements in the sidebar.
…UD pages Replaces native confirm()/alert() in the crud store, ChatWidget, and PhotoDropzone with confirmAsync()/toast, and wires the sidebar nav-counts store into the app layout for accessible badge updates.
- Extract a shared focusTrap Svelte action (ui + mobile) that traps Tab, handles Escape, restores focus on close, and optionally locks scroll — dedupes the byte-identical trapFocus() previously copy-pasted between ConfirmDialog and EditModal (finding #40). - Fix bg-opacity-50/90, a dead Tailwind v3 utility under this project's v4 setup, so ConfirmDialog/EditModal/ImagePreview backdrops actually dim instead of rendering fully opaque (finding #9). - Give ConfirmDialog focus-restore-on-close and the same modal-open scroll lock EditModal already had (finding #14). - Apply the same focus-trap pattern to mobile's ConfirmSheet, which had none despite declaring alertdialog/aria-modal (finding #16). - Queue confirmAsync() calls instead of silently cancelling a pending one when a second call arrives before it resolves (finding #6). Findings: #6, #9, #14, #16, #40 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- handleBatchDelete now uses Promise.allSettled instead of Promise.all, and unconditionally clears selection + reloads afterward, so a partial failure no longer leaves successfully-archived items stuck selected/visible; failed ids are now surfaced in the error message (finding #5). - properties/+page.svelte now renders {error || crud.error} so a failed archive (crud.error) actually shows feedback instead of looking identical to success (finding #2). - properties/+page.svelte: collapsed the duplicate fetch-trigger paths (an $effect and several explicit loadPropertyDetails() calls firing on the same selectedProperty/activeTab change) down to the $effect alone, and added a request-id staleness guard so a stale in-flight response can no longer overwrite newer data after fast property/tab switching (findings #13, #37). Findings: #2, #5, #13, #37 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Reset editData.previous_reading_id/current_reading_id when the edit- billing Property dropdown changes, so a PATCH can no longer submit reading IDs that belong to a different property (finding #3). - "Override readings per property" now looks up the property by the already-known d.propertyId instead of re-matching on room_name text, fixing wrong-property misapplication on name collisions (finding #4). - runDiscovery() gets the same cancelled-request-id guard already used elsewhere in this file, so fast meter-group/end-date switching can no longer apply a stale discovery response (finding #12). - Replaced the two leftover alert() calls with pushToast(), finishing the migration the rest of this file's success paths already use (finding #20). - The expanded-cycle Amount column now shows N/A alongside Consumption/ Current-Reading when the reading can't be resolved, instead of a confident-looking ₱ figure (finding #21), and computes via the existing billAmount() helper instead of inline multiplication, matching every other amount in this file (finding #30). - Replaced hand-rolled cycle-level selection/edit state and two ad hoc Promise.all archive flows with a second createCrudStore<BillingCycle>() — batch/single cycle archiving now goes through the same Promise.allSettled-based handleBatchDelete/handleSoftDelete as billings, instead of duplicating (and inheriting the partial-failure gap of) that logic (findings #22, #38, #39). Cycle/billing store errors are now both rendered alongside the page-level error. - Replaced 8 hand-rolled {_seconds,_nanoseconds} constructions in cycle create/update with the existing fromISOString() helper (finding #29 subset). Findings: #3, #4, #12, #20, #21, #22, #29, #30, #38, #39 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Extract a shared compressAndSuggest() helper for the batch/manual photo-upload handlers, closing two races in the same move: a stale OCR response can no longer land on a freshly-reset manual form (finding #7 — closed via a per-form generation counter bumped on every reset) or on the wrong batch row after a meter-group change reassigns batchRows mid-upload (finding #8 — closed via stable per-row property-id identity checks instead of array index). Also removes the batch/manual duplication itself (finding #33). - Split the shared selectedMeterGroup into a filter-only state and a new batchMeterGroup for the create-reading form — picking one no longer silently desyncs the other's displayed value (finding #19). - Reworked the reload plumbing accordingly: the filter's meter-group select now only re-applies filters, the batch form's select only reloads batch properties — previously one selector's onchange did both, which was the root cause of #19. - Applied the same local-midnight date parsing already used for the batch-date preview to the actual submitted reading_date on both the batch and manual forms, instead of the UTC-parsing new Date(dateString) the adjacent comment warned against (finding #18), and routed both through the existing toTimestamp() helper (finding #29 subset). - shouldSeedReading() now paginates through all historical readings for a property/meter-group pair instead of checking only the first capped page of 100 (finding #25). - Split the single shared `error` string into `error` (table/filters), `batchFormError`, and `manualFormError` so a background operation in one form can no longer mask an error relevant to a different one (finding #26). - Added missing scope="col" to the readings table headers that lacked it (finding #24). Findings: #7, #8, #18, #19, #24, #25, #26, #29, #33 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Extract getMeterGroupId()/isMainMeterEntry() into a new property.util.ts and use them at every typeof entry === 'string' ? entry : entry.meter_group_id (and the paired is_main_meter unwrap) call site in properties/+page.svelte — the most duplicated logic in this batch, spanning loadPropertyDetails, openEditModal, the property list's "Main" badge, and the selected-property header (finding #31, folds in #36). - Extract the ~90-line duplicated meter-group-select + main-meter- checkbox-with-warning block (present in both the create and edit property forms) into a shared PropertyMeterGroupFields component, mirroring how PhotoDropzone is already shared between the readings tabs (finding #35). Findings: #31, #35, #36 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- nav-counts.svelte.ts: add invalidateNavCounts() and call it from the shared crud store's soft-delete/batch-delete handlers, so sidebar badge counts refresh after an archive instead of being stuck at their first post-login snapshot for the rest of the session (finding #11). - TopBar.svelte: wrap displayBreadcrumbs in $derived — it reads $page.url.pathname but was computed once at component init, and TopBar persists across client-side nav, so breadcrumbs never updated after the first render (finding #15). - Sidebar.svelte: use $authStore directly instead of manually mirroring it into local $state via $effect + .subscribe() — the native auto-subscription already works, as settings/+page.svelte already showed (finding #41). Also inlines the zero-logic getInitialsFromName() wrapper at its one call site (finding #44). - Introduce shared --sidebar-width/--topbar-height CSS variables in layout.css and reference them from Sidebar/TopBar/RightPanel and the (app) layout's inline offsets, replacing 5 independent hardcoded 200px/52px values that had no shared source of truth (finding #43). - SelectionToolbar.svelte: fix naive `${label}s` pluralization ("2 propertys selected" → "2 properties selected") for every consonant+y entityLabel currently in use (finding #23). - ActionButtons.svelte: delete the unused generic `actions` render path — all 4 call sites only ever use onEdit/onSoftDelete (finding #42). Findings: #11, #15, #23, #41, #42, #43, #44 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- BaseModel.created_at/updated_at retyped from FirestoreTimestamp to string, and is_deleted/deleted_at dropped entirely — the API's global JSON replacer already converts timestamps to ISO strings and unconditionally strips is_deleted/deleted_at before a response reaches the client, so the old types actively lied about the wire contract (finding #10). Updated the one place that built BaseModel- shaped mocks directly (true-reading.test.ts) to match; all runtime code was already using toDate()'s dual string/object handling defensively, so behavior is unchanged. - Removed deleteBillingCycle() (finding #27) and formatKwh() (finding #49) — both zero call sites, both superseded by softDeleteBillingCycle()/formatReading() respectively. - UpdateMeterGroupRequest/UpdatePropertyRequest/UpdateBillingCycleRequest are now Partial<Create*Request> instead of hand-duplicated mirrors, so a field added to one now propagates automatically (finding #47). tenant/billing/reading Update*Request types were left alone — those genuinely diverge from their Create* counterparts. - CreateSeedReadingRequest is now a type alias for the byte-identical CreateReadingRequest instead of a duplicate (finding #48). Findings: #10, #27, #47, #48, #49 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Extract createCrudApi<T, CreateReq, UpdateReq, Params, BatchCreateResp>() (get/getById/create/createBatch/update/updateBatch/softDelete/restore/ clearCache against apiGet/apiPost/apiPatch/apiDelete + toQueryString), precedented by stores/list-store.svelte.ts's existing read-side factory. meter-groups.ts/properties.ts/tenants.ts/readings.ts/ billings.ts/billing-cycles.ts now delegate to it internally while keeping their exact existing exported function names/signatures (recordMeterGroupReset, ocrReadingImage, getReadingsByIds, etc. stay as module-specific additions) — no call site elsewhere in the app needed to change (finding #45). - cache.ts now imports and calls each module's own clearCache() instead of re-deriving the same 6 endpoint URLs independently — one source of truth for those paths instead of two (finding #46). - client.ts: extract a doFetch(headers) closure shared by the initial request and the 401-retry, so the AbortController/timeout/error-shape handling can't silently drift between the two copies it used to have (finding #50). auth.svelte.ts's writable-store-to-runes migration (finding #51) was left as-is per the plan — lowest priority, already-working code, not worth the risk in the broadest-touching group of this pass. Verified with npm run check (0 errors), npm run test:unit (24/24 passing), and a full npm run build. Findings: #45, #46, #50 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- ReadingHistory.svelte: the property filter's $effect read selectedPropertyId inside its own conditional body, making Svelte track it as a dependency alongside utilityFilter — every property selection re-triggered the effect, which unconditionally reset selectedPropertyId right back to '', making the dropdown completely unusable. Now tracks the previous utilityFilter value explicitly via a plain (non-$state) closure variable, initialized lazily on the effect's first run, instead of relying on implicit dependency tracking to tell "utility changed" apart from "property changed" (finding #1). - CaptureReadings.svelte: wrap the step indicator in aria-live="polite" and move focus to the step heading on every step transition (an $effect keyed on `step`), not just initial mount (finding #17). - Settings.svelte/CaptureReadings.svelte: finish the getErrorMessage() migration Billings.svelte already had (findings #28, #53), and extract a shared <ErrorBanner> component used by all 5 screens that render one, closing the drift where Settings had grown a dismiss button the other four never got (finding #52). - ReadingHistory.svelte: batch the three independent onMount fetches with Promise.all instead of sequential awaits, matching the pattern Billings.svelte/Home.svelte already used (finding #54), and fix availableProperties from `$derived(() => ...)` (called as a function) to the correct `$derived.by(() => ...)` primitive (finding #57). - mobile/lib/api/client.ts: add a shared buildQueryString() helper and use it in readings.ts/billings.ts/billing-cycles.ts instead of each hand-writing the same URLSearchParams boilerplate (finding #55), and make apiGet/apiPost/apiPatch properly generic so billing-cycles.ts's apiGet<BillingCyclesResponse>(path) call (previously unsupported by client.ts's signature) actually works as typed, and readings.ts/ billings.ts pick up matching response types (finding #58). - CaptureReadings.svelte: replace the hand-rolled resultBanner (a second, already-diverging copy of Toast.svelte's hex color palette) with pushToast(), matching Billings.svelte/Settings.svelte (finding #56). Verified with npm run build (clean, no compiler warnings). Mobile has no typecheck/lint script in this session (tsc --noEmit fails on a pre-existing tsconfig `--ignoreDeprecations` incompatibility, confirmed present before this branch's changes) — build is the available gate. Findings: #1, #17, #28, #52, #53, #54, #55, #56, #57, #58 (decisions/20260728_ui-mobile-codebase-review-findings.md) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
No description provided.