-
Notifications
You must be signed in to change notification settings - Fork 28
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: secure_store compatible with flutter 3.27.1 #429
fix: secure_store compatible with flutter 3.27.1 #429
Conversation
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~429 Documentation is deployed and generated using docs.page. |
WalkthroughThe pull request introduces configuration updates for Android Gradle build files in the Changes
Sequence DiagramsequenceDiagram
participant UI as ProtectWalletScreen
participant BiometricsStore as BiometricsStore
participant Navigation as Navigation
UI->>BiometricsStore: isAvailable()
BiometricsStore-->>UI: Future<bool>
alt Biometrics Supported
UI->>UI: Show Biometric Protection Button
UI->>Navigation: Navigate Back
else Biometrics Not Supported
UI->>UI: Show Empty SizedBox
end
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🔭 Outside diff range comments (1)
packages/secure_store/android/build.gradle (1)
Line range hint
11-12
: Update Gradle and Kotlin plugin versions for Flutter 3.27.1 compatibility.The current versions are outdated and may cause compatibility issues:
com.android.tools.build:gradle:7.2.0
→8.1.0
- Kotlin version
1.7.10
→1.9.10
This aligns with the versions used in the example app's settings.gradle.
🧹 Nitpick comments (3)
packages/secure_store/example/android/settings.gradle (1)
19-23
: Consider using version catalogs for plugin versions.While the explicit plugin versions are good for reproducibility, consider moving them to
libs.versions.toml
for better maintainability across modules.packages/wallet_kit/lib/wallet_screens/protect_wallet_screen.dart (2)
20-21
: Clarify the purpose of theisBiometricSupported
flag
TheisBiometricSupported
flag only checks if the current platform supports biometrics, not whether it is available/configured on a particular device. You may want to rename it toisPlatformBiometricCapable
or similarly, so it’s more semantically accurate and less likely to be confused with actual biometric readiness.
45-46
: Check for mounted context before navigation
When making asynchronous calls, there is a possibility that the widget might have been disposed before theNavigator
call. Consider usingif (!context.mounted) return;
in Flutter 3.7 or higher, or manage navigation in a callback that checks the widget’s lifecycle to avoid potential exceptions.- Navigator.of(context).popUntil((route) => route.isFirst); + if (context.mounted) { + Navigator.of(context).popUntil((route) => route.isFirst); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/secure_store/android/build.gradle
(2 hunks)packages/secure_store/example/android/app/build.gradle
(1 hunks)packages/secure_store/example/android/build.gradle
(0 hunks)packages/secure_store/example/android/gradle/wrapper/gradle-wrapper.properties
(1 hunks)packages/secure_store/example/android/settings.gradle
(1 hunks)packages/wallet_kit/lib/wallet_screens/protect_wallet_screen.dart
(2 hunks)
💤 Files with no reviewable changes (1)
- packages/secure_store/example/android/build.gradle
✅ Files skipped from review due to trivial changes (1)
- packages/secure_store/example/android/gradle/wrapper/gradle-wrapper.properties
🔇 Additional comments (4)
packages/secure_store/example/android/settings.gradle (1)
12-16
: LGTM! Repository configuration follows security best practices.The repository configuration correctly uses official sources (Google, Maven Central, Gradle Plugin Portal) in the recommended order.
packages/secure_store/android/build.gradle (1)
28-29
: LGTM! Namespace declaration follows Android best practices.The explicit namespace declaration is required for Android Gradle Plugin 8.0+ and follows best practices.
packages/secure_store/example/android/app/build.gradle (2)
1-5
: LGTM! Plugin configuration follows modern Gradle practices.The use of the plugins DSL and the order of plugin declarations is correct and aligns with Flutter 3.27.1 requirements.
Line range hint
43-44
: Address TODO comment regarding Application ID.The TODO comment suggests using a custom Application ID. For production builds, replace
com.example.example
with your organization's reverse domain name.Would you like me to help generate a more appropriate Application ID based on your organization's domain?
ProtectWalletScreen
Summary by CodeRabbit
Chores
Refactor