Move the menu bar and the image list to NativeControls - #69
Merged
Merged
Conversation
Both controls are now native rather than WinForms-drawn, which is what screen readers need from them. The menu was a MenuStrip: a panel that draws things resembling menu items, with an accessibility tree assembled alongside. It is now a real HMENU attached with SetMenu, announced as a menu bar with real submenus. The menu is described as a spec in one place instead of a designer block plus a separate wiring block, so the whole structure reads at once. Enabled state lives in the spec and is read on every popup, which is why UpdateMenuState no longer pushes flags into fifteen controls. RefreshShortcutKeys is gone with it. It existed to make ToolStrip re-render accelerator text after a language change; a native menu takes its display string from the spec, so rebuilding the spec is the whole job. The image list was a WinForms ListView, which is a SysListView32 wearing a window class no screen reader recognizes - both UI Automation and NVDA pick their list handling by class name. It is now the real control, so every column is read rather than just the first. Three things had no direct counterpart and were translated: - SelectedIndices became SelectedItems, which carries the rows themselves; an index is row.Index where one is still wanted. - AutoResizeColumns became AutoSizeListColumns, because an auto-size is a width you assign rather than a method you call. - ListViewItem plus five SubItems.Add became one NativeListViewItem constructor, in BuildRow. Two subtler ones, both easy to miss: - imageListView.Focused is false while the list has focus, because focus lives on the native child window. The Delete-key guard now asks ContainsFocus, which is true. - Column headers are no longer controls, so the catalog walk in ApplyLocalization cannot reach them. They are localized explicitly in LocalizeColumns. Every string was already in the catalogs, so no translation is lost. Verified against Windows rather than the source: GetMenu returns a real HMENU with four top-level items, and the list window's class name is SysListView32. Fixes #68 Claude-Session: https://claude.ai/code/session_01TJ8i7jHmkUjVCjVccLp6Hf
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.
Fixes #68
Summary
Both controls are now native rather than WinForms-drawn.
The menu was a
MenuStrip— a panel drawing things that resemble menu items, with an accessibility tree assembled alongside. It is now a realHMENUattached withSetMenu, announced as a menu bar with real submenus.The image list was a WinForms
ListView, which is aSysListView32but wears a window class no screen reader recognizes — both UI Automation and NVDA select their list handling by class name. It is now the real control, so every column is read rather than only the first.Verified against Windows, not the source
What the menu change removes
The menu is described as a spec in one place, instead of a designer block plus a separate
Clickwiring block. Enabled state lives in the spec and is read on every popup, soUpdateMenuStatesets two flags rather than pushing enabled state into fifteen controls.RefreshShortcutKeysis gone. It existed to makeToolStripre-render accelerator text after a language change; a native menu takes its display string from the spec, so rebuilding the spec is the whole job.Three mechanical translations
SelectedIndices→SelectedItems, which carries the rows themselves;row.Indexwhere an index is still wanted.AutoResizeColumns→AutoSizeListColumns, because an auto-size is a width you assign rather than a method you call.ListViewItem+ fiveSubItems.Add→ oneNativeListViewItemconstructor, inBuildRow.Two subtler ones, both easy to miss
imageListView.Focusedis now false while the list has focus, because focus lives on the native child window. Measured:Focused = False,ContainsFocus = True. The Delete-key guard would have silently stopped working; it now asksContainsFocus.Column headers are no longer controls, so the GetText catalog walk in
ApplyLocalizationcannot reach them the way it reachedColumnHeader.Text. They are localized explicitly inLocalizeColumns. Every string was already in the catalogs, so no translation is lost — including Hebrew.Worth a listen before merging
This is an accessibility change, so the real test is by ear: Alt to open the menu bar, arrow through it, and confirm the list reads all five columns rather than just the file name.
Also worth checking
Program.cs:31, which swallowsNullReferenceExceptions from "ListView.WndProc / ListView.Unhook races with accessibility". That workaround targets the WinForms accessibility layer this change removes, so it may now be unnecessary — the logs recordTargetSite.Nameon each suppression, which would confirm it.https://claude.ai/code/session_01TJ8i7jHmkUjVCjVccLp6Hf