feat(popup): extract connectivity-error-to-popup classification into a shared hook - #27
Merged
Merged
Conversation
…a shared hook grow-frontend and hear-frontend each carry a byte-for-byte copy of this classification logic in their own AppContent.tsx, including the same bug (an AuthRequired error on a route that doesn't require auth was silently dropped instead of surfacing a popup). Centralizing it here means that class of bug only needs fixing once; apps still own their own popup components via the renderers they pass in.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The transport barrel re-exports fetch-wrapper, useFetchWrapper, site-urls, and query-client too; pulling it in dragged those largely-untested modules into vitest's all:false coverage set and tripped the repo's 80% threshold.
Function coverage was failing CI (55.55% < 80%) because the prior tests only exercised the AuthRequired branches, leaving the hook's other renderer branches and most app-error classes' constructors unexercised.
Functions coverage was still short (77.77% < 80%): the two onClose callbacks passed to the Spotify/Google auth-error renderers were never invoked, and importing app-error.ts directly (instead of via a barrel) brought its previously-untested branches (InvalidInputError.toString(), BackendError's backendMessage override, BackendSpotifyUserNotAllowlistedError, getSpotifyAllowlistMailtoHref) into the coverage-counted set.
vi.mocked() casts the renderer mocks so direct .mock access type-checks.
Andreas-Garcia
added a commit
that referenced
this pull request
Aug 15, 2026
Adds the shared useConnectivityErrorPopup hook (#27).
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
grow-the-music-tree-frontendandhear-the-music-tree-frontendeach carry a byte-for-byte duplicate of the connectivity-error → popup classification logic in their ownAppContent.tsx, including the same bug: anAuthRequirederror on a route that doesn't require auth (e.g. a public reference page) was silently dropped instead of surfacing a popup (fixed one-off in grow-frontend PR #123).useConnectivityErrorPopuptoapp-kit'spopupsubpath: owns the error-type/route-requirement classification and the redisplay-dedup logic, while apps supply their own branded popup components via arenderersobject (renderAuthPopup,renderInternalErrorPopup, etc.) — so this fix (and any future one) only needs to land once.AppContent.tsxto consume this hook once a newapp-kitversion is published.Test plan
useConnectivityErrorPopup.test.tsxcovers: AuthRequired on an auth-required route →renderAuthPopup; AuthRequired on a route that doesn't require auth →renderInternalErrorPopup(the regression case); popup hides once the connectivity error clears.🤖 Generated with Claude Code