Use the raised surface token for diff hunk headers - #73
Merged
Conversation
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.
Why
Three
MaterialTheme.colorSchemeusages remained after the type-scale migration, deliberately left because they had no mapping inTheme.kt'scolorScheme(...). I measured what they actually resolve to:surfaceVariant#49454Fpurple-grey#E7E0EClavender-tintedtertiary#EFB8C8pink#7D5260rosesecondary#CCC2DClavender#625B71purple-greyThese are Material 3's baseline purple palette, rendering inside an app whose direction is "Nordic Clarity × JetBrains" with calm neutrals and an emerald accent. None of it was chosen.
This PR fixes the one whose semantic purpose is unambiguous. The other two need a design decision and are documented below rather than guessed at.
What changed
GitDiffScreen.HunkHeader— the@@ -1,4 +1,6 @@strip above each diff hunk — now usesAslTheme.colors.surfaceContainerHighinstead ofMaterialTheme.colorScheme.surfaceVariant.The semantic is exact: a hunk header is a raised container strip within a surface, which is precisely what
surfaceContainerHighexists for.Behavior
Rendering stays consistent, and the visual hierarchy is preserved exactly — the header remains lighter than the surface it sits on:
#2B2D30#49454F#35373C#FFFFFF#E7E0EC#E9EAEELight mode is effectively identical (
#E7E0EC→#E9EAEE). Dark mode loses a purple tint it never should have had, moving to the palette's own neutral grey at the same relative weight.The decision still needed — git graph lane colours
GitHistoryScreen.GitGraphGutterbuilds a categorical palette indexed bypalette[lane % 6]to tell branch lanes apart. It carries no semantic meaning; the colours only need to be distinguishable. It is currently assembled from three different sources:Four separate problems, only one of which is the
MaterialThemeusage:#FF8F00; Emerald → 98 from lane 4's#43A047. Below roughly 80 these are not distinguishable on a 10dp lane, so an Amber user sees two lanes that look the same.errorred used as a lane colour — semantically misleading; red means failure everywhere else in this app.Why this is not fixed here: there is no existing token for a qualitative palette — every colour in
AslColorSchemeis semantic (error,warning,success,info,accentPrimary). Creating one means choosing six colours, which changes rendering by definition. Baking the current six in as a token would preserve the collisions and the unintended pink.The decision required: add
AslColorScheme.graphLanes: List<Color>— six qualitative colours defined per light and dark scheme, chosen to be mutually distinguishable at 10dp and to avoid the semantic colours soerrorred never reads as a branch.A reasonable starting point already exists in the palette: the syntax colours are seven designed, distinguishable, light/dark-aware hues tuned for this app (
syntaxType#4EC9B0,syntaxNumber#6897BB,syntaxKeyword#CC7832,syntaxString#6A8759,syntaxFunction#FFC66D,syntaxVariable#A9B7C6). Whether to reuse those hues or commission a dedicated ramp is a design call, not a refactor.Until then
GitHistoryScreenkeeps itsMaterialThemeimport, and its two baseline entries stay — visible rather than silently papered over.Verification
./gradlew test— all modules pass../gradlew :app:compileDebugKotlin— passes../gradlew detekt— passes.lightColorScheme()/darkColorScheme()rather than recalled.MaterialTheme:Theme.kt(the adapter that installs the tokens) andGitHistoryScreen.kt(the undecided lane palette).Baseline
feature/git/detekt-baseline.xmlregenerated — 1 removed, 0 added (ForbiddenImport:GitDiffScreen.kt). Project total 242 → 241.