ADFA-4826: Add shared IDE Compose theming in common-compose - #1653
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 Walkthrough
WalkthroughThe PR adds the ChangesShared Compose theming
Plugin installation contract
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR centralizes existing Compose theme definitions while preserving the behavior of the profiler and floating-window themes, with the listed builds and tests passing. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Context
participant IdeTheme
participant ideColorScheme
participant MaterialTheme
Context->>IdeTheme: Read theme and dark-mode state
IdeTheme->>ideColorScheme: Resolve XML color attributes
ideColorScheme-->>IdeTheme: Return Material 3 ColorScheme
IdeTheme->>MaterialTheme: Apply scheme, typography, and content color
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@common-compose/src/test/java/com/itsaky/androidide/common/compose/IdeColorSchemeTest.kt`:
- Around line 7-10: Update IdeColorSchemeTest to use JUnit Jupiter by replacing
org.junit.Test with org.junit.jupiter.api.Test, and replace
assertEquals/assertTrue usages with equivalent Truth assertions throughout the
test class. Keep the existing test behavior and coverage unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe123155-b3a9-4cdd-95e5-a6970f48d4aa
📒 Files selected for processing (10)
ARCHITECTURE.mdcommon-compose/build.gradle.ktscommon-compose/src/main/java/com/itsaky/androidide/common/compose/IdeColorScheme.ktcommon-compose/src/main/java/com/itsaky/androidide/common/compose/IdeTheme.ktcommon-compose/src/test/java/com/itsaky/androidide/common/compose/IdeColorSchemeTest.ktfloating-window/build.gradle.ktsfloating-window/src/main/java/com/itsaky/androidide/floating/ui/FloatingTheme.ktprofiler/build.gradle.ktsprofiler/src/main/java/org/appdevforall/cotg/profiler/ui/theme/ProfilerTheme.ktsettings.gradle.kts
ac52c42 to
5a90696
Compare
New leaf module holding the Compose theme any module can opt into: IdeColorScheme derives a Material3 scheme from the IDE's own colour resources, IdeTheme applies it and seeds LocalContentColor so text on a themed surface inherits the right colour. Compose types are exposed as `api` because consumers write Compose against them. Modules that are not Compose depend on nothing new.
Both modules carried their own near-identical copy of the IDE colour derivation. They now delegate to common-compose, so there is one place where the IDE's Compose colours are defined.
5a90696 to
69c15a9
Compare
Jira: ADFA-4826
Bottom of a 3-PR stack. Extracts the Compose colour scheme and theme that
profilerandfloating-windowhad each duplicated into a newcommon-composemodule, so the refactoring UI in the two PRs above has one theme to build on.What's here
common-composemodule:IdeColorScheme(light/dark IDE palettes) andIdeTheme.FloatingThemeandProfilerThemenow delegate to it instead of carrying their own copies (-111 lines).IdeColorSchemeTestcovers the palettes.2 commits, 10 files, +308/-111.
Verification
:common-compose:compileV8DebugKotlin,:profiler:compileV8DebugKotlin,:floating-window:compileV8DebugKotlinpass.:common-compose:testV7DebugUnitTest- 7 tests, 0 failures.spotlessCheckpasses.No behavioural change intended: both themes should render exactly as before.
Stack
Review and merge bottom-up.
Stack created with GitHub Stacks CLI • Give Feedback 💬