-
Notifications
You must be signed in to change notification settings - Fork 4
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
Share and rate app #140
Share and rate app #140
Conversation
WalkthroughThis pull request includes multiple changes across various files in the Khelo application. Key modifications involve the addition of new localization entries for user profile settings, enhancements to the iOS Changes
Possibly related PRs
Suggested reviewers
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 (
|
1b0ccbf
to
73f51cc
Compare
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: 3
🧹 Outside diff range and nitpick comments (5)
khelo/windows/flutter/generated_plugins.cmake (1)
Line range hint
1-2
: Reminder: This is an auto-generated fileAs indicated by the header comment, this is an auto-generated file. Any changes should be made through proper Flutter commands (e.g.,
flutter pub get
) rather than direct modifications.khelo/ios/Runner/Info.plist (1)
76-78
: Fix XML indentation to match file style.The indentation uses spaces instead of tabs and doesn't align with the rest of the file.
<key>LSApplicationQueriesSchemes</key> - <array> - <string>itms-apps</string> - </array> + <array> + <string>itms-apps</string> + </array>khelo/pubspec.yaml (1)
44-44
: Consider organizing dependencies by categoryThe
share_plus
package should be grouped with other platform integration packages for better maintainability.Consider reorganizing as:
- share_plus: ^10.1.2 # UI cupertino_icons: ^1.0.8 + + # Platform Integration + share_plus: ^7.2.1 + url_launcher: ^6.3.0 - url_launcher: ^6.3.0khelo/lib/ui/flow/profile/profile_view_model.freezed.dart (1)
Line range hint
1-1
: Reminder: Generated code modifications.Since this is a generated file (as indicated by the header comment), ensure that any changes to the debugging capabilities were made in the source file and not directly in this generated file. The Freezed generator will overwrite any manual changes.
khelo/lib/ui/flow/profile/profile_screen.dart (1)
34-39
: Consider moving URLs to a configuration fileThese hardcoded URLs should be moved to a dedicated configuration file (e.g.,
lib/config/app_config.dart
) for better maintainability and easier updates across environments (staging/production).Example implementation:
// lib/config/app_config.dart class AppConfig { static const privacyPolicyUrl = "https://canopas.github.io/khelo/privacy-policy"; static const termsAndConditionsUrl = "https://canopas.github.io/khelo/terms-and-condition"; static const playStoreLink = "https://play.google.com/store/apps/details?id=com.canopas.khelo"; static const appStoreLink = "https://apps.apple.com/app/khelo/id6480175424"; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
khelo/assets/images/ic_share.svg
is excluded by!**/*.svg
khelo/assets/images/ic_star.svg
is excluded by!**/*.svg
khelo/ios/Podfile.lock
is excluded by!**/*.lock
khelo/lib/gen/assets.gen.dart
is excluded by!**/gen/**
khelo/pubspec.lock
is excluded by!**/*.lock
📒 Files selected for processing (10)
docs/_layouts/default.html
(0 hunks)khelo/assets/locales/app_en.arb
(1 hunks)khelo/ios/Runner/Info.plist
(1 hunks)khelo/lib/ui/flow/profile/profile_screen.dart
(2 hunks)khelo/lib/ui/flow/profile/profile_view_model.dart
(2 hunks)khelo/lib/ui/flow/profile/profile_view_model.freezed.dart
(2 hunks)khelo/macos/Flutter/GeneratedPluginRegistrant.swift
(2 hunks)khelo/pubspec.yaml
(1 hunks)khelo/windows/flutter/generated_plugin_registrant.cc
(2 hunks)khelo/windows/flutter/generated_plugins.cmake
(1 hunks)
💤 Files with no reviewable changes (1)
- docs/_layouts/default.html
🔇 Additional comments (16)
khelo/windows/flutter/generated_plugins.cmake (2)
13-13
: LGTM! Plugin registration looks correct.
The share_plus
plugin is properly registered in the Windows plugin list and maintains correct alphabetical ordering.
13-13
: Verify share_plus plugin configuration
Let's ensure the plugin is properly configured in the project.
✅ Verification successful
Let me gather more information to verify the share_plus plugin configuration properly.
Let me try one more verification to check the share_plus plugin configuration in the Windows platform files since we already see it's registered in the generated plugins file.
Based on the verification results from all our checks:
- We found the share_plus plugin is properly registered in the Windows generated plugins file
- The plugin is properly imported in the Windows platform code
- The iOS Podfile shows share_plus is included in dependencies
- While we don't see explicit CMake configurations, this is normal as the plugin may use default registration
Share_plus plugin configuration appears correct
The plugin is properly registered in the necessary platform files and no configuration issues were detected. The presence in generated_plugins.cmake and the proper imports indicate correct integration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify share_plus plugin configuration in pubspec.yaml and other platform files
# Check if share_plus is properly configured in pubspec.yaml
echo "Checking pubspec.yaml configuration..."
rg "share_plus:" -A 2 "pubspec.yaml"
# Verify plugin registration in other platform files
echo "Checking plugin registration in other platforms..."
fd "GeneratedPluginRegistrant" --exec rg "share_plus"
Length of output: 355
Script:
#!/bin/bash
# Let's check the complete plugin configuration across different platform files
# Check pubspec.yaml with case-insensitive search
echo "Checking pubspec.yaml with case-insensitive search..."
rg -i "share.?plus" "pubspec.yaml"
# Check all platform-specific plugin files
echo "Checking platform-specific plugin files..."
fd "generated_plugin_registrant" --exec rg -i "share.?plus" {}
# Check iOS platform configuration
echo "Checking iOS plugin configuration..."
fd "Podfile" --exec cat {}
# Check Android platform configuration
echo "Checking Android plugin configuration..."
fd "build.gradle" --exec rg -i "share.?plus" {}
Length of output: 61119
Script:
#!/bin/bash
# Check Windows platform configuration for share_plus
# Check Windows plugin registration
echo "Checking Windows plugin registration..."
fd "CMakeLists.txt" --exec rg -i "share.?plus" {}
# Check Windows plugin source files
echo "Checking Windows plugin source files..."
fd -e cpp -e h --exec rg -i "share.?plus" {}
Length of output: 278
khelo/windows/flutter/generated_plugin_registrant.cc (1)
16-16
: LGTM! The share_plus plugin is correctly registered.
The SharePlusWindows plugin is properly integrated into the Windows plugin registry. Note that this is an auto-generated file and should not be modified manually.
Also applies to: 34-35
khelo/macos/Flutter/GeneratedPluginRegistrant.swift (1)
21-21
: LGTM! Plugin registration looks correct.
The share_plus
plugin is properly imported and registered following Flutter's plugin registration patterns.
Let's verify the plugin declaration in pubspec.yaml:
Also applies to: 40-40
✅ Verification successful
Plugin dependency is properly declared in pubspec.yaml
The share_plus
plugin is correctly declared with version ^10.1.2
in the project's dependencies, which aligns with the plugin registration in the GeneratedPluginRegistrant.swift file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify share_plus plugin declaration in pubspec.yaml
# Check if share_plus is properly declared in pubspec.yaml
if ! grep -A 1 "share_plus:" pubspec.yaml; then
echo "Warning: share_plus plugin not found in pubspec.yaml"
exit 1
fi
Length of output: 206
Script:
#!/bin/bash
# Let's first find the pubspec.yaml file location
fd pubspec.yaml
# Then check for share_plus plugin declaration
fd pubspec.yaml --exec grep -A 1 "share_plus:" {}
Length of output: 148
khelo/ios/Runner/Info.plist (1)
75-78
: LGTM! Required configuration for App Store integration.
The addition of LSApplicationQueriesSchemes
with itms-apps
is necessary to enable the app rating functionality through the App Store.
khelo/pubspec.yaml (1)
44-44
:
Verify the share_plus package version
The specified version ^10.1.2
seems incorrect as the latest stable version on pub.dev is 7.2.1.
Consider updating to the latest stable version:
- share_plus: ^10.1.2
+ share_plus: ^7.2.1
khelo/lib/ui/flow/profile/profile_view_model.dart (2)
1-12
: LGTM! Import changes are appropriate.
The new imports support the added functionality:
dart:io
for platform-specific logicshare_plus
for sharing capabilitiesfoundation.dart
for Flutter's core functionality
110-123
: Verify UI integration and error handling.
Please ensure:
- The UI properly displays the error states from these methods
- The loading states are handled appropriately during async operations
✅ Verification successful
UI integration and error handling are properly implemented
The verification shows:
- Both
onRateUs
andonShareApp
methods are correctly integrated inprofile_screen.dart
with proper tap handlers - The profile screen implements error handling through
showErrorSnackBar
for displaying errors to users - The view model properly manages errors through the
actionError
state and has error handling for other async operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for UI components using these methods
rg -t dart "onRateUs|onShareApp" --glob "lib/ui/**/*"
# Verify error handling in UI
ast-grep --pattern 'actionError != null' -l dart
Length of output: 255
Script:
#!/bin/bash
# Search for UI components using these methods with broader path pattern
rg -t dart "onRateUs|onShareApp"
# Look for profile view model usage
rg -t dart "ProfileViewModel"
# Search for error handling patterns in dart files
rg -t dart "catch|throw|error" --glob "**/*profile*.dart"
Length of output: 7253
khelo/lib/ui/flow/profile/profile_view_model.freezed.dart (3)
167-167
: LGTM: DiagnosticableTreeMixin addition enhances debugging capabilities.
The addition of DiagnosticableTreeMixin
to _$ProfileStateImpl
is a good practice for Flutter state classes as it provides better debugging support in the Flutter DevTools and error messages.
189-191
: LGTM: toString method signature properly updated for diagnostics.
The updated toString
method signature with DiagnosticLevel
parameter aligns with Flutter's diagnostic system, allowing for different levels of verbosity in debug output.
193-205
: Verify the source class implements proper diagnostic properties.
The debugFillProperties
implementation looks correct, but ensure the source class (profile_view_model.dart
) properly implements the necessary diagnostic properties.
✅ Verification successful
These commands will help us:
- Find the actual ProfileState class implementation
- Locate the source file
- Verify if this is a generated file from freezed package
This will help us verify if the source class properly implements all the diagnostic properties that are being used in the generated file.
The diagnostic properties implementation is correct and complete.
The debugFillProperties
implementation in the generated file correctly reflects all the properties defined in the source ProfileState
class:
actionError
currentUser
appVersion
enableUserNotification
shouldShowNotificationBanner
The implementation follows Flutter's diagnostic guidelines by:
- Calling
super.debugFillProperties
- Adding the type information
- Including all state properties with appropriate property types
- Using proper property names that match the source class
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the source class properly implements diagnostic properties
# and follows Flutter's diagnostic guidelines
# Search for the source class implementation
rg -A 5 "class ProfileState" lib/
Length of output: 80
Script:
#!/bin/bash
# Let's try searching in the entire repository for the ProfileState class
rg -A 5 "class ProfileState"
Length of output: 527
Script:
#!/bin/bash
# Let's also look for the source file name pattern
fd "profile_view_model.dart$"
Length of output: 150
Script:
#!/bin/bash
# Let's check if this is a generated file by looking for @freezed annotation
rg "@freezed" -B 2 -A 2
Length of output: 26477
Script:
#!/bin/bash
# Let's check the source ProfileState class implementation
cat khelo/lib/ui/flow/profile/profile_view_model.dart
Length of output: 4170
khelo/lib/ui/flow/profile/profile_screen.dart (3)
207-214
: LGTM: Share functionality implementation
The share functionality is well-implemented with:
- Proper localization
- Platform-specific store links
- Consistent UI pattern
199-199
: Verify URL handling implementation
The openUrl
method is used for opening privacy policy and terms & conditions. Ensure proper error handling and loading states are implemented in the notifier.
Also applies to: 205-206
✅ Verification successful
URL handling implementation is properly implemented with error handling
The openUrl
method in profile_view_model.dart
already has proper error handling:
- Uses try-catch block to handle URL parsing and launching errors
- Updates state with error information if something goes wrong
- Uses
LaunchMode.externalApplication
for secure external browser launching
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for openUrl implementation and error handling
ast-grep --pattern 'openUrl($_) {
$$$
}'
Length of output: 44
Script:
#!/bin/bash
# Search for openUrl method with broader pattern to catch different syntax
ast-grep --pattern 'openUrl'
# Also search with ripgrep for any URL opening related code
rg "openUrl|launchUrl|launch.*Uri" -A 3
Length of output: 1881
215-219
: Verify rate functionality implementation
The rate functionality looks good but verify:
- Platform-specific rating URL construction in
onRateUs()
- Error handling for URL launching
✅ Verification successful
Rate functionality implementation is properly handled
The implementation in profile_view_model.dart
shows:
- Platform-specific URL construction is correctly handled:
- Android: Uses market URL with dynamic package name
- iOS: Uses iTunes URL with specific app ID
- Web/Others: Falls back to GitHub URL
- URL launching is implemented using the
url_launcher
package with proper error handling throughLaunchMode.externalApplication
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for onRateUs implementation
ast-grep --pattern 'onRateUs() {
$$$
}'
Length of output: 43
Script:
#!/bin/bash
# Search for onRateUs implementation with broader pattern and context
rg "onRateUs" -A 10
# Also search for rate-related implementations
rg "rate.*url" -i
# Search for URL launching implementation
rg "launchUrl|launch.*url" -i
Length of output: 2877
khelo/assets/locales/app_en.arb (2)
962-963
: LGTM: Clear and consistent title strings.
The new title strings follow the established naming pattern and are appropriately concise.
968-978
: Verify the store links population.
The share message template is well-structured, but we should verify that the Android and iOS store links are properly populated at runtime.
✅ Verification successful
Let me gather more information about how the share message is being used.
Let me check where these store link variables are defined and populated.
Store links are properly configured and populated
The verification shows that the store links are properly defined as constants in the _ProfileScreenState
class:
- Android:
playStoreLink
points to "https://play.google.com/store/apps/details?id=com.canopas.khelo" - iOS:
appStoreLink
points to "https://apps.apple.com/app/khelo/id6480175424"
These links are correctly passed to the share message template when the share action is triggered.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for the store links configuration
# Look for store URL configurations
rg -A 2 "store.*url|store.*link" --glob "*.{dart,yaml,json}"
# Look for share message usage to verify link population
ast-grep --pattern 'profile_setting_share_app_message'
Length of output: 219
Script:
#!/bin/bash
# Look for the implementation details around the share message usage
rg -B 5 -A 10 "profile_setting_share_app_message" --glob "*.dart"
# Search for any store URL/link constants or configurations
rg -l "(playStore|appStore|storeUrl|storeLink)" --glob "*.{dart,yaml,json}"
Length of output: 1502
Script:
#!/bin/bash
# Look for the definition and initialization of playStoreLink and appStoreLink variables
rg -B 10 -A 2 "playStoreLink|appStoreLink" --glob "*.dart"
# Look for the ProfileNotifier class implementation that handles onShareApp
ast-grep --pattern 'class ProfileNotifier'
Length of output: 2511
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
khelo/lib/ui/flow/profile/profile_view_model.dart
(3 hunks)
🔇 Additional comments (2)
khelo/lib/ui/flow/profile/profile_view_model.dart (2)
1-12
: LGTM: Import changes are appropriate
The new imports support the added functionality - Platform checks for rating URLs and Share functionality.
Line range hint 1-136
: Verify integration with profile settings UI
The implementation aligns with the PR objectives for sharing and rating functionality. However, let's verify the integration with the profile settings UI.
✅ Verification successful
Integration with profile settings UI is properly implemented
The verification shows that both onShareApp
and onRateUs
methods are correctly integrated in the profile screen UI:
- Methods are called from appropriate UI elements using
_settingItem
widgets - Share functionality includes proper localization with store links
- Rate us functionality is integrated with proper icon and localization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for profile settings UI files that should integrate these methods
rg -l "onShareApp|onRateUs" --type dart
Length of output: 137
Script:
#!/bin/bash
# Check the integration in profile_screen.dart
rg "onShareApp|onRateUs" khelo/lib/ui/flow/profile/profile_screen.dart -C 5
Length of output: 756
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
khelo/lib/ui/flow/profile/profile_view_model.dart
(3 hunks)
🔇 Additional comments (2)
khelo/lib/ui/flow/profile/profile_view_model.dart (2)
1-12
: LGTM! Import changes are appropriate.
The new imports support the added functionality for platform-specific operations and sharing features.
31-32
: LGTM! Constants are well-defined.
The constants are appropriately named and scoped for their intended use.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
khelo/lib/ui/flow/profile/profile_view_model.dart
(3 hunks)
🔇 Additional comments (2)
khelo/lib/ui/flow/profile/profile_view_model.dart (2)
1-1
: LGTM! Import changes are appropriate.
The new imports support the added functionality - Platform checks and sharing capabilities.
Also applies to: 8-8, 12-12
31-31
: Consider inlining the URL.
As per previous review feedback, if webFallbackUrl
is only used once in onRateUs
, consider defining it directly in the URL construction instead of creating a separate constant.
b9714ca
to
015cae1
Compare
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 and nitpick comments (1)
khelo/lib/ui/flow/profile/profile_view_model.dart (1)
113-126
: Move the iOS app ID to a configuration file.While the implementation looks good overall, the iOS app ID (
6480175424
) should be moved to a configuration file or environment variable to maintain better separation of concerns and make it easier to update.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
khelo/lib/ui/flow/profile/profile_view_model.dart
(3 hunks)
🔇 Additional comments (2)
khelo/lib/ui/flow/profile/profile_view_model.dart (2)
1-12
: LGTM! Imports are properly organized.
The new imports for dart:io
and share_plus
are necessary for the platform-specific checks and sharing functionality.
31-31
: Consider inlining the webFallbackUrl constant.
As per the previous review comment and since this URL is only used once in the onRateUs
method, consider inlining it directly in the method to reduce unnecessary abstraction.
implement share and rate us in app
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Chores
share_plus
plugin for sharing functionality across platforms.share_plus
dependency.