feat: give the app a way to reach its own help — a ? chip and F1 - #2351
Merged
Merged
Conversation
The help was already written. About carries the version to quote, the release notes, a "Report a problem" button that pre-fills version and elevation, an "Ask a question" button that deep-links the Q&A category, the changelog and the repo. None of it was findable: About is the LAST child of grp-info, the 11th of 12 sidebar groups, and every group except Cleanup starts collapsed. So a confused user had to expand a group they had no reason to open (#1640). Two ways in, one route: - a 28x28 ? chip in the sidebar footer beside the Appearance chip, copied from it verbatim including the accessibility a Border needs and a Button would only partly supply — a UIA name, a tab stop, keyboard focus, and Enter/Space, since ButtonBase honours Enter only where KeyboardNavigation.AcceptsReturn is set; - F1, from anywhere. The chip goes through ShellAcceleratorCommand(Key.F1) rather than executing OpenAboutTabCommand itself, so the chip and the key cannot drift into meaning two different things — one of them going stale after a rename is otherwise invisible until someone tries the one nobody tested. ShellAcceleratorCommand is deliberately NOT a case inside AcceleratorCommand. That method returns null for a tab whose content is not built, which is correct for F5 and Escape — there is nothing to refresh or cancel on a tab nobody has opened — and would have made F1 silent on the first frame, exactly when a lost user reaches for it. It is also checked BEFORE the per-tab branch in the handler, because that branch returns early for any key that is not Escape or F5. Scope note: the issue also proposed re-opening a Welcome overlay from the same chip. That overlay is #1635 and does not exist, so this is the reachability half only. The issue's other premise is already stale — it says the repo hyperlink is "the only outbound documentation link in the entire UI", and About has had Report a problem, Ask a question, View license and Changelog for some time. What was missing was never the content. Two guards, and the second is the one that matters: - F1_ResolvesToAbout_WithoutConsultingTheOpenTab asserts both lookups side by side, so the distinction cannot be folded away. Plus a Theory over F5, Escape, F2, F and Enter: a shell lookup that claimed F5 would swallow it before the tab ever saw it, and every existing test would still pass because they all call AcceleratorCommand directly rather than going through the handler's ordering. - TheShell_AsksForAShellAccelerator_BeforeTheOpenTabsOwn reads the handler's source. Without it, deleting the call from the handler leaves F1 resolving correctly to a command nothing executes — a view-model surface tested and bound by nothing, which is the most common defect shape in this codebase. It asserts the ORDER too, not just the presence. Both new integration tests read the shared NavSurfaceFixture and execute nothing, so no second MainWindowViewModel is built: three extra constructions once took CI from 6 to 22 minutes. Also repaired a pre-existing fragility this change tripped. Escape_IsNotGatedOnCanExecute sliced 1200 characters from Window_KeyDown's signature and asserted the F5 CanExecute check was inside; adding the F1 branch above pushed it past 1200, so the test went red over a line that had not changed. Raising the number just moves the next false failure further out, so the slice is brace-matched now and ends where the method ends. Red ritual, six mutations, each rebuilt before running and restored from saved bytes: F1 dropped from the lookup; F1 routed to the wrong command (resolving to *something* is not the property); the lookup claiming F5 as well; the handler no longer asking the lookup; the lookup moved below the per-tab gate; and the chip losing Enter/Space, which also proves EveryMouseClickableElement_IsAlsoKeyboardOperable sees the new element rather than passing vacuously. Unit 5799 passed, 0 failed. Integration 46 in that class, 0 failed. All four projects 0 warnings. dotnet format clean. SECURITY.md moved to 1.110.x for the minor bump. Closes #1640
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
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.
Problem
The help was already written. Nothing could reach it.
About carries the version to quote, the release notes, a "Report a problem" button that pre-fills version and elevation, an "Ask a question" button that deep-links the Q&A category, the changelog and the repo link. And About is the last child of
grp-info, the 11th of 12 sidebar groups, in a group that starts collapsed — every group except Cleanup does. So a confused user had to expand a group they had no reason to open. There was no?anywhere, no F1, no menu.One premise in the issue is already stale and worth correcting: it says the repo hyperlink is "the only outbound documentation link in the entire UI". About has had Report a problem, Ask a question, View license and Changelog for some time. What was missing was never the content — only the way in.
What changed
Two ways in, one route.
A 28×28
?chip in the sidebar footer, beside the Appearance chip and copied from it verbatim — including the accessibility aBorderneeds and aButtonwould only partly supply: a UIA name, a tab stop, keyboard focus, and Enter/Space, sinceButtonBasehonours Enter only whereKeyboardNavigation.AcceptsReturnis set.F1, from anywhere.The chip routes through
ShellAcceleratorCommand(Key.F1)rather than executingOpenAboutTabCommanditself, so the chip and the key cannot drift into meaning two different things — one of them going stale after a rename is otherwise invisible until someone tries the one nobody tested.Why
ShellAcceleratorCommandis separate fromAcceleratorCommandAcceleratorCommandreturnsnullfor a tab whose content is not built. That is correct for F5 and Escape: there is nothing to refresh or cancel on a tab nobody has opened. It would have made F1 silent on the very first frame, which is exactly when a lost user reaches for it. So F1 gets its own lookup, and the handler checks it before the per-tab branch — because that branch returns early for any key that is not Escape or F5.Scope
The issue also proposed re-opening a Welcome overlay from the same chip. That overlay is #1635 and does not exist yet, so this is the reachability half only.
Verification
Two guards, and the second is the one that matters.
F1_ResolvesToAbout_WithoutConsultingTheOpenTabasserts both lookups side by side, so the distinction cannot be quietly folded away. Plus a Theory over F5, Escape, F2, F and Enter: a shell lookup that claimed F5 would swallow it before the open tab ever saw it, and every existing test would still pass, because they all callAcceleratorCommanddirectly rather than going through the handler's ordering.TheShell_AsksForAShellAccelerator_BeforeTheOpenTabsOwnreads the handler's source. Without it, deleting the call from the handler leaves F1 resolving correctly to a command nothing executes — a view-model surface that is tested and bound by nothing, which is the most common defect shape in this codebase. It asserts the ORDER as well as the presence.Both new integration tests read the shared
NavSurfaceFixtureand execute nothing, so no secondMainWindowViewModelis built — three extra constructions once took CI from 6 minutes to 22.Red ritual — six mutations, each rebuilt before running, each restored from saved bytes:
KeyDownEveryMouseClickableElement_IsAlsoKeyboardOperablesees the new element rather than passing vacuouslyA pre-existing fragility this change tripped, and fixed
Escape_IsNotGatedOnCanExecutesliced 1200 characters fromWindow_KeyDown's signature and asserted the F5CanExecutecheck was inside it. Adding the F1 branch above pushed that check past 1200, so the test went red over a line that had not changed. Raising the number only moves the next false failure further out, so the slice is brace-matched now and ends where the method ends.MainWindowViewModelTests, 0 failed (40 before).dotnet format --verify-no-changes: clean on app, unit and integration.Docs
1.110.xfor the minor bump.Closes #1640