Use InAppMessage dimensions relative to host activity #696
Conversation
- implementation of fix in adobe#618
| val currentActivity = LocalContext.current as? Activity ?: run { | ||
| onDisposed() | ||
| Log.debug(ServiceConstants.LOG_TAG, "MessageFrame", "Unable to get the current activity. Dismissing the message.") | ||
| return | ||
| } |
There was a problem hiding this comment.
Can you use this approach for finding the activity as it is safer https://github.com/adobe/aepsdk-assurance-android/blob/main/code/assurance/src/main/java/com/adobe/marketing/mobile/assurance/internal/ui/ContextExt.kt#L21
| * An extension for finding the activity from a context. | ||
| */ | ||
| internal fun Context.findActivity(): Activity? { | ||
| var context = this | ||
| while (context is ContextWrapper) { | ||
| if (context is Activity) return context | ||
| context = context.baseContext | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
Since this is only used in the UI area, can we make this a file private function inside MessageFrame.kt for now? We can move this to broader utils later when we find its uses at other places.
There was a problem hiding this comment.
sounds good, will move it
|
I'll share a snapshot once I get the android tests fixed |
| // multi-window modes. | ||
| val contentView = currentActivity.findViewById<View>(android.R.id.content) | ||
| val contentHeightDp = with(LocalDensity.current) { contentView.height.toDp() } | ||
| val contentWidthDp = with(LocalDensity.current) { contentView.width.toDp() } |
There was a problem hiding this comment.
There are a bunch of tests in MessageScreenTests which assert for the IAM height and width based on the screen height and width. Could you please check if those need to be fixed?
rymorale
force-pushed
the
MOB-21325-iam-edge-to-edge-fix
branch
from
fedafac to
3da2923
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #696 +/- ##
============================================
- Coverage 81.66% 81.60% -0.06%
Complexity 2150 2150
============================================
Files 192 192
Lines 8936 8944 +8
Branches 1118 1122 +4
============================================
+ Hits 7297 7298 +1
- Misses 1085 1089 +4
- Partials 554 557 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
I'm releasing a snapshot from the dev branch to share with the issue reporter |
Implementation of fix in #618. The description from that pr is:
Description
Problem : Currently, the size of the InAppMessage is based on the screen height and width. This displays the IAM ass desired for all single activity cases. However, this is not always the case when the activity is not occupying the entire screen.
Such cases will see a full screen message within the activity irrespective of the dimensions of the actual content size of
the activity. So 70% height would be 70% screen height and exceed the activity content view height where as it should have been 70% of the height within the activity.
Solution: Use the Use InAppMessage dimensions relative to host activity and not the entire screen.
Related Issue
Motivation and Context
Problem : Currently, the size of the InAppMessage is based on the screen height and width. This displays the IAM ass desired for all single activity cases. However, this is not always the case when the activity is not occupying the entire screen.
Such cases will see a full screen message within the activity irrespective of the dimensions of the actual content size of
the activity. So 70% height would be 70% screen height and exceed the activity content view height where as it should have been 70% of the height within the activity.
Solution: Use the Use InAppMessage dimensions relative to host activity and not the entire screen.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: