-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Android build fixes #4984
fix: Android build fixes #4984
Conversation
@vaxi87 is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
Thank you for following the naming conventions for pull request titles! 🙏 |
WalkthroughThe pull request introduces several changes across the Android project. In the build configuration, four new dependencies have been added to the Gradle file, and new library version references have been set in the version catalog. The Possibly related PRs
Suggested labels
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/android/app/src/main/res/layout/activity_main.xml (1)
1-21
: Layout looks good but needs minor improvementsThe layout structure is solid, using ConstraintLayout properly for positioning the button. However, there are a few best practices that could be implemented:
- The button text should use a string resource rather than hardcoded text for better internationalization support
- The
tools:layout_editor_absoluteX
andtools:layout_editor_absoluteY
attributes are unnecessary when using constraints<?xml version="1.0" encoding="utf-8"?> <androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:tools="http://schemas.android.com/tools" android:id="@+id/main" android:layout_width="match_parent" android:layout_height="match_parent" tools:context=".MainActivity"> <Button android:id="@+id/button" android:layout_width="wrap_content" android:layout_height="wrap_content" - android:text="Click me!" + android:text="@string/button_click_me" app:layout_constraintBottom_toBottomOf="parent" app:layout_constraintLeft_toLeftOf="parent" app:layout_constraintRight_toRightOf="parent" app:layout_constraintTop_toTopOf="parent" - tools:layout_editor_absoluteX="158dp" - tools:layout_editor_absoluteY="336dp" /> </androidx.constraintlayout.widget.ConstraintLayout>Also, don't forget to add the string resource to your strings.xml file:
<string name="button_click_me">Click me!</string>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/android/app/build.gradle.kts
(1 hunks)packages/android/app/src/main/AndroidManifest.xml
(2 hunks)packages/android/app/src/main/java/com/formbricks/demo/MainActivity.kt
(1 hunks)packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Color.kt
(0 hunks)packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Theme.kt
(0 hunks)packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Type.kt
(0 hunks)packages/android/app/src/main/res/layout/activity_main.xml
(1 hunks)packages/android/formbricksSDK/consumer-rules.pro
(1 hunks)packages/android/formbricksSDK/proguard-rules.pro
(1 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/ActionClass.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/ActionClassReference.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/EnvironmentData.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/EnvironmentResponseData.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/Project.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/Survey.kt
(0 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/network/FormbricksApiService.kt
(2 hunks)packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/webview/FormbricksFragment.kt
(1 hunks)packages/android/gradle/libs.versions.toml
(2 hunks)
💤 Files with no reviewable changes (9)
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/EnvironmentResponseData.kt
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/Project.kt
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/EnvironmentData.kt
- packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Type.kt
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/ActionClass.kt
- packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Color.kt
- packages/android/app/src/main/java/com/formbricks/demo/ui/theme/Theme.kt
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/ActionClassReference.kt
- packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/model/environment/Survey.kt
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run Linters / Linters
- GitHub Check: Build Formbricks-web / Build Formbricks-web
- GitHub Check: Run E2E Tests / Run E2E Tests
🔇 Additional comments (13)
packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/webview/FormbricksFragment.kt (1)
140-140
: Good fix for preventing dialogs from closing unexpectedly!Setting
setCancelable(false)
on the dialog prevents it from closing when users tap outside or scroll on the dialog, which directly addresses one of the PR objectives. This is a simple but effective solution to improve the user experience.packages/android/formbricksSDK/consumer-rules.pro (1)
2-3
: ProGuard rules properly configuredGood addition of the ProGuard rules to keep the
Formbricks
andFormbricksConfig
classes. This prevents obfuscation and optimization of these classes, which is crucial for proper SDK functionality, especially with older Kotlin versions that might have issues with certain annotations.packages/android/app/build.gradle.kts (1)
55-58
: Dependencies appropriately added for XML layout approachThe added dependencies are necessary for the traditional XML layout approach:
androidx.appcompat
for compatibility featuresmaterial
for Material Design componentsandroidx.activity
for activity functionalityandroidx.constraintlayout
for the ConstraintLayout used in the new layoutThese additions align with the transition from Compose to XML layouts described in the PR.
packages/android/app/src/main/AndroidManifest.xml (2)
17-18
: Theme changed to AppCompat.Light.NoActionBarGood change to use an AppCompat theme instead of a custom theme. This is likely related to the main activity implementation changes and helps with compatibility issues.
15-18
: Removed windowSoftInputMode="adjustPan" attributeThe removal of the
android:windowSoftInputMode="adjustPan"
attribute will change how the soft keyboard behaves. This appears to address the PR objective of "preventing dialogs from closing unexpectedly when the user scrolls" since the default behavior is typicallyadjustResize
, which may handle dialogs and scrolling better.packages/android/formbricksSDK/src/main/java/com/formbricks/formbrickssdk/network/FormbricksApiService.kt (1)
31-34
: Added ignoreUnknownKeys for JSON parsingGreat improvement! Setting
ignoreUnknownKeys = true
makes the JSON parsing more resilient to API changes by ignoring unexpected fields in the response. This directly addresses the PR objective of fixing crashes with older Kotlin versions, as JSON serialization exceptions are a common source of such crashes.packages/android/formbricksSDK/proguard-rules.pro (1)
34-36
: Added ProGuard keep rules for critical SDK classesExcellent addition of ProGuard rules to preserve important SDK classes. This directly addresses the PR objective of "implementing fixes for ProGuard configurations" and will prevent runtime issues when ProGuard is enabled by ensuring these classes aren't obfuscated or removed:
DataBinderMapperImpl
- Required for data binding to work correctlyFormbricks
- The main SDK classFormbricksConfig
- Configuration classpackages/android/app/src/main/java/com/formbricks/demo/MainActivity.kt (6)
4-4
: Import properly added for View-based UI approach.The addition of this import supports the migration from Compose to traditional View-based UI approach.
6-8
: Appropriate imports for AppCompat and window insets.These imports correctly support the migration to AppCompatActivity and proper window insets handling.
13-13
: Good base class change to AppCompatActivity.Changing from FragmentActivity to AppCompatActivity is appropriate for traditional View-based UI and provides better compatibility across Android versions.
16-16
: Edge-to-edge display properly maintained.The enableEdgeToEdge() call is correctly preserved to utilize the full screen space.
26-31
: Proper implementation of content view with window insets handling.The migration to XML-based layout with proper window insets handling is correctly implemented. This approach correctly adjusts padding based on system bars for edge-to-edge display.
33-36
: Button click handling properly implemented.The button retrieval using findViewById and click listener setup is correctly implemented, maintaining the same tracking functionality as before.
packages/android/app/src/main/java/com/formbricks/demo/MainActivity.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you very much for fixing these issues :-)
What does this PR do?
Fixes for the following:
Checklist
Required
pnpm build
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Refactor
Chores