chore: Improve public settings screen UX #453

mahmoud-elmorabea · 2024-10-10T20:34:53Z

This PR is one of series of PRs that update the UX of the settings page for the Java_Layout sample app.

PR stack:

Create a new internal settings screen -> chore: Create internal settings screen #452
Update public settings screen -> This PR
Update theme to new brand -> chore: Update sample app theme to new brand #454

Notes:

I will collect all changes in a base branch update-settings-page-ux and then merge them as one chore to the main branch
Choosing chore not to trigger an SDK release when this is merged

The general idea of the improvements is to have our settings screen map as close as possible to the configuration of our SDK when it's being initialized. You can see how closely the fields in this public settings screen map to the config:

The UI was tested on multiple devices

Phone - API 35	Phone - API 21	Tablet

Video of how it works

public-settings-page.webm

github-actions · 2024-10-10T20:35:06Z

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

java_layout: update-public-settings-screen (1729000443)
kotlin_compose: update-public-settings-screen (1729000445)

codecov · 2024-10-10T20:37:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (update-settings-page-ux@ffa1f75). Learn more about missing BASE report.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             update-settings-page-ux     #453   +/-   ##
==========================================================
  Coverage                           ?   51.21%           
  Complexity                         ?      287           
==========================================================
  Files                              ?       95           
  Lines                              ?     2269           
  Branches                           ?      353           
==========================================================
  Hits                               ?     1162           
  Misses                             ?     1007           
  Partials                           ?      100

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mahmoud-elmorabea · 2024-10-10T20:36:47Z

...ayout/src/main/java/io/customer/android/sample/java_layout/ui/settings/SettingsActivity.java

+        ViewUtils.prepareForAutomatedTests(binding.settingsCdpApiKeyLabel, R.string.acd_cdp_api_key_input);
+        ViewUtils.prepareForAutomatedTests(binding.settingsSiteIdKeyLabel, R.string.acd_site_id_input);
+        ViewUtils.prepareForAutomatedTests(binding.settingsSaveButton, R.string.acd_save_settings_button);
+        ViewUtils.prepareForAutomatedTests(binding.settingsRestoreDefaultsButton, R.string.acd_restore_default_settings_button);


@Shahroz16 @mrehan27 I couldn't find those UI tests, do I need to update something for the new fields?

The tests are available in a separate Appium repository. We haven't maintained them in a while though, but having unique and consistent values across all sample apps should be sufficient for now.

mahmoud-elmorabea · 2024-10-10T20:37:17Z

...ayout/src/main/java/io/customer/android/sample/java_layout/ui/settings/SettingsActivity.java

    }

    private void setupObservers() {
-
-        binding.deviceTokenTextInput.setText(CustomerIO.instance().getRegisteredDeviceToken());


Moved to internal settings screen

The device token location should move to the main screen under the email.

@scotttwittrockcio maybe we can add it there for convenience but also keep it in the internal settings?

I don't think it's needed in both places. It's a really useful piece of information when testing push messages. It helps me quickly validate the device id matches the one in fly, and the one I'm using to send a test push to.

mahmoud-elmorabea · 2024-10-10T20:37:22Z

...ayout/src/main/java/io/customer/android/sample/java_layout/ui/settings/SettingsActivity.java

        settingsViewModel.getSDKConfigObservable().observe(this, config -> {
            binding.progressIndicator.hide();
            updateIOWithConfig(config);
            parseLinkParams();
        });
    }

-    private boolean isHostURLInvalid(String url) {


Moved to internal settings screen

github-actions · 2024-10-10T20:39:28Z

📏 SDK Binary Size Comparison Report

No changes detected in SDK binary size ✅

github-actions · 2024-10-10T20:39:53Z

Build available to test
Version: update-public-settings-screen-SNAPSHOT
Repository: https://s01.oss.sonatype.org/content/repositories/snapshots/

mrehan27 · 2024-10-15T13:03:30Z

...out/src/main/java/io/customer/android/sample/java_layout/data/model/CustomerIOSDKConfig.java

-    private final Boolean deviceAttributesTrackingEnabled;
-    @Nullable
-    private final Boolean debugModeEnabled;
+    private final boolean screenTrackingEnabled;


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.

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

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.

...va_layout/src/main/java/io/customer/android/sample/java_layout/sdk/CustomerIORepository.java

mrehan27

Everything looks good, except for what Scott suggested in his comment. Looks like device token is missing from public settings screen. Is there a separate PR for that change?

mahmoud-elmorabea · 2024-10-15T17:00:01Z

@mrehan27 Yes, I will have follow up PRs for changes to login page and other parts

mahmoud-elmorabea commented Oct 10, 2024

View reviewed changes

mahmoud-elmorabea mentioned this pull request Oct 10, 2024

chore: Create internal settings screen #452

Merged

mahmoud-elmorabea self-assigned this Oct 10, 2024

mahmoud-elmorabea mentioned this pull request Oct 10, 2024

chore: Update sample app theme to new brand #454

Merged

mahmoud-elmorabea marked this pull request as ready for review October 10, 2024 21:12

mahmoud-elmorabea requested a review from a team October 10, 2024 21:12

Base automatically changed from create-internal-settings-screen to update-settings-page-ux October 14, 2024 13:19

mahmoud-elmorabea added 4 commits October 14, 2024 16:21

Add new config attributes to CustomerIOSDKConfig

25514b3

Update SDK init to use the new attributes in CustomerIOSDKConfig

c42ef04

Update public settings screen with new UI

993a5d4

Update SettingsActivity to use the updated UI binding

8f5d2ac

mahmoud-elmorabea force-pushed the update-public-settings-screen branch from 97bf238 to 8f5d2ac Compare October 14, 2024 13:22

mrehan27 requested changes Oct 15, 2024

View reviewed changes

Adjust flushAt and flushInterval values

2cad68b

mahmoud-elmorabea requested a review from mrehan27 October 15, 2024 13:53

mrehan27 reviewed Oct 15, 2024

View reviewed changes

mahmoud-elmorabea requested a review from mrehan27 October 15, 2024 17:00

mrehan27 approved these changes Oct 15, 2024

View reviewed changes

mahmoud-elmorabea merged commit c89791b into update-settings-page-ux Oct 16, 2024
33 of 35 checks passed

mahmoud-elmorabea deleted the update-public-settings-screen branch October 16, 2024 14:10

mahmoud-elmorabea mentioned this pull request Oct 18, 2024

chore: Update settings page UX #455

Merged

mahmoud-elmorabea added a commit that referenced this pull request Oct 18, 2024

chore: Improve public settings screen UX (#453)

7244d3d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Improve public settings screen UX #453

chore: Improve public settings screen UX #453

mahmoud-elmorabea commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading

mahmoud-elmorabea Oct 10, 2024

mrehan27 Oct 15, 2024

mahmoud-elmorabea Oct 10, 2024

scotttwittrockcio Oct 11, 2024

mahmoud-elmorabea Oct 11, 2024

scotttwittrockcio Oct 11, 2024

mahmoud-elmorabea Oct 10, 2024

github-actions bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024

mrehan27 Oct 15, 2024

mahmoud-elmorabea Oct 15, 2024

mrehan27 Oct 15, 2024

mrehan27 left a comment

mahmoud-elmorabea commented Oct 15, 2024

chore: Improve public settings screen UX #453

chore: Improve public settings screen UX #453

Conversation

mahmoud-elmorabea commented Oct 10, 2024 • edited Loading

The UI was tested on multiple devices

Video of how it works

github-actions bot commented Oct 10, 2024 • edited Loading

Sample app builds 📱

codecov bot commented Oct 10, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 10, 2024 • edited Loading

📏 SDK Binary Size Comparison Report

github-actions bot commented Oct 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrehan27 left a comment

Choose a reason for hiding this comment

mahmoud-elmorabea commented Oct 15, 2024

mahmoud-elmorabea commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

codecov bot commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading