Say what the theme's dynamicColor parameter does, and test the branches that run
The KDoc claimed dynamic colour "stays switchable so users can opt back to the brand palette". Nothing switches it: MainActivity is the only caller and passes no arguments, so dynamicColor is always true and the two brand-palette branches are dead. A reader who trusted that sentence would go looking for a setting that has never existed. Replace the claim with what is true today and point at #68, which holds the decision -- add a switch, delete the dead branches along with the template palette, or replace that palette first. None of the three is taken here. ThemeKt had no test, so nothing would have caught the branches being swapped either. Assert what the theme resolves by reading MaterialTheme.colorScheme inside the content lambda: the two live branches on background luminance, which is the one thing two schemes off the same device palette do not share, and the dead pair by passing dynamicColor explicitly. Both KDocs say plainly that the test is the only thing that passes it, so the coverage is not misread as evidence a switch exists -- which is the misreading #68 exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -25,8 +25,18 @@ private val LightColorScheme = lightColorScheme(
|
||||
* Material 3 theme.
|
||||
*
|
||||
* Dynamic color (Material You) needs API 31+; minSdk is 33, so it is available
|
||||
* unconditionally and no version guard is required. It stays switchable so users can
|
||||
* opt back to the brand palette.
|
||||
* unconditionally and no version guard is required.
|
||||
*
|
||||
* [dynamicColor] has no caller. `MainActivity` is the single call site and takes the
|
||||
* default, so the parameter is always `true`, the two dynamic branches always win, and
|
||||
* [DarkColorScheme] and [LightColorScheme] are dead: nothing in the app can opt back to the
|
||||
* brand palette. `ThemeColorSchemeTest` reaches those two branches only by passing
|
||||
* [dynamicColor] explicitly -- a test doing it, not a feature.
|
||||
*
|
||||
* That is known rather than an oversight. #68 holds the choice between adding a switch,
|
||||
* deleting the dead branches together with the template palette, and replacing that palette
|
||||
* first; it is undecided, so nothing here should be read as a promise that any of them
|
||||
* happens.
|
||||
*/
|
||||
@Composable
|
||||
fun LibreMediaConverterTheme(
|
||||
|
||||
Reference in New Issue
Block a user