Skip to content
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

chore: sample app fixes #136

Merged
merged 3 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion apps/amiapp_flutter/android/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ plugins {

android {
namespace 'io.customer.amiapp_flutter'
compileSdkVersion 33
compileSdkVersion 34
ndkVersion flutter.ndkVersion

compileOptions {
// Flag to enable support for the new language APIs
coreLibraryDesugaringEnabled true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, what new language API are we introducing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Sets Java compatibility to Java 8
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
Expand Down Expand Up @@ -40,6 +43,7 @@ android {
targetSdkVersion 33
versionCode 1
versionName "1.0"
multiDexEnabled true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this increases build times when it's enabled so just calling it out to make sure this is required and it wasn't automatically added or you forgot to remove it. If you added it intentionally, no problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. This too was added only for flutter_local_notifications gradle setup.

}

signingConfigs {
Expand Down Expand Up @@ -70,6 +74,9 @@ flutter {

dependencies {
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:1.7.21"
// Required for flutter_local_notifications, see more:
// https://pub.dev/packages/flutter_local_notifications#gradle-setup
coreLibraryDesugaring 'com.android.tools:desugar_jdk_libs:1.2.2'
// Adding customer.io android sdk dependencies so we can use them in native code
// These are not generally needed and should be avoided
implementation "io.customer.android:tracking"
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions apps/amiapp_flutter/ios/Env.swift.example
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation

class Env {
static var siteId: String = "siteid"
static var apiKey: String = "apikey"
static let siteId: String = "siteid"
static let apiKey: String = "apikey"
}
2 changes: 1 addition & 1 deletion apps/amiapp_flutter/ios/Runner/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import CioTracking
import FirebaseMessaging
import FirebaseCore

@UIApplicationMain
@main
@objc class AppDelegate: FlutterAppDelegate {
override func application(
_ application: UIApplication,
Expand Down
15 changes: 8 additions & 7 deletions apps/amiapp_flutter/lib/src/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import 'screens/events.dart';
import 'screens/login.dart';
import 'screens/settings.dart';
import 'theme/sizes.dart';
import 'utils/extensions.dart';
import 'utils/logs.dart';

/// Main entry point of AmiApp
class AmiApp extends StatefulWidget {
final AmiAppAuth auth;

const AmiApp({required this.auth, Key? key}) : super(key: key);
const AmiApp({required this.auth, super.key});

@override
State<AmiApp> createState() => _AmiAppState();
Expand Down Expand Up @@ -98,7 +99,7 @@ class _AmiAppState extends State<AmiApp> {
// If user is not signed in and public view is not allowed, redirect
// to login screen
final isPublicViewAllowed =
state.location.toAppScreen()?.isPublicViewAllowed == true;
state.uri.toString().toAppScreen()?.isPublicViewAllowed == true;
if (!signedIn && !isPublicViewAllowed) {
return Screen.login.location;
}
Expand All @@ -118,8 +119,8 @@ class _AmiAppState extends State<AmiApp> {
path: Screen.settings.path,
builder: (context, state) => SettingsScreen(
auth: _auth,
siteIdInitialValue: state.queryParameters['site_id'],
apiKeyInitialValue: state.queryParameters['api_key'],
siteIdInitialValue: state.uri.queryParameters['site_id'],
apiKeyInitialValue: state.uri.queryParameters['api_key'],
),
),
GoRoute(
Expand Down Expand Up @@ -149,15 +150,15 @@ class _AmiAppState extends State<AmiApp> {
// Initial route will not be tracked if user is logged in as there is no
// route change, tracking initial screen manually for this case.
// Events/screens can only be tracked after SDK has been initialized.
if (_router.location.toAppScreen() != Screen.dashboard) {
if (_router.currentLocation().toAppScreen() != Screen.dashboard) {
_onRouteChanged();
}
return value;
});
_customerIOSDK.addListener(_handleSDKConfigurationsChanged);

// Listen to screen changes for observing screens
_router.addListener(() => _onRouteChanged());
_router.routerDelegate.addListener(() => _onRouteChanged());

super.initState();
}
Expand Down Expand Up @@ -213,7 +214,7 @@ class _AmiAppState extends State<AmiApp> {

void _onRouteChanged() {
if (_customerIOSDK.sdkConfig?.screenTrackingEnabled == true) {
final Screen? screen = _router.location.toAppScreen();
final Screen? screen = _router.currentLocation().toAppScreen();
if (screen != null) {
CustomerIO.screen(name: screen.name);
}
Expand Down
14 changes: 14 additions & 0 deletions apps/amiapp_flutter/lib/src/utils/extensions.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

extension AmiAppExtensions on BuildContext {
void showSnackBar(String text) {
Expand Down Expand Up @@ -99,3 +100,16 @@ extension AmiAppDoubleExtensions on double {
return toStringAsFixed(0);
}
}

extension LocationExtensions on GoRouter {
// Get location of current route
// This is a workaround to get the current location as location property
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is labeled as a workaround, it makes me wonder if this is a short-term solution? To make our sample app an up-to-date reference for customers, should we replace this with recommended best practice code in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'm not expert with GoRouter so not fully sure about best practices for it at the moment. But yes, we can put at a ticket to simplify and improve this for customers. Sample apps project could be the best time to handle this? 🤔

// was removed from GoRouter in v9.0.0
// See migration guide:
// https://flutter.dev/go/go-router-v9-breaking-changes
String currentLocation() {
final RouteMatch lastMatch = routerDelegate.currentConfiguration.last;
final RouteMatchList matchList = lastMatch is ImperativeRouteMatch ? lastMatch.matches : routerDelegate.currentConfiguration;
return matchList.uri.toString();
}
}
Loading
Loading