refactor(lobby): consolidate server management + lobby polish#347
Merged
Conversation
Front the relative "last activity" time on both the list and grid room cards with a small muted clock icon, wrapped in a "Last activity" tooltip — so the recency reads at a glance without printing the words on every card. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Following a design review with Erik, consolidate server management and tidy the lobby: - Remove the dedicated /servers ServerListScreen (route, sidebar gear, onboarding "All servers" link, AppRoutes.servers, public-path entry). Server actions now live in a per-tile vertical-more menu on the sidebar: Sign in (disconnected) / Log out (connected + auth) / Remove. The menu reveals on hover and stays shown for the selected tile so it's reachable without a mouse; the slot is reserved so the title doesn't shift. - Extract the platform-conditional logout logic into a shared auth/server_logout.dart (logoutServer + friendlyLogoutError); the tile menu reads the auth providers and surfaces failures via SnackBar. - Drop the rooms-pane server-address heading — the selected server is named in the sidebar. - Drop the per-tile auth/identity status subtitle — identity lives in the account block, connection state in the menu. - Below the tablet (600) breakpoint, hide the list/grid toggle and force list view; the persisted choice is untouched and honoured again >=600. Tests: delete the two server_list_screen suites, rewire the auth/lobby suites, and add menu + hover-reveal coverage. Sidebar-tile tests now wrap in a ProviderScope since the menu is a ConsumerWidget. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Restore the auth-status signal removed in the previous commit as a small colored leading dot on each sidebar server tile: - green (success) — auth required and signed in; - red (danger) — auth required but not signed in (or expired); - neutral (onSurfaceVariant) — no authentication required. Derived from the per-entry session signal (no extra API call — /api/user_info only feeds the account-block name), so the dot lives in a Watch and updates on sign-in / expiry without a server-map mutation. The tooltip carries the same status as text for accessibility. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
91jaeminjo
force-pushed
the
polish/lobby-server-actions
branch
from
4aba73f to
76738ef
Compare
…rdance A failed log-out preserves the local session, so the failure needs to stay visible. Replace the transient SnackBar with a red error icon in the tile's trailing slot: its tooltip carries the message, and tapping it opens a dialog with the full text plus Try again / Dismiss. The dialog is the surface a touch user reads the error on, since hover tooltips aren't reachable there. Distinguish the remove path from a plain log-out in both the dialog title and the debug log — a remove-path failure keeps the server rather than removing it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Restate the section-label, status-dot, and activity-clock comments as present-tense design rationale, and remove the status-dot colour legend that duplicated the authoritative one on _StatusDot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Make logoutServer's platform branch a `web` seam (defaulting to kIsWeb) so the web and native orderings it exists to protect are both reachable in VM tests, then pin them: native clears local only after endSession returns and never pre-fetches discovery; web clears local before navigating and passes the discovered end_session_endpoint; a failed endSession (native) or discovery fetch (web) preserves the session. Add friendlyLogoutError formatting cases, including the generic no-type-name path. Cover the sidebar log-out success and the log-out-before-remove branch for a connected authenticated server. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the log-out failure dialog (Try again / Dismiss) with a popup menu on the tile — Try again / Show error detail / Remove server — so the trailing slot stays a menu in every state instead of switching to a dialog. "Remove server" makes a best-effort sign-out and removes the entry regardless of the outcome, so a server whose IdP log-out keeps failing can still be removed. Model the three post-log-out outcomes (keep / remove on success / remove regardless) as an `_AfterLogout` enum rather than a `removeAfter` bool. Log when a web sign-out finds no `end_session_endpoint`: local state is cleared but the IdP session can't be ended, so surface that partial log-out instead of returning silently. Tidy a few comments: drop temporal phrasing from the rooms pane and describe the web log-out as a full-page navigation rather than a sheet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exercise the log-out failure menu (Try again / Show error detail / Remove server), the remove-regardless escape hatch when sign-out keeps failing, and the in-flight gate that replaces the tile menu so a second action can't fire while a sign-out is outstanding. Assert that below the tablet breakpoint a persisted grid choice is forced to list and the view-mode toggle is hidden. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ging Switch on `_AfterLogout` in the tile menu's logout handler so adding a disposition is a compile error rather than a silent misclassification by an `==`/`!=` chain. Log the web "no end_session_endpoint" partial logout at warning level so the cleared-local-only outcome is visible in production. Describe the `_AfterLogout` enum by its present outcomes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Log at warning level when an active session has no id_token, since the RP-initiated logout then omits id_token_hint and the IdP may not end its session. Rename `_LogoutFailure.wasRemove` to `removalWasIntended` so it names what it means, and describe the error affordance as widget state that resets when the menu is disposed. Cover the web no-end_session_endpoint path: local is still cleared and endSession still runs with a null endpoint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
2 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.
Lobby polish from a design review with Erik. Stacked on
feat/onboarding(PR #346) since it builds on those commits.Changes
Room cards
Last activitytooltip, instead of printing the label on every card.Server management — one surface
/serversServerListScreen(route, sidebar gear, onboarding "All servers" link,AppRoutes.servers, public-path entry).auth/server_logout.dart(logoutServer+friendlyLogoutError); the tile menu reads the auth providers and surfaces failures inline on the tile (a persistent error icon, never a SnackBar).Lobby pane tidy-ups
Tests
server_list_screensuites; rewired the auth/lobby suites; added ⋮-menu + hover-reveal coverage. Sidebar-tile tests now wrap in aProviderScope(the menu is aConsumerWidget).flutter analyzeclean.Open question
The per-tile auth-status subtitle removal is pending a colleague's answer on whether to reinstate that indicator in some form — easy to dial back if so.
🤖 Generated with Claude Code