Dedupe connection error bars into a single thin top banner#865
Merged
shanselman merged 1 commit intoJun 26, 2026
Merged
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 6:10 PM ET / 22:10 UTC. Summary Reproducibility: yes. at source/proof level: current main still defines AuthErrorBar and separately publishes connection:authentication-failed, and the inspected before screenshot shows both bars for one failure. I did not run the WinUI app locally because this was a read-only review. Review metrics: 4 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused banner consolidation after required checks finish green, preserving the existing Connection-page recovery card as the detailed troubleshooting surface. Do we have a high-confidence way to reproduce the issue? Yes, at source/proof level: current main still defines AuthErrorBar and separately publishes connection:authentication-failed, and the inspected before screenshot shows both bars for one failure. I did not run the WinUI app locally because this was a read-only review. Is this the best way to solve the issue? Yes: consolidating onto the existing snapshot-driven connection:issue banner and making actionable connection notifications win is the narrowest maintainable path I found. The existing recovery card still owns detailed repair guidance, while the banner reliably routes users to Connection. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9ac8cfb24f35. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
clawsweeper
Bot
added
proof: sufficient
bkudiess
force-pushed
the
bkudiess-dedupe-connection-info-bars
branch
from
19ad06d to
bd37f32
Compare
A connection/auth failure rendered two error bars: the global top-window InfoBar plus an in-page "Connection Error" InfoBar on the Connection page. The auth pipeline also published two global notifications for one failure, which forced the banner action to degrade to "Show more". Remove the in-page AuthErrorBar and consolidate to one top banner that: - carries the gateway/node error states the in-page bar covered - always routes the user to the Connection page via an "Open Connection" action (connection notifications are prioritized as the visible banner, preferring an actionable one over an action-less transient) - renders as a single thin line: bold headline + " - detail", with the action as a right-aligned hyperlink (Windows-aligned InfoBar) Other changes: - Stop publishing the duplicate connection:authentication-failed banner; the snapshot-driven connection:issue notification is the single source. - Re-home transient errors: WSL host-action failures use the inline card status (or a top notification when the card is hidden); connect/switch failures publish via ShowTransientConnectionError on the same banner id. - Remove now-dead resw strings (ConnectionPage_AuthGuidance*, ConnectionPage_ConnectFailed) across all five locales. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
shanselman
force-pushed
the
bkudiess-dedupe-connection-info-bars
branch
from
bd37f32 to
9ab3be8
Compare
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.
Summary
A connection/auth failure rendered two error bars: the global top-window InfoBar and an in-page "Connection Error" InfoBar on the Connection page. The auth pipeline also published two global notifications for a single failure, which forced the banner's action to degrade to "Show more" instead of routing to Connection.
This consolidates everything into one thin top banner and removes the in-page bar, with no loss of states/messages.
What changed
AuthErrorBar(XAML + all code paths) and its now-dead resw strings (ConnectionPage_AuthGuidance*,ConnectionPage_ConnectFailed) across all 5 locales.connection:authentication-failedbanner; the snapshot-drivenconnection:issuenotification is the single source.ShowTransientConnectionErroron the same banner id (cannot double-bank).Validation
./build.ps1— all projects buildScreenshots
Before: two bars (top "Gateway connection failed" + in-page "Connection Error").
After: one thin banner —
⊗ Gateway connection failed — Transport error … Open Connection ✕— with the Connection page showing only its own recovery card.