Skip to content

Pin the scrollbar contract on a grid whose bars actually realize - #554

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/548-datagrid-columns-test
Sep 22, 2026
Merged

erikdarlingdata merged 1 commit into
devfrom
fix/548-datagrid-columns-test

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

Fixes #548. Keeps the existing flag-only contract test untouched (its assertion is real — the AllowAutoHide value is template-bound, not assigned on apply), corrects its stale construction comment, and adds the columns-bearing sibling: a grid that overflows both axes, pinning that templated bars realize at all, keep AllowAutoHide through 12's template binding, and the vertical bar reports visible. This is the part of the #464 slim-rail contract the suite could never see before, short of the geometry itself (which stays a manual check by design — no Skia). Full suite 712 passed / 1 known skip.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX

The existing DataGrid case turns out to assert AllowAutoHide on bars
that never apply their template, because the grid it builds has no
columns and Avalonia's DataGrid does not auto-generate them — so no
row realizes and nothing overflows. The bound flag it checks is still
meaningful, and it stays exactly as it was; its construction comment
now says what is actually happening instead of claiming a real grid.

The new sibling gives a grid columns, makes it overflow both ways,
and pins the state every grid in the app actually runs in: templated
bars exist, each keeps AllowAutoHide from the attached property the
App.axaml rule sets (12 binds it in-template), and the vertical bar
is visible. Zero templated bars realize without columns, so the
NotEmpty assertion alone separates the two cases.

Fixes #548

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sr43FouJ6RizUyoWQeR5FX
@claude

claude Bot commented Sep 22, 2026

Copy link
Copy Markdown

Reviewed. This is a test-only change to tests/PlanViewer.Core.Tests/ScrollBarVisibilityTests.cs — no production code, SQL, or versioned-project files touched, so most of the standing checklist (untrusted plan XML, T-SQL, VSIX/AssemblyInfo version sync, PlanViewer.Web linked includes) doesn't apply here.

The new AGridWithColumnsRealizesItsBarsAndKeepsAllowAutoHide test does what the PR description claims: it gives the grid real columns (one sized wide enough to force horizontal overflow, fifty rows in a 250px-tall grid to force vertical overflow) so the PART_ scrollbars actually template and realize, then asserts AllowAutoHide on the realized bars and visibility on the vertical one — closing the gap the columns-less sibling test structurally can't reach. DataGridTextColumn/DataGridLength usage matches existing patterns in PlanViewerControl.Statements.cs, and the .First(...)/.Single() style for locating a specific bar matches the existing helpers in this same file (ThumbOf), so no new sharp edges there.

No correctness or coverage issues found — looks good to merge.

@erikdarlingdata
erikdarlingdata merged commit 182eb77 into dev Sep 22, 2026
3 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/548-datagrid-columns-test branch September 22, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant