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

Add AndroidManifest option to override In-App Messages gray overlay and dropshadow #2051

Merged
merged 3 commits into from
Apr 15, 2024

Conversation

jennantilla
Copy link
Contributor

@jennantilla jennantilla commented Apr 12, 2024

Description

One Line Summary

Add AndroidManifest option to override In-App Messages gray overlay and dropshadow.

Details

Customers would like the ability to control some In-App Message display "defaults":

  • a dark gray background in center modal and full screen IAMs
  • a dropshadow on all IAMs

Customers would like the option to disable this overlay.

With this PR, developers can now add com.onesignal.inAppMessageHideGrayOverlay and com.onesignal.inAppMessageHideDropShadow to AndroidManifest.xml. Setting these values to true will not show a shadow or gray overlay. Omitting these values or setting them to false will display the default In-App design with drop shadow and gray overlay.

<meta-data android:name="com.onesignal.inAppMessageHideGrayOverlay" android:value="true"/> 
<meta-data android:name="com.onesignal.inAppMessageHideDropShadow" android:value="true"/>

Examples without the dropshadow

Example with the dropshadow

Example with the gray overlay

Example without the gray overlay

Scope

New and existing In-App Message visual behavior.

Other

In the future, we can further consider the use case of having a drop shadow and transparent background:

Currently, the drop shadow is for the entire IAM, including the transparent parts. The question would be whether this is expected, or a shadow around each visible element only.

Future work could include aligning Android and iOS appearance based on whatever is deemed "expected".

Testing

Manual testing

Tested on all IAM types with a Pixel 7 Pro emulator running Android 14 without AndroidManifest meta-data, with meta-data set to true, and with meta-data set to false.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

@@ -626,11 +641,17 @@ internal class InAppMessageView(
}
}

var overlayColor = Color.parseColor("#BB000000")

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 block of code repeats twice, we can make it into a method and call that method when we need to get the color.

Color.parseColor("#00000000") below is also the transparent color. We can consolidate into one variable and prefer Color.TRANSPARENT for clarity.

- separate out overlay color getter into its own method to reduce redundancy
- update wording for dropsahdow getter for succinctness
- add back constants to companion object
@jennantilla jennantilla requested a review from nan-li April 12, 2024 21:16
@jkasten2 jkasten2 merged commit c3ba173 into main Apr 15, 2024
2 checks passed
@jkasten2 jkasten2 deleted the feat/iam_toggle branch April 15, 2024 20:49
@jkasten2 jkasten2 mentioned this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants