Render features through the design system instead of Material - #90
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
Seven feature modules depended on Material directly and used it in 36 files —
Text,Scaffold,HorizontalDivider,SnackbarHost,ModalBottomSheet. Every one of those bypasses the design system: Material's defaults win, the design tokens do not apply, and a change to the app's look has to be made seven times instead of once.The design system already had 60+ components. It was missing the primitives features actually reach for most.
What changed
New in
:designsystem:AslText(String and AnnotatedString),AslScaffold,AslHorizontalDivider/AslVerticalDivider,AslSnackbarHost+AslSnackbarState, andAslTextStyles. Dividers default to theborderSubtletoken and the scaffold tosurface/textPrimaryrather than Material's colour scheme.All 36 feature files migrated. All seven feature modules dropped
implementation(libs.androidx.compose.material3).:feature:editoralso droppedmaterial-icons-extended, which turned out to be entirely unused.Enforced in the build.
verifyModuleBoundariesnow fails when any:feature:*module declares a dependency onandroidx.compose.material3orandroidx.compose.material.:designsystemis exempt — wrapping Material is its job.Two things that a typealias could not fix
AslTypographyis a MaterialTypography, so readingAslTypography.titleMediumputs Material on the caller's classpath even though the result is a plainTextStyle.AslTextStylesexposes the same scale asandroidx.compose.ui.text.TextStylevalues, which is a Compose UI type, so features get the type scale without the dependency.AslSnackbarStatestarted astypealias AslSnackbarState = SnackbarHostStateand that failed for the same reason — an alias still resolves to the Material type. It is now a real wrapper holding the Material state internally.Also fixed: two holes in the boundary check itself
Found while auditing this work:
The test-configuration exemption was too loose. It matched
"test" in name.lowercase(), so a configuration namedcontestImplementationwould have been treated as test-only and exempted from every rule. Now matched on the camelCase name (startsWith("test") || contains("Test")), which still exempts real names liketestImplementation,androidTestApianddebugUnitTestRuntimeOnly.Anchoring that match initially over-corrected and reported 56 false violations, which is how I know the rule is actually load-bearing rather than decorative.
Behavior
No visual change intended: each wrapper delegates to the Material component it replaces, with defaults sourced from design tokens instead of Material's colour scheme.
TerminalSettingsSheetmoved from a hand-rolledModalBottomSheetto the existingAslBottomSheetand gained amodifierparameter.Tests
:designsystemis not; a non-Material library is not; Material from a test configuration is not; real Gradle test-configuration names are exempt;contestImplementationis not.material3back to:feature:onboardingproduced the violation with an actionable message, and removing it passed.That mutation test also caught a real mistake — restoring the file with
git checkout --silently reinstated the dependency I had removed but not yet committed, and the check flagged it on the next run.Verification
./gradlew test detekt verifyModuleBoundaries :app:assembleDebug— BUILD SUCCESSFUL../gradlew -p build-logic test— BUILD SUCCESSFUL.Boundary report: 171 dependencies checked, 0 baseline entries, 0 violations.
Audit:
material3appears in 0 feature build files and 0 feature sources. The one remaining textual match is a string literal in the editor's autocomplete catalog, which is data rather than a dependency.Note: the two new wrappers are baselined for
LongParameterList. A wrapper has to mirror the parameter list of what it wraps, so the finding is inherent to the pattern rather than something to burn down.