Fix stuck 'Searching Spotify' notification and silent library-fetch failures - #1023
Open
FlatterAtMainz wants to merge 8 commits into
Open
Fix stuck 'Searching Spotify' notification and silent library-fetch failures#1023FlatterAtMainz wants to merge 8 commits into
FlatterAtMainz wants to merge 8 commits into
Conversation
.output-control__inner capped its height with max-height on mobile but used overflow-y: visible, so a snapserver device/group list taller than the popup just overflowed off-screen with no way to scroll to it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
max-height: calc(100vh - 140px) sizes the popup using the full layout viewport, which on mobile includes space the browser's address bar can occupy. When the address bar is visible, the actually-visible viewport is shorter than 100vh, so the popup (anchored to the bottom, growing upward) extends above what's visible, hiding its top row. Add a 100dvh override, which tracks the real visible viewport, after the 100vh fallback for browsers that don't support dvh. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
On mobile the popup only auto-opens via PlaybackControls expanding (force_expanded=true), which also hides the popup's own close button (display:none in CSS) and suppressed its own click-outside overlay. That left no way to dismiss just the output-control popup - only tapping in the empty space above it (collapsing the whole playback bar) worked. Always render the click-outside overlay regardless of force_expanded so the popup can close independently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Errors surfaced through handleException often ended up either blank or as a bare technical fragment (e.g. "Could not load your profile" with no explanation, or a raw "SyntaxError: JSON.parse: unexpected end of input"). Add a humanizeError util that recognises common failure shapes (invalid/empty JSON responses, network failures, expired/revoked OAuth grants, known HTTP status codes) and prefixes the description with a plain-language explanation, applied centrally in the HANDLE_EXCEPTION handler so every existing call site benefits. Also: - handleException now falls back to a plain Error object's .message for the description, and no longer throws when data.error is undefined (message derivation had the same unguarded access). - Wrap the JSON.parse of xhr.responseText in HANDLE_EXCEPTION in a try/catch - a non-JSON error response would otherwise throw inside the exception handler itself. - Spotify getMe() and refreshToken() failures now get specific messages pointing at expired/revoked authorization, since that was the actual root cause behind the vague errors we hit in practice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… optional chaining - Humanized error explanations now live under errors.* in en.yaml (via i18n()) instead of being hardcoded in JS, so they're translatable like the rest of the UI. - Replace `x && x.y` null checks with optional chaining per review feedback.
…ailures getSearchResults never resolved its process notification on request failure or on cancel-button click, leaving it stuck in the UI forever. The five Spotify library-fetch actions had the same underlying gap (no rejection handler at all), silently swallowing errors and leaking stuck process state on any failed request.
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
getSearchResultsstarts a "Searching Spotify" process notification (uiActions.startProcess) but never resolved it on request failure or on cancel-button click — only the success path calledprocessFinished. Since a process notification only auto-closes in response toPROCESS_FINISHED/PROCESS_CANCELLED, a failed search (network blip, rate limit, expired token) left it stuck inrunningforever, and clicking the notification's × just flipped it to a dead-endcancellingstate with no way out (nothing ever calledprocessCancelledfor this action).processFinishedon the error path, and checking for acancellingstatus in the success handler (matching the pattern already used correctly elsewhere in this file, e.g.getLibraryMoods).getLibraryMoods,getLibraryPlaylists,getLibraryAlbums,getLibraryArtists,getLibraryTracks). These run withnotification: falseso the bug wasn't visibly stuck in the UI, but a failed request silently leftrunningprocess state in Redux forever and the promise rejection was completely unhandled (not even surfaced as an error notification).Test plan