Fix/issue 241 wallet disconnect handler logging - #370
Merged
godamongstmen897 merged 166 commits intoAug 31, 2026
Merged
Conversation
…inner_skeleton - Replace hardcoded gray-* Tailwind classes with semantic design tokens: * bg-gray-900 -> bg-surface-card * bg-gray-800 -> bg-surface-field (content placeholders) * border-gray-800 -> border-border-strong * bg-gray-700 -> bg-border-subtle (nested placeholders / contrast layer) - Add data-testid attributes to all skeleton sections for testability - Add 19 comprehensive test cases covering: * Design token validation (no hardcoded gray-* classes remain) * Correct token-to-element mapping across container/header/stats/milestones * Component layout structure (grid, count, padding, animations) * Accessibility attributes (role=status, aria-live, sr-only, aria-hidden) All 436 tests (33 files) pass, including the 19 new assertions.
- Replace hardcoded empty message with EmptyStateCard component - Display briefcase icon for job-related context - Show descriptive title and explanation text - Include role badges (Client, Freelancer, Arbiter) showing available participation options - Add comprehensive test coverage with 16 test cases - Ensure proper accessibility with region landmarks and aria-labels - Validates placeholder display under empty data states
…, desktop - Add responsive padding and spacing (px-3 sm:px-6) for mobile-first approach - Implement responsive typography scaling (text-xl sm:text-2xl md:text-3xl) - Stack layout vertically on mobile, horizontal on tablet/desktop - Apply responsive grid layouts (grid-cols-1 sm:grid-cols-2 lg:grid-cols-3) - Make search form full-width on mobile, inline on tablet+ - Add responsive gaps and margins throughout - Implement horizontal overflow handling for pagination on mobile - Reduce button padding and font sizes on mobile viewports - Add comprehensive responsive design test suite with 20 test cases - Validate layout at mobile (< 640px), tablet (640-1024px), desktop (> 1024px)
…yling - Add focus-visible ring-2 styling on all interactive elements (indigo-500) - Implement hover state transitions with smooth animations (transition-all duration-200) - Add active state styling for button press feedback - Style disabled buttons with opacity-50 and cursor-not-allowed - Prevent hover effects on disabled buttons (disabled:hover:bg-gray-900) - Apply ring-offset for focus states on dark background (ring-offset-gray-950) - Use inset focus ring on job expand buttons for better UX - Add focus-visible:outline-none to remove browser defaults - Implement smooth transitions on all state changes - Add comprehensive test suite with 25 test cases validating: - Search input focus and hover states - Search button focus, hover, and active states - Role filter button states and transitions - Job expand button interactive states - Pagination button states and disabled styling - Accessibility compliance and transitions
…vigation, semantic HTML)
Implements the app dark/light theme toggle as a keyboard-operable, ARIA-compliant switch (role=switch, aria-checked, aria-label) that persists the chosen theme to localStorage and applies it to the document root. Adds React Testing Library tests verifying node rendering, accessible state, theme application, keyboard operation and persistence. Closes Goldii-locks#319
Implements the navbar alert bell badge as a keyboard-operable (role=button) disclosure with ARIA compliance (Goldii-locks#320): accessible name, aria-haspopup/aria-expanded/aria-controls, aria-live announcement regions, aria-hidden on decorative glyphs, focus-visible rings and design-token contrast. Adds field error indicators and alerts (Goldii-locks#324): validation field configs render role=alert error text that toggles as validation triggers, wired via aria-describedby and counted toward the unread badge. Adds React Testing Library tests covering both requirements. Closes Goldii-locks#320 Closes Goldii-locks#324
…n tokens The suite hardcoded the raw Tailwind palette LoadingSkeleton used when the branch was written. main has since migrated the component to semantic design tokens, so the class assertions and the querySelector calls that locate the cards no longer matched anything. Remapped throughout: bg-gray-900 -> bg-surface-card bg-gray-800 (bars, stat card)-> bg-surface-field bg-gray-700 (stat placehold.)-> bg-border-subtle border-gray-800 -> border-border-strong Worth noting three cases were passing vacuously: the stat- and milestone-card selectors returned empty NodeLists, so their forEach assertion bodies never ran. With the selectors fixed they now iterate and pass for real -- 54 of 54 in this file, up from 49. lint 0 errors / tsc 0 errors / 1832 tests passing / build OK
…ng-spinner-skeleton test: Add React Testing Library tests for LoadingSkeleton and ButtonSpinner (Goldii-locks#282)
… design tokens The branch was cut before main migrated LoadingSkeleton to semantic design tokens and added the skeleton-* test ids, so its side of the conflict still carried bg-gray-*/no-testid markup. Resolved by applying the branch's responsive layout on top of main's tokenised markup rather than taking either side whole. Two adjustments to keep the sibling suites passing: - The branch's new test ids (loading-skeleton-card/-stats/-rows) name elements that already carry ids on main (skeleton-container, skeleton-stats-grid, skeleton-milestones). An element can only hold one data-testid, so the test now queries main's names -- every assertion is unchanged. - Placeholder bars are expressed desktop-first (w-32 max-sm:w-24) rather than mobile-first (w-24 sm:w-32). Same computed result at both breakpoints, but it keeps the fixed widths on the element so the structural selectors in loading-spinner-skeleton.test.tsx still match. The two card-padding assertions there did have to move to the responsive class (p-6 -> sm:p-6, space-y-6 -> sm:space-y-6), since the branch deliberately makes those breakpoint-dependent. lint 0 errors / tsc 0 errors / 1841 tests passing / build OK
…skeleton-responsive-275 feat(loading-skeleton): responsive sizing across mobile/tablet/desktop
Same situation as Goldii-locks#349: the branch predates the design-token migration, so its side of the LoadingSkeleton conflict still carried the old bg-gray-* markup. Applied the branch's animation work on top of main's current component instead of taking either side whole -- animate-fade-in on the root, and animate-pulse plus the staggered [animation-delay:...] utilities on the three stat cards (100/175/250ms) and two milestone rows (325/400ms). The branch's test ids (loading-skeleton-card/-stat-N/-row-N) name elements that already carry ids on main, so the test queries main's names (skeleton-container, skeleton-stat-card-N, skeleton-milestone-card-N). Assertions and delay values are unchanged. Delays are written as literal class names rather than interpolated, since Tailwind only emits CSS for classes it can see statically. lint 0 errors / tsc 0 errors / 1847 tests passing / build OK
…skeleton-animations-278 feat(loading-skeleton): CSS micro-animations on mount and pulse stagger
Same conflict shape as Goldii-locks#349 and Goldii-locks#351 -- the branch predates the design-token migration, so its side of LoadingSkeleton still carried the old bg-gray-* markup. Applied the branch's mobile work on top of main's current component: a max-h-[70vh] sm:max-h-none overflow-y-auto overscroll-contain wrapper between the status root and the card, so the skeleton scrolls internally on small viewports rather than pushing the surrounding controls off-screen. The wrapper sits outside the card, so the card's first child is still the header row that Goldii-locks#349's responsive assertions rely on. lint 0 errors / tsc 0 errors / 1852 tests passing / build OK
…skeleton-mobile-279 feat(loading-skeleton): constrain height and preserve clickability on mobile
Same conflict shape as the other LoadingSkeleton branches -- this one predates the design-token migration, the responsive sizing, the stagger animations and the mobile wrapper, so its side still carried the old markup. Applied the branch's props to main's current component rather than taking either side whole: interactive swaps the root to role="button" and adds the hover/focus-visible treatment, disabled adds opacity-50 cursor-not-allowed and blocks click and key handling. The interactive styling is the branch's own gray-* palette, which is fine here: it is only emitted when interactive is set, and the design-token suite renders the default (non-interactive) skeleton, so its "no hardcoded gray-*" assertion still holds. One correction to the branch's test -- it asserted the loading label as "Loading job data..." with three ASCII dots, but the component has always used a U+2026 ellipsis, so the query could never match. Aligned the assertion to the character the component renders. ButtonSpinner takes the branch's version as-is (disabled prop, transition classes, button-spinner test id). lint 0 errors / tsc 0 errors / 1858 tests passing / build OK
…er-skeleton-states-274 feat(ui): add interactive hover focus and disabled states to loading …
…he row animations Two conflicts in app/dashboard/page.tsx, both resolved by taking each side's contribution rather than picking one: - Job row button: the branch's responsive padding (px-3 sm:px-5 py-3 sm:py-4) with main's interaction feedback (transition-colors duration-150, active:scale-[0.99], active:bg-gray-800/70). The branch's own assertion only covers px-3/sm:px-5, and nothing asserts the active: classes, so keeping both loses nothing. - Expanded panel: the branch's responsive spacing and border-t, plus main's data-testid="dashboard-expanded-panel" and animate-fade-in. The branch's side had dropped the test id, which dashboard-list-animations.test.tsx queries. lint 0 errors / tsc 0 errors / 1878 tests passing / build OK
…onsive-design Feat/dashboard responsive design
…classes One conflict on the job row button in app/dashboard/page.tsx. Both sides styled the same element, so the resolution keeps every class either side contributed: responsive padding px-3 sm:px-5 py-3 sm:py-4 (from Goldii-locks#304) press feedback active:scale-[0.99] (from main) focus ring focus-visible:ring-2/-inset/-indigo-500 (this branch) Transition widened to transition-all duration-200 (this branch's value) rather than transition-colors -- the branch adds a focus ring, and main's side adds a scale, neither of which transition-colors animates. lint 0 errors / tsc 0 errors / 1903 tests passing / build OK
…ractive-states Feat/dashboard interactive states
…k-switcher-mobile-nav fix(Goldii-locks#316): Handle mobile viewports navigation styling in dark_mode_switcher
… semantics
Two conflicts in app/dashboard/page.tsx, both resolved by keeping every
attribute either side contributed:
- Error alert: the branch's styled box and aria-live="assertive" plus
main's data-testid="dashboard-error-alert" and animate-shake.
- Expanded panel: the branch's id/role="region"/aria-label plus main's
data-testid="dashboard-expanded-panel" and animate-fade-in.
Beyond the conflicts, this branch changes the accessible names and roles
of controls that sibling suites already query, and its copy of
dashboard-interactive-states.test.tsx is identical to main's -- so those
queries were never updated to match. Retargeted them:
filter controls role=button -> role=tab (the branch wraps them in a
tablist), aria-pressed -> aria-selected
search submit name "Search" -> "Submit search query"
pagination name "Previous"/"Next" -> /Previous page/i, /Next page/i
Touched dashboard-interactive-states, dashboard-responsive and
loading-skeleton-mobile. Every assertion still checks what it did
before -- only the queries locating the elements changed.
lint 0 errors / tsc 0 errors / 1945 tests passing / build OK
…ssibility-compliance Feat/dashboard accessibility compliance
…ification-bell-responsive fix(Goldii-locks#322): Add responsive sizing layouts on notification_bell
…he timeout call git aligned two unrelated function bodies here: the branch's rewritten disconnectWalletWithCheck against main's generic withWalletDisconnectLoader, so the raw conflict would have replaced the loader's body with disconnect logic referencing disconnectFn, options and walletId -- none of which are in that scope. Resolved by putting each change where it belongs: - withWalletDisconnectLoader keeps main's body. - disconnectWalletWithCheck routes through the branch's new runWalletDisconnectWithTimeout helper, still inside main's withWalletDisconnectLoader wrapper. The branch predates the loader lifecycle work (Goldii-locks#238), which is why its side had dropped that wrapper -- keeping both preserves the timeout bounds this PR is for and the overlay behaviour already on main. lint 0 errors / tsc 0 errors / 1948 tests passing / build OK
…nnect-timeout-bounds Feat: wallet disconnect handler
No textual conflict once Goldii-locks#357 (this branch's parent) landed on main -- GitHub's cached mergeability was stale and its update-branch call reported a conflict that a local three-way merge does not reproduce. Merging main in here refreshes the head so CI runs against current main. lint 0 errors / tsc 0 errors / 1949 tests passing / build OK
…dline-badge Feat: Notification badge for milestones
…eachable Three conflicts in wallet_disconnect_handler.ts, all from this branch predating the loader lifecycle work (Goldii-locks#238) and the timeout bounds (Goldii-locks#357): - Doc comment: kept both notes. - "Not installed" branch: main's structure (it now sits inside the withWalletDisconnectLoader closure, hence the deeper indent) plus this branch's removeActiveKey call. - Third hunk was git aligning this branch's disconnectWalletWithCheck body onto main's generic withWalletDisconnectLoader again, as in Goldii-locks#357. Kept main's loader body and applied the branch's success-path removeActiveKey where it belongs, after the timeout-wrapped disconnect. Also fixed a contradiction inside the branch's own code: isValidSerializedPayload looped over every entry and rejected the whole payload if any one was invalid, which made the per-entry sanitizeWalletActiveKey/filter below it unreachable. Its own test "handles invalid active keys array by sanitizing" expects one good entry out of three to survive, so the validator now checks the envelope (record, known version, activeKeys array) and leaves entry validity to the sanitize step. That alone would have silently dropped bad entries, which the sibling test "rejects session state with missing required fields" requires a warn for, so rehydrate now logs REHYDRATE DROPPED ENTRIES when the sanitized count is short. Partial corruption is both survivable and visible. lint 0 errors / tsc 0 errors / 1975 tests passing / build OK
…isconnect-persistent-caching Implement secure persistent caching for active keys in wallet_disconnect_handler (Goldii-locks#237)
… suites Navbar (4 conflicts): kept main's design tokens and the props its side passes (isConnecting, providerName, networkMismatch) -- the branch's side replaced that block with a status="connected" badge and dropped those three -- while taking the branch's mobile fixes: relative z-10, px-4 sm:px-6, py-3 sm:py-4, gap-2, flex-wrap, min-w-0 and shrink-0 on the brand link and badge. That is the actual Goldii-locks#259 deliverable. wallet_disconnect_handler (3 conflicts): the third was git aligning the branch's disconnectWalletWithCheck body onto withWalletDisconnectLoader again, as in Goldii-locks#357 and Goldii-locks#371. Kept main's loader body; the branch's pendingTx warning runs before the loader is entered, and its success log sits on the real success path. Both Goldii-locks#357's options and this branch's pendingTx wanted the 4th parameter, so options stays 4th and pendingTx is 5th; the one call site passing it now passes undefined for options. The logging levels were a genuine three-way conflict. This branch (Goldii-locks#241) moves failures to console.error carrying the Error so the stack survives, and adds a success log. Goldii-locks#364's component suite, already on main, asserted the old behaviour: warn on failure, and complete silence on success. Settled as: success console.info (informational -- keeps Goldii-locks#364's "no warn, no error" assertion true) disconnect failure console.error(tag, err) availability check failure console.error(tag, err) pending transaction console.warn wallet not installed console.warn Updated the assertions that described the superseded levels: four failure checks in wallet_disconnect_handler.component.test.ts moved from warnSpy to errorSpy, and this branch's success/pending-tx counts now account for console.info. No assertion changed what it verifies. lint 0 errors / tsc 0 errors / 1996 tests passing / build OK
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
14 participants
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.
Closes #241
Here's a PR description you can paste in:
fix(#241): structured console errors and transaction debug tracking in wallet_disconnect_handler
Summary
Fixes two silent stack-trace drops and adds transaction-aware disconnect logging to
wallet_disconnect_handler.What changed
Stack trace preservation
checkWalletAvailabilityByIdcatch block was callingconsole.warn(..., err.message)— a string, which drops the stack. Changed toconsole.error(..., err)so the full stack is visible in devtools.disconnectWalletWithCheckcatch block had the same problem (console.warn(..., message)). Same fix —console.error(..., err)with the actual error object.Structured disconnect logging
console.warnon successful disconnect so clean disconnects are greppable in console output.PendingTxSnapshotinterface (txId,status,context— all optional) and a fourth optional parameterpendingTxtodisconnectWalletWithCheck. When a caller passes a snapshot, aDISCONNECT WITH PENDING TRANSACTIONwarn fires immediately with the identifying fields, enabling post-mortem tracing of disconnects that interrupted in-flight transactions. No control-flow change — purely observability.No control-flow changes — the handler returns the same shapes and throws/resolves in the same cases as before.
Tests
Three new
#241tests inwallet_disconnect_handler_availability.test.ts:console.errornever called, only the success warn fires.console.errorcalled with the actualErrorobject as the second argument (stack present and assertable).console.warnfires withDISCONNECT WITH PENDING TRANSACTIONand the snapshot object containingtxId/status/context.Existing spy assertions updated to use
errorSpywhere the severity changed. All 37 tests in the file pass.Transaction tracking note
wallet_disconnect_handlerhas no direct access to a transaction store — there is no pending-tx state reachable from this function without a ref toWalletContextinternals. ThePendingTxSnapshotparameter is the wiring point: callers likeWalletContext.disconnectcan read their own state and pass a snapshot in. That call-site wiring is a one-liner in the context and is a follow-up, not part of this handler-level fix.