Skip to content

fix: protect NiceButton disabled colors from consumer implicit Label style#30

Merged
nord- merged 1 commit into
masterfrom
fix/disabled-color-vsm
Jun 10, 2026
Merged

fix: protect NiceButton disabled colors from consumer implicit Label style#30
nord- merged 1 commit into
masterfrom
fix/disabled-color-vsm

Conversation

@nord-

@nord- nord- commented Jun 10, 2026

Copy link
Copy Markdown
Owner

Buggen (drabbar alla konsumenter)

Stock-MAUI-mallens implicita Label-stil har en CommonStates/Disabled-VSM-setter som sätter grå TextColor. När knappen disablas propagerar IsEnabled ned till de interna _textLabel/_iconLabel, och den ärvda stilens visual states attachar på dem. VSM-setters vinner över lokalt satta värden, så de skriver tyst över färgerna som ApplyColors sätter — vilket besegrar både 1.8.0:s kontrastfix och den nya DisabledTextColor-bindablen. Eftersom i princip varje MAUI-app behåller mallens implicita stilar var fixen verkningslös i praktiken.

Fixen

Varje intern label får en egen element-nivå CommonStates-grupp med tomma Normal/Disabled-states. Element-nivå-VSG slår stil-nivå-VSG i precedence, så mallens VSM kan aldrig attacha och ApplyColors får sista ordet — som designat. Kräver ingen åtgärd från konsumenten.

Bygger rent (0 varningar/fel) för net10.0-android + net10.0-ios. README uppdaterad.

Inte med i denna PR

  • Test: ett test som verifierar att de interna lablarna har lokala VisualStateGroups skulle kräva att instansiera NiceButton, vilket behöver MAUI-runtime/Application-kontext. Det fristående NiceEntry.Tests (länkad ren geometri, ingen MAUI-workload) kan inte göra det. Verifieras bäst på enhet/simulator.
  • Audit: stock-mallen har Disabled-VSM även på implicita Entry/Editor-stilar. De Labeled*-kontrollernas interna MAUI-element hanterar inte disabled-färger explicit, så ingen kontrast-bugg uppstår där idag — men det är värt att hålla i minnet om sådana färger införs.

…style

A stock MAUI app template ships an implicit Label style whose CommonStates/Disabled visual state sets a grey TextColor. When the button is disabled, IsEnabled propagates to the inner icon/text labels and that inherited style attaches its visual states to them; visual-state setters outrank locally-set values, so they silently override the colors ApplyColors assigns — defeating both the 1.8.0 disabled-contrast fix and the DisabledTextColor bindable. Since virtually every MAUI app keeps the template's implicit styles, the contrast fix was ineffective in practice.

Give each inner label its own element-level CommonStates group with empty Normal/Disabled states. An element-level visual-state group outranks a style-level one, so the inherited style's states never attach and ApplyColors keeps the final say.
Copilot AI review requested due to automatic review settings June 10, 2026 14:16
@nord- nord- added bug Something isn't working patch labels Jun 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a .NET MAUI styling precedence issue where an app template’s implicit Label style (via CommonStates/Disabled VSM setters) could silently override NiceButton’s intended disabled text/icon colors on its internal labels. The fix ensures NiceButton.ApplyColors remains the source of truth for disabled coloring without requiring any consumer action.

Changes:

  • Adds element-level (empty) CommonStates visual-state groups to NiceButton’s internal _iconLabel and _textLabel to block inherited style-level VSM setters.
  • Updates the README to document why disabled colors won’t be overridden by implicit Label styles.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
README.md Documents the disabled-color protection behavior and why it works.
NiceEntry/NiceButton.cs Installs per-label element-level CommonStates VSM groups to neutralize inherited style-level disabled setters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nord-

nord- commented Jun 10, 2026

Copy link
Copy Markdown
Owner Author

Code review

No blocking issues found. Checked for bugs (MAUI VSM semantics), CLAUDE.md compliance, git history, prior-PR feedback, and code-comment consistency.

Verified specifically:

  • Locally set VisualStateGroups wins over style-provided VSM (local > style precedence), and empty states make GoToState a no-op with nothing to unapply on Disabled→Normal — ApplyColors keeps the final say, which fixes the consumer-template override (measured Gray600 #404040 on #3A3A3A in FlightRecorderLog before this fix).
  • No other VisualStateManager/GoToState usage in NiceButton that the wholesale list replacement could suppress; ctor timing is safe (pure attached-property write, no visual tree needed).
  • The branch is behind master (base 238fe17, before the a11y work from fix: CI hardening and control binding/lifecycle cleanup #28/feat: autocomplete API expansion and NiceButton accessibility #29), but the merge is CLEAN — verified via git merge-tree that the merged file retains both SemanticDescription/accessibility code and NeutralizeInheritedVisualStates. A rebase before merge would still be nice so CI builds both features together.

Two take-it-or-leave-it nits: the parameter is typed VisualElement but named label (either type it Label or rename to element), and the calls sit mid-ctor rather than with the tail setup calls (RebuildContent/ApplyColors) — neither affects correctness.

Suggest a follow-up issue for the same treatment on other controls with internal MAUI elements (the stock template also ships Disabled VSM states on implicit Entry/Editor styles).

@nord- nord- merged commit 4f1b626 into master Jun 10, 2026
2 checks passed
@nord- nord- deleted the fix/disabled-color-vsm branch June 10, 2026 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants