chore: put the four body-text tokens on the implicit style, then adopt them - #2344
Merged
Merged
Conversation
…t them #1634's remaining half was blocked on a prerequisite rather than on effort. Caption, Subtle, SectionTitle and SectionLabel carried no BasedOn, and in WPF an explicit keyed style REPLACES the keyless <Style TargetType="TextBlock"> instead of merging with it. So swapping a raw FontSize="11" for Style="{StaticResource Caption}" looked like a no-op and was not: the element gave up TextRenderingMode=ClearType on the way. 172 elements were waiting behind that. Step 1 — BasedOn on the four, dropping the setters it then supplies (FontFamily on all four, Foreground on SectionTitle, which restated the inherited TextPrimary). SectionLabel had to MOVE to the typography block: BasedOn is a StaticResource reference, a StaticResource cannot resolve a key defined later in the same dictionary, and the keyless TextBlock style sits below where SectionLabel was. Left in place it would have thrown while App.xaml parsed, which is at startup — and this machine never launches the app, so only the resolved-value test would have caught it. Step 2 — 145 conversions across 38 views: 76 at 11, 54 at 12, 15 at 14. The 15 at 14 also drop FontWeight="SemiBold", because all 15 already said exactly what SectionTitle sets. Every one is a size-attribute removal and nothing else. The count is 145 rather than the 148 the issue estimated, and neither number knew about the 5 elements that set Style through a <TextBlock.Style> child for its Triggers: an attribute scan cannot see those, and giving them a Style attribute as well is "property has already been set and can be set only once". The compiler caught all five; the guard now skips them by inspecting the element body. The 24 where the token would visibly differ are untouched, as agreed — 17 name no Foreground at 11 or 12, where the token's colour differs from the inherited TextPrimary, and 7 name no FontWeight at 14, where SectionTitle would bolden them. They need a look, not a sweep. Display stays out, and a test pins that. #1634 proposed including it because the 59 tab titles would move from TextFormattingMode=Display to Ideal. Measured on a real TextBlock, that premise is wrong: Ideal is WPF's default, so they already render that way, and the only difference BasedOn would make is TextRenderingMode, Auto to ClearType — the same one-property change the other four just took. Left out anyway, because the decision was taken on the larger claim. EveryMetricRungSize_IsReachedThroughItsRung now covers 11, 12 and 14 as well, conditionally: at 11 or 12 the token is required only of an element that names its own Foreground, at 14 only of one that names its own FontWeight. That is the same condition that decided which 145 were safe, so the 24 pass without an allowlist to rot, and a NEW element that could have used a token fails. Its rung lookup also stopped requiring FontSize to be the first setter — Subtle writes Foreground first, so the old pattern would have dropped it from the dictionary and enforced a scale with a hole in it. Red ritual, four cases, each red for the right reason and green after restore: a converted element put back to a raw 12; one of the deliberately-raw 24 given its own colour, which is what makes the swap required; a token renamed out from under the guard (caught by the floor, not by silence); BasedOn removed from Caption, which the resolved-value test catches and a compile cannot. The baseline is itself two proofs. 24 elements keep a raw 11, 12 or 14 and are not flagged, so the condition is not a blanket rule. And 5 elements set Style through a child element AND name their own Foreground at 11 or 12, so a scan that could not see the child would flag all five. Unit suite 5774 passed, 0 failed. Integration typography tests 15 passed. All four projects build with 0 warnings. No version bump and no CHANGELOG entry: nothing a user can see changes, which is the claim the resolved-value test exists to support. Refs #1634 — steps 3 and 4 remain, and both are per-site judgement.
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.
The prerequisite
#1634's remaining half was blocked on something, not just unfinished.
Caption,Subtle,SectionTitleandSectionLabelcarried noBasedOn, and in WPF an explicit keyed style replaces the keyless<Style TargetType="TextBlock">rather than merging with it. So swapping a rawFontSize="11"forStyle="{StaticResource Caption}"looked like a no-op and was not — the element gave upTextRenderingMode=ClearTypeon the way. 172 elements were waiting behind that, and the type-scale guard says so in its own remarks: it deliberately did not cover 11, 12 and 14 for exactly this reason.Step 1 — BasedOn on the four
Plus dropping the setters it then supplies:
FontFamilyon all four, andForegroundonSectionTitle, which restated theTextPrimaryit now inherits.Caption,SubtleandSectionLabelkeep their own colour because they deliberately differ from it.SectionLabelhad to move.BasedOnis aStaticResourcereference, and aStaticResourcecannot resolve a key defined later in the same dictionary — the keylessTextBlockstyle sits below whereSectionLabelwas declared, in the middle of the control templates. Left in place it would have thrown whileApp.xamlparsed, which is to say at startup. This machine never launches the app, so the resolved-value test is the only thing that would have caught it.Step 2 — 145 conversions across 38 views
76 at 11, 54 at 12, 15 at 14. The 15 at 14 also drop
FontWeight="SemiBold", because all fifteen already said exactly whatSectionTitlesets and leaving it beside a style that declares the same thing is the drift this issue is about. Every edit is a size-attribute removal and nothing else: 145 insertions, 145 deletions, one line per element.145 rather than the 148 the issue estimated, and neither number knew why. Five elements set
Stylethrough a<TextBlock.Style>child — a Style built inline for itsTriggers— which an attribute scan cannot see. Giving those aStyleattribute as well isMC3024: property has already been set and can be set only once. The compiler caught all five; the guard now skips them by inspecting the element body, so the next sweep will not find them either.The 24 that would visibly change are untouched, as agreed. 17 name no
Foregroundat 11 or 12, where the token's colour differs from the inheritedTextPrimary; 7 name noFontWeightat 14, whereSectionTitlewould bolden them. Those are steps 3 and 4 of the issue and they are per-site judgement, not a sweep.A premise in the issue turned out to be wrong
#1634 proposed including
Displaybecause the 59 tab titles would move fromTextFormattingMode=DisplaytoIdeal. Measured on a realTextBlock:Idealis WPF's default, so they already render that way. The only differenceBasedOnwould make toDisplayisTextRenderingMode,AutotoClearType— the same one-property change the other four just took.Left out anyway, because the decision to exclude it was taken on the larger claim and the smaller one deserves its own look at 28px.
TypographyTokenTests.Display_StillStandsAloneAndStillResolvesItsColourasserts both values, so it fails the moment someone completes the set — which is the prompt to do that look first rather than an oversight sitting in the file.The guard
EveryMetricRungSize_IsReachedThroughItsRungnow covers 11, 12 and 14, conditionally — the same condition that decided which 145 were safe:Foreground;FontWeight.So the 24 pass without an allowlist to rot, and a new element that could have used a token fails. Its rung lookup also stopped requiring
FontSizeto be the first setter:SubtledeclaresForegroundfirst, so the old pattern would have silently dropped it from the dictionary and enforced a scale with a hole in it.Verification
Red ritual — four mutations, each red for the right reason, green after restore, every file restored from saved bytes:
BasedOnremoved fromCaptionThe baseline is itself two proofs. 24 elements keep a raw 11/12/14 and are not flagged, so the condition is not a blanket rule. And the five
<TextBlock.Style>elements name their ownForegroundat 11 or 12 — so a scan that could not see the child element would flag all five, and the guard is green.dotnet build -c Release, 0 errors, 0 warnings.dotnet format --verify-no-changes: clean on tests and integration tests.No release
No version bump and no CHANGELOG entry: nothing a user can see changes, which is the claim the resolved-value test exists to support.
AllTabsSmokeUiTestsnavigates all 59 tabs in CI, so a{StaticResource}that failed to resolve at runtime — the failure mode a compile cannot catch — would fail there.Refs #1634 — steps 3 and 4 remain.