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.

Sign up for GitHub

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: Improve public settings screen UX #453

Merged
merged 5 commits into from
Oct 16, 2024
Merged

chore: Improve public settings screen UX #453

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
25514b3
Add new config attributes to CustomerIOSDKConfig
mahmoud-elmorabea Oct 10, 2024
c42ef04
Update SDK init to use the new attributes in CustomerIOSDKConfig
mahmoud-elmorabea Oct 10, 2024
993a5d4
Update public settings screen with new UI
mahmoud-elmorabea Oct 10, 2024
8f5d2ac
Update SettingsActivity to use the updated UI binding
mahmoud-elmorabea Oct 10, 2024
2cad68b
Adjust flushAt and flushInterval values
mahmoud-elmorabea Oct 15, 2024
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
95 changes: 61 additions & 34 deletions .../src/main/java/io/customer/android/sample/java_layout/data/model/CustomerIOSDKConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.customer.android.sample.java_layout.support.Optional;
import io.customer.android.sample.java_layout.utils.StringUtils;
import io.customer.datapipelines.extensions.RegionExtKt;
import io.customer.sdk.core.util.CioLogLevel;
import io.customer.sdk.data.model.Region;

/**
Expand All @@ -27,7 +28,11 @@ private static class Keys {
static final String FLUSH_AT = "cio_sdk_flush_at";
static final String TRACK_SCREENS = "cio_sdk_track_screens";
static final String TRACK_DEVICE_ATTRIBUTES = "cio_sdk_track_device_attributes";
static final String DEBUG_MODE = "cio_sdk_debug_mode";
static final String LOG_LEVEL = "cio_sdk_log_level";
static final String REGION = "cio_sdk_region";
static final String TRACK_APPLICATION_LIFECYCLE = "cio_sdk_track_application_lifecycle";
static final String TEST_MODE_ENABLED = "cio_sdk_test_mode";
static final String IN_APP_MESSAGING_ENABLED = "cio_sdk_in_app_messaging_enabled";
}

public static CustomerIOSDKConfig getDefaultConfigurations() {
Expand All @@ -39,6 +44,10 @@ public static CustomerIOSDKConfig getDefaultConfigurations() {
20,
true,
true,
CioLogLevel.DEBUG,
Region.US.INSTANCE,
true,
true,
true);
}

Expand All @@ -57,7 +66,11 @@ public static Optional<CustomerIOSDKConfig> fromMap(@NonNull Map<String, String>
Integer flushAt = StringUtils.parseInteger(bundle.get(Keys.FLUSH_AT), defaultConfig.flushAt);
boolean screenTrackingEnabled = StringUtils.parseBoolean(bundle.get(Keys.TRACK_SCREENS), defaultConfig.screenTrackingEnabled);
boolean deviceAttributesTrackingEnabled = StringUtils.parseBoolean(bundle.get(Keys.TRACK_DEVICE_ATTRIBUTES), defaultConfig.deviceAttributesTrackingEnabled);
boolean debugModeEnabled = StringUtils.parseBoolean(bundle.get(Keys.DEBUG_MODE), defaultConfig.debugModeEnabled);
CioLogLevel logLevel = CioLogLevel.Companion.getLogLevel(bundle.get(Keys.LOG_LEVEL), CioLogLevel.DEBUG);
Region region = Region.Companion.getRegion(bundle.get(Keys.REGION), Region.US.INSTANCE);
boolean applicationLifecycleTrackingEnabled = StringUtils.parseBoolean(bundle.get(Keys.TRACK_APPLICATION_LIFECYCLE), defaultConfig.applicationLifecycleTrackingEnabled);
boolean testModeEnabled = StringUtils.parseBoolean(bundle.get(Keys.TEST_MODE_ENABLED), defaultConfig.testModeEnabled);
boolean inAppMessagingEnabled = StringUtils.parseBoolean(bundle.get(Keys.IN_APP_MESSAGING_ENABLED), defaultConfig.inAppMessagingEnabled);

CustomerIOSDKConfig config = new CustomerIOSDKConfig(cdpApiKey,
siteId,
Expand All @@ -67,7 +80,11 @@ public static Optional<CustomerIOSDKConfig> fromMap(@NonNull Map<String, String>
flushAt,
screenTrackingEnabled,
deviceAttributesTrackingEnabled,
debugModeEnabled);
logLevel,
region,
applicationLifecycleTrackingEnabled,
testModeEnabled,
inAppMessagingEnabled);
return Optional.of(config);
}

Expand All @@ -82,7 +99,11 @@ public static Map<String, String> toMap(@NonNull CustomerIOSDKConfig config) {
bundle.put(Keys.FLUSH_AT, StringUtils.fromInteger(config.flushAt));
bundle.put(Keys.TRACK_SCREENS, StringUtils.fromBoolean(config.screenTrackingEnabled));
bundle.put(Keys.TRACK_DEVICE_ATTRIBUTES, StringUtils.fromBoolean(config.deviceAttributesTrackingEnabled));
bundle.put(Keys.DEBUG_MODE, StringUtils.fromBoolean(config.debugModeEnabled));
bundle.put(Keys.LOG_LEVEL, config.logLevel.name());
bundle.put(Keys.REGION, config.getRegion().getCode());
bundle.put(Keys.TRACK_APPLICATION_LIFECYCLE, StringUtils.fromBoolean(config.applicationLifecycleTrackingEnabled));
bundle.put(Keys.TEST_MODE_ENABLED, StringUtils.fromBoolean(config.testModeEnabled));
bundle.put(Keys.IN_APP_MESSAGING_ENABLED, StringUtils.fromBoolean(config.inAppMessagingEnabled));
return bundle;
}

Expand All @@ -98,22 +119,29 @@ public static Map<String, String> toMap(@NonNull CustomerIOSDKConfig config) {
private final Integer flushInterval;
@Nullable
private final Integer flushAt;
@Nullable
private final Boolean screenTrackingEnabled;
@Nullable
private final Boolean deviceAttributesTrackingEnabled;
@Nullable
private final Boolean debugModeEnabled;
private final boolean screenTrackingEnabled;
Copy link
Contributor

Choose a reason for hiding this comment

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

The use of wrapper classes here was intentional to differentiate between default values and user-set values. Just wanted to confirm if the switch to primitive classes is deliberate and ensures that all cases are covered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't clear to me that it was intentional so I just wanted to keep it consistent with the new values. Do we still want to keep them as nullable values? Having two getters for each field returning nullable boolean and primitive boolean was too confusing.

Do we still want to differentiate between default values and user values? Not sure if we actually use that distinction anywhere though

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that using object classes adds some management overhead, but primitive types will always resolve to either true or false which can't fully capture the essence of SDK default values. Although it only becomes relevant if SDK's default value changes. I can't think of any other major use cases where it would matter, so as long as saving and restoring values features are working fine, I think using primitives is fine and won't really make a difference in most cases.

private final boolean deviceAttributesTrackingEnabled;
@NonNull
private final CioLogLevel logLevel;
@NonNull
private final Region region;
private final boolean applicationLifecycleTrackingEnabled;
private final boolean testModeEnabled;
private final boolean inAppMessagingEnabled;

public CustomerIOSDKConfig(@NonNull String cdpApiKey,
@NonNull String siteId,
@Nullable String apiHost,
@Nullable String cdnHost,
@Nullable Integer flushInterval,
@Nullable Integer flushAt,
@Nullable Boolean screenTrackingEnabled,
@Nullable Boolean deviceAttributesTrackingEnabled,
@Nullable Boolean debugModeEnabled) {
boolean screenTrackingEnabled,
boolean deviceAttributesTrackingEnabled,
@NonNull CioLogLevel logLevel,
@NonNull Region region,
boolean applicationLifecycleTrackingEnabled,
boolean testModeEnabled,
boolean inAppMessagingEnabled) {
this.cdpApiKey = cdpApiKey;
this.siteId = siteId;
this.apiHost = apiHost;
Expand All @@ -122,7 +150,11 @@ public CustomerIOSDKConfig(@NonNull String cdpApiKey,
this.flushAt = flushAt;
this.screenTrackingEnabled = screenTrackingEnabled;
this.deviceAttributesTrackingEnabled = deviceAttributesTrackingEnabled;
this.debugModeEnabled = debugModeEnabled;
this.logLevel = logLevel;
this.region = region;
this.applicationLifecycleTrackingEnabled = applicationLifecycleTrackingEnabled;
this.testModeEnabled = testModeEnabled;
this.inAppMessagingEnabled = inAppMessagingEnabled;
}

@NonNull
Expand Down Expand Up @@ -155,38 +187,33 @@ public Integer getFlushAt() {
return flushAt;
}


@Nullable
public Boolean isScreenTrackingEnabled() {
public boolean isScreenTrackingEnabled() {
return screenTrackingEnabled;
}

@Nullable
public Boolean isDeviceAttributesTrackingEnabled() {
public boolean isDeviceAttributesTrackingEnabled() {
return deviceAttributesTrackingEnabled;
}

@Nullable
public Boolean isDebugModeEnabled() {
return debugModeEnabled;
@NonNull
public CioLogLevel getLogLevel() {
return logLevel;
}

/**
* Features by default are nullable to help differentiate between default/null values and
* values set by user.
* Unwrapping nullable values here for ease of use by keeping single source of truth for whole
* sample app.
*/
public boolean isTestModeEnabled() {
return testModeEnabled;
}

public boolean screenTrackingEnabled() {
return Boolean.FALSE != screenTrackingEnabled;
public boolean isInAppMessagingEnabled() {
return inAppMessagingEnabled;
}

public boolean deviceAttributesTrackingEnabled() {
return Boolean.FALSE != deviceAttributesTrackingEnabled;
public boolean isApplicationLifecycleTrackingEnabled() {
return applicationLifecycleTrackingEnabled;
}

public boolean debugModeEnabled() {
return Boolean.FALSE != debugModeEnabled;
@NonNull
public Region getRegion() {
return region;
}
}
54 changes: 26 additions & 28 deletions ...layout/src/main/java/io/customer/android/sample/java_layout/sdk/CustomerIORepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
import io.customer.messagingpush.ModuleMessagingPushFCM;
import io.customer.sdk.CustomerIO;
import io.customer.sdk.CustomerIOBuilder;
import io.customer.sdk.core.util.CioLogLevel;
import io.customer.sdk.data.model.Region;

/**
* Repository class to hold all Customer.io related operations at single place
Expand All @@ -31,24 +29,22 @@ public void initializeSdk(SampleApplication application) {
// Initialize Customer.io SDK builder
CustomerIOBuilder builder = new CustomerIOBuilder(application, sdkConfig.getCdpApiKey());

// Enable detailed logging for debug builds.
if (sdkConfig.debugModeEnabled()) {
builder.logLevel(CioLogLevel.DEBUG);
}
// Modify SDK settings for testing purposes only.
// If you don't need to override any of these settings, you can skip this line.
configureSdk(builder, sdkConfig);

// Enable optional features of the SDK by adding desired modules.
// Enables push notification
builder.addCustomerIOModule(new ModuleMessagingPushFCM());
// Enables in-app messages
builder.addCustomerIOModule(new ModuleMessagingInApp(
new MessagingInAppModuleConfig.Builder(sdkConfig.getSiteId(), Region.US.INSTANCE)
.setEventListener(new InAppMessageEventListener(appGraph.getLogger()))
.build()
));

// Modify SDK settings for testing purposes only.
// If you don't need to override any of these settings, you can skip this line.
configureSdk(builder, sdkConfig);
// Enables in-app messages
if (sdkConfig.isInAppMessagingEnabled()) {
builder.addCustomerIOModule(new ModuleMessagingInApp(
new MessagingInAppModuleConfig.Builder(sdkConfig.getSiteId(), sdkConfig.getRegion())
.setEventListener(new InAppMessageEventListener(appGraph.getLogger()))
.build()
));
}

// Finally, build to finish initializing the SDK.
builder.build();
Expand All @@ -72,21 +68,23 @@ private void configureSdk(CustomerIOBuilder builder, final CustomerIOSDKConfig s
builder.cdnHost(cdnHost);
}

if (sdkConfig.getFlushAt() != null) {
builder.flushAt(sdkConfig.getFlushAt());
}
if (sdkConfig.getFlushInterval() != null) {
builder.flushInterval(sdkConfig.getFlushInterval());
if (sdkConfig.isTestModeEnabled()) {
builder.flushInterval(1);
builder.flushAt(1);
mahmoud-elmorabea marked this conversation as resolved.
Show resolved Hide resolved
} else {
if (sdkConfig.getFlushAt() != null) {
builder.flushAt(sdkConfig.getFlushAt());
}
if (sdkConfig.getFlushInterval() != null) {
builder.flushInterval(sdkConfig.getFlushInterval());
mahmoud-elmorabea marked this conversation as resolved.
Show resolved Hide resolved
}
}

final Boolean screenTrackingEnabled = sdkConfig.isScreenTrackingEnabled();
if (screenTrackingEnabled != null) {
builder.autoTrackActivityScreens(screenTrackingEnabled);
}
final Boolean deviceAttributesTrackingEnabled = sdkConfig.isDeviceAttributesTrackingEnabled();
if (deviceAttributesTrackingEnabled != null) {
builder.autoTrackDeviceAttributes(deviceAttributesTrackingEnabled);
}
builder.autoTrackActivityScreens(sdkConfig.isScreenTrackingEnabled());
builder.autoTrackDeviceAttributes(sdkConfig.isDeviceAttributesTrackingEnabled());
builder.trackApplicationLifecycleEvents(sdkConfig.isApplicationLifecycleTrackingEnabled());
builder.region(sdkConfig.getRegion());
builder.logLevel(sdkConfig.getLogLevel());
}

public void identify(@NonNull String email, @NonNull Map<String, String> attributes) {
Expand Down
6 changes: 5 additions & 1 deletion ...ain/java/io/customer/android/sample/java_layout/ui/settings/InternalSettingsActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ private static CustomerIOSDKConfig createNewSettings(CustomerIOSDKConfig current
currentSettings.getFlushAt(),
currentSettings.isScreenTrackingEnabled(),
currentSettings.isDeviceAttributesTrackingEnabled(),
currentSettings.isDebugModeEnabled()
currentSettings.getLogLevel(),
currentSettings.getRegion(),
currentSettings.isApplicationLifecycleTrackingEnabled(),
currentSettings.isTestModeEnabled(),
currentSettings.isInAppMessagingEnabled()
);
}

Expand Down
Loading
Loading