Skip to content

Commit 291b8cf

Browse files
evan-masseauclaude
evan-masseau
and
claude
authored
fix(forms): floating form offset clamping + data-klaviyo-device device info (#443)
* fix(forms): clamp floating form size to honor offsets Bring Android floating-form offset handling in line with iOS. Offsets (plus safe-area insets) are treated as the margin from the screen edge and take priority over the requested form size: if form dimensions plus margins would exceed the screen, the form now shrinks to fit. If it already fits, margins anchor it away from the edge without shrinking. - Introduce calculateLayoutParams() that folds safe-area + user offsets into available width AND height, clamps requested dims via min(), and returns gravity-relative x/y consistent with Android's WindowManager model. - Respect left/right offsets for horizontally-centered positions (TOP, BOTTOM, CENTER) and top/bottom offsets for CENTER by shifting toward the smaller margin (matches iOS calculateFrame semantics). - FULLSCREEN continues to ignore offsets and fill the screen. - Update calculateFormBottomGap() to account for the asymmetric-margin shift on CENTER and for the clamped form height, keeping the keyboard-shift overlap math correct. - Remove the now-dead FormPosition.isHorizontallyCentered() helper. - Expand FloatingFormWindowTest coverage for clamping, asymmetric centering, safe-area + offset interaction, and the FULLSCREEN short-circuit. * fix(forms): clamp form bottom gap to zero Forms taller than the available screen area (or CENTER with heavily asymmetric margins) could produce a negative calculateFormBottomGap value, which in turn over-shifted the flyout when the keyboard opened. Floor the gap at zero so keyboard-shift math stays well-defined. * feat(forms): expose device info to onsite JS via data-klaviyo-device Adds a `data-klaviyo-device` attribute on the in-app forms template head that publishes screen dimensions, safe-area insets, orientation, and device pixel ratio using CSSOM conventions. This lets onsite-in-app compute flexible-form dimensions at HTML parse time without querying potentially-stale `window.screen.*` before view-hierarchy attachment. The attribute is re-published whenever insets change (via the existing `setOnApplyWindowInsetsListener`) and on orientation changes (via the presentation manager's `ConfigurationChanged` handler), so onsite stays in sync with the device state. * fix(forms): harden device-info push thread-safety and test coverage * feat(forms): rename layout offsets key and add addSafeAreaInsetsToOffsets flag Rename the formWillAppear wire key `margin` to `offsets` to match the Android internal naming. Read `offsets` first and fall back to `margin` for backward compatibility with older onsite payloads, emitting a one-time verbose deprecation log the first time the fallback is hit. Add a new optional `addSafeAreaInsetsToOffsets` flag (default true) that lets onsite opt out of the SDK's safe-area handling. When false, the SDK treats offsets as absolute distances from the screen edge and skips safe area entirely for both position math and available-space clamping, so onsite is fully responsible for any safe-area inset it wants to apply. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(forms): defer device-info push to next resumed activity to avoid rotation lag ConfigurationChanged fires before the old activity is destroyed, so Display.rotation/rootWindowInsets still reflect the prior orientation when read from Registry.lifecycleMonitor.currentActivity at that moment. Pushing device info then left `data-klaviyo-device` one rotation behind on every subsequent rotation. Replace the immediate pushDeviceInfo() call in onConfigurationChanged with a one-shot ActivityObserver that waits for the next Resumed event (when the new activity's Display metrics are fresh), with a safety timeout to unregister if the new activity never resumes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(forms): tighten device-info push timeout from 5s to 1s Rotation-to-Resumed is typically sub-300ms. 5s was overspecified and meant stale attribute data could linger for 5s in the pathological destroyed- without-replacement path. 1s comfortably exceeds real recreation time. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(forms): clarify device-info observer skips clearTimers intentionally BugBot flagged that clearTimers() does not reset deviceInfoPushObserver / deviceInfoPushTimeout. That is deliberate — the data-klaviyo-device attribute should stay fresh on the preloaded webview whether or not a form is presenting. dismiss() does not destroy the webview, and pushDeviceInfo already guards against a null webview. Also remove the diagnostic console-log snippet from the HTML template. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): prefer Activity.display on API 30+; silent pushDeviceInfo null branch - DeviceInfo.kt: replace the blanket @Suppress(DEPRECATION) on windowManager.defaultDisplay with a proper version split. Uses the non-deprecated Activity.display on API 30+ (also activity-scoped for multi-display correctness) and falls back to the deprecated API only on API 23-29 where there's no alternative. - KlaviyoWebViewClient.kt: switch pushDeviceInfo to block body and drop the verbose log on the null-webview branch. Null webview is an expected idle state (preload / post-destroy / between register & unregister), not an error — logging on every rotation while idle is noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): drop JS escape layer; inject device info via JSON.stringify Instead of embedding the JSON payload inside a single-quoted JS string literal and escaping single quotes / backslashes, embed the raw JSON as a JS object expression and wrap with JSON.stringify at runtime. JSON is a subset of JS, so the engine parses the object literal natively and then stringifies it back for the attribute value. Removes the jsEscape extension and its two tests — no longer needed. The initial template-replacement path is unchanged (HTML-attribute context was never affected by JS escaping). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): drop legacy margin wire-key fallback now that onsite is migrated Onsite has fully migrated to the 'offsets' wire key in production — no remaining payloads will emit the legacy 'margin' key. Remove the fallback, the one-shot AtomicBoolean deprecation log, and the test-only reset helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): rename marginX locals to offsetX for naming consistency Local variables in FloatingFormWindow.calculateLayoutParams still used the old margin terminology. Renamed to offset to match the wire key and the Offsets data class. Pure rename — no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor(forms): source device-info bounds from currentWindowMetrics on API 30+ Align DeviceInfoProvider with FloatingFormWindow — both now read window dimensions from Activity.windowManager.currentWindowMetrics.bounds on API 30+, falling back to resources.displayMetrics on older APIs. Kills a minor inconsistency where device-info injection and offset math could in theory report different 'screen' sizes in multi-window scenarios. Empirically the two paths produce the same values on modern Android phones in both full-screen and split-screen; this change is about using the canonically-correct API rather than fixing an observed regression. DeviceInfo.from() signature changes: takes screenWidthPx/Height/density primitives instead of a full DisplayMetrics object. Pure-function unit tests simplify accordingly (DisplayMetrics helper no longer needed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1aa33a4 commit 291b8cf

12 files changed

Lines changed: 1154 additions & 307 deletions

File tree

‎sdk/forms/src/main/assets/InAppFormsTemplate.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
data-forms-data-environment='FORMS_ENVIRONMENT'
88
data-klaviyo-local-tracking="1"
99
data-klaviyo-profile="{}"
10+
data-klaviyo-device='DEVICE_INFO'
1011
>
1112
<meta charset="UTF-8">
1213
<meta name="viewport"

‎sdk/forms/src/main/java/com/klaviyo/forms/bridge/DeviceInfo.kt‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
package com.klaviyo.forms.bridge
2+
3+
import android.content.res.Configuration
4+
import android.content.res.Resources
5+
import android.os.Build
6+
import android.view.Surface
7+
import androidx.core.view.WindowInsetsCompat
8+
import androidx.core.view.WindowInsetsCompat.Type.displayCutout
9+
import androidx.core.view.WindowInsetsCompat.Type.systemBars
10+
import com.klaviyo.core.Registry
11+
import kotlin.math.roundToInt
12+
import org.json.JSONObject
13+
14+
/**
15+
* Describes the device's current physical display characteristics, exposed to onsite JS
16+
* via the `data-klaviyo-device` attribute on the HTML `<head>` element.
17+
*
18+
* The shape intentionally mirrors CSSOM conventions so onsite code can treat the payload
19+
* as a reliable, orientation-aware substitute for `window.screen.*` during the synchronous
20+
* HTML parse phase, before the webview attaches to the view hierarchy.
21+
*/
22+
internal data class DeviceInfo(
23+
val screenWidthDp: Int,
24+
val screenHeightDp: Int,
25+
val insetTopDp: Int,
26+
val insetBottomDp: Int,
27+
val insetLeftDp: Int,
28+
val insetRightDp: Int,
29+
val orientation: Orientation,
30+
val dpr: Int
31+
) {
32+
/**
33+
* Serializes the device info into its JSON representation as published on the
34+
* `data-klaviyo-device` head attribute.
35+
*/
36+
fun toJson(): String = JSONObject()
37+
.put(
38+
KEY_SCREEN,
39+
JSONObject()
40+
.put(KEY_WIDTH, screenWidthDp)
41+
.put(KEY_HEIGHT, screenHeightDp)
42+
)
43+
.put(
44+
KEY_SAFE_AREA_INSETS,
45+
JSONObject()
46+
.put(KEY_TOP, insetTopDp)
47+
.put(KEY_BOTTOM, insetBottomDp)
48+
.put(KEY_LEFT, insetLeftDp)
49+
.put(KEY_RIGHT, insetRightDp)
50+
)
51+
.put(KEY_ORIENTATION, orientation.cssValue)
52+
.put(KEY_DPR, dpr)
53+
.toString()
54+
55+
/**
56+
* CSSOM `ScreenOrientation.type` vocabulary.
57+
* See https://drafts.csswg.org/screen-orientation/#enumdef-orientationtype
58+
*/
59+
internal enum class Orientation(val cssValue: String) {
60+
PortraitPrimary("portrait-primary"),
61+
PortraitSecondary("portrait-secondary"),
62+
LandscapePrimary("landscape-primary"),
63+
LandscapeSecondary("landscape-secondary");
64+
65+
companion object {
66+
/**
67+
* Derives the CSSOM orientation label from the Android [Configuration.orientation]
68+
* and [android.view.Display.getRotation] values.
69+
*
70+
* Android's rotation values describe the counter-clockwise rotation applied to the
71+
* natural orientation; pairing that with whether the logical orientation is portrait
72+
* or landscape gives us enough to pick the `*-primary` vs `*-secondary` flavor.
73+
*
74+
* Caveat: this mapping assumes a natural-portrait device, which holds for all phones
75+
* and is sufficient for our product scope. On natural-landscape devices (some tablets
76+
* and foldables) the rotation-to-CSSOM mapping may diverge from
77+
* [`screen.orientation.type`](https://drafts.csswg.org/screen-orientation/#dom-screenorientation-type):
78+
* for example, `rotation = 90` on a natural-landscape device is reported here as
79+
* `portrait-secondary`, where the web platform would report `portrait-primary`.
80+
*/
81+
fun from(configOrientation: Int, rotation: Int): Orientation {
82+
val isPortrait = configOrientation != Configuration.ORIENTATION_LANDSCAPE
83+
return when (rotation) {
84+
Surface.ROTATION_0 -> if (isPortrait) PortraitPrimary else LandscapePrimary
85+
Surface.ROTATION_90 -> if (isPortrait) PortraitSecondary else LandscapePrimary
86+
Surface.ROTATION_180 -> if (isPortrait) PortraitSecondary else LandscapeSecondary
87+
Surface.ROTATION_270 -> if (isPortrait) PortraitPrimary else LandscapeSecondary
88+
else -> if (isPortrait) PortraitPrimary else LandscapePrimary
89+
}
90+
}
91+
}
92+
}
93+
94+
companion object {
95+
private const val KEY_SCREEN = "screen"
96+
private const val KEY_WIDTH = "width"
97+
private const val KEY_HEIGHT = "height"
98+
private const val KEY_SAFE_AREA_INSETS = "safeAreaInsets"
99+
private const val KEY_TOP = "top"
100+
private const val KEY_BOTTOM = "bottom"
101+
private const val KEY_LEFT = "left"
102+
private const val KEY_RIGHT = "right"
103+
private const val KEY_ORIENTATION = "orientation"
104+
private const val KEY_DPR = "dpr"
105+
106+
/**
107+
* Build a [DeviceInfo] snapshot from the given display metrics and insets.
108+
*
109+
* Separating computation from Android platform lookups keeps the logic pure and
110+
* testable. See [DeviceInfoProvider] for the live-device lookup entry point.
111+
*/
112+
fun from(
113+
screenWidthPx: Int,
114+
screenHeightPx: Int,
115+
density: Float,
116+
configuration: Configuration,
117+
rotation: Int,
118+
insetLeftPx: Int,
119+
insetTopPx: Int,
120+
insetRightPx: Int,
121+
insetBottomPx: Int
122+
): DeviceInfo {
123+
// Guard against pathological density (<= 0) which would cause NaN from pxToDp.
124+
// This most commonly occurs in test doubles.
125+
val safeDensity = density.takeIf { it > 0f } ?: 1f
126+
fun pxToDpRounded(px: Int): Int = (px / safeDensity).roundToInt()
127+
return DeviceInfo(
128+
screenWidthDp = pxToDpRounded(screenWidthPx),
129+
screenHeightDp = pxToDpRounded(screenHeightPx),
130+
insetTopDp = pxToDpRounded(insetTopPx),
131+
insetBottomDp = pxToDpRounded(insetBottomPx),
132+
insetLeftDp = pxToDpRounded(insetLeftPx),
133+
insetRightDp = pxToDpRounded(insetRightPx),
134+
orientation = Orientation.from(configuration.orientation, rotation),
135+
dpr = safeDensity.roundToInt().coerceAtLeast(1)
136+
)
137+
}
138+
}
139+
}
140+
141+
/**
142+
* Live-device lookup for [DeviceInfo]. Reads from the currently tracked activity when available,
143+
* falling back to the system-wide resources so early callers (before activity attachment) still
144+
* produce a reasonable snapshot.
145+
*/
146+
internal object DeviceInfoProvider {
147+
148+
/**
149+
* Snapshot the current device state. Prefers values from the tracked activity so that
150+
* safe-area insets and rotation reflect the actual window placement rather than the raw
151+
* display.
152+
*/
153+
fun current(): DeviceInfo {
154+
val activity = Registry.lifecycleMonitor.currentActivity
155+
val resources = activity?.resources ?: Resources.getSystem()
156+
val configuration = resources.configuration
157+
val density = resources.displayMetrics.density
158+
159+
// Source window dimensions from `currentWindowMetrics` on API 30+ so we report the
160+
// activity's actual drawable rect in multi-window scenarios (matches the offset-math
161+
// path in FloatingFormWindow). Fall back to resources.displayMetrics on older APIs,
162+
// which is activity-scoped and already window-aware post-N in practice.
163+
val (screenWidthPx, screenHeightPx) = runCatching {
164+
if (activity != null && Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
165+
val bounds = activity.windowManager.currentWindowMetrics.bounds
166+
bounds.width() to bounds.height()
167+
} else {
168+
resources.displayMetrics.let { it.widthPixels to it.heightPixels }
169+
}
170+
}.getOrElse { resources.displayMetrics.let { it.widthPixels to it.heightPixels } }
171+
172+
val rotation = runCatching {
173+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
174+
activity?.display
175+
} else {
176+
@Suppress("DEPRECATION")
177+
activity?.windowManager?.defaultDisplay
178+
}?.rotation ?: Surface.ROTATION_0
179+
}.getOrDefault(Surface.ROTATION_0)
180+
181+
data class InsetsPx(val left: Int, val top: Int, val right: Int, val bottom: Int)
182+
val insetsPx = runCatching {
183+
activity?.window?.decorView?.rootWindowInsets?.let { raw ->
184+
val compat = WindowInsetsCompat.toWindowInsetsCompat(raw)
185+
val insets = compat.getInsets(systemBars() or displayCutout())
186+
InsetsPx(insets.left, insets.top, insets.right, insets.bottom)
187+
}
188+
}.getOrNull() ?: InsetsPx(0, 0, 0, 0)
189+
190+
return DeviceInfo.from(
191+
screenWidthPx = screenWidthPx,
192+
screenHeightPx = screenHeightPx,
193+
density = density,
194+
configuration = configuration,
195+
rotation = rotation,
196+
insetLeftPx = insetsPx.left,
197+
insetTopPx = insetsPx.top,
198+
insetRightPx = insetsPx.right,
199+
insetBottomPx = insetsPx.bottom
200+
)
201+
}
202+
}

‎sdk/forms/src/main/java/com/klaviyo/forms/bridge/FormLayout.kt‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,6 @@ internal enum class FormPosition {
1616
BOTTOM_RIGHT,
1717
CENTER;
1818

19-
/**
20-
* Returns true if this position uses horizontal centering (CENTER_HORIZONTAL or CENTER).
21-
* These positions need width adjusted for horizontal safe area insets so forms
22-
* don't extend into display cutouts in landscape.
23-
*/
24-
fun isHorizontallyCentered(): Boolean = when (this) {
25-
TOP, BOTTOM, CENTER, FULLSCREEN -> true
26-
TOP_LEFT, TOP_RIGHT, BOTTOM_LEFT, BOTTOM_RIGHT -> false
27-
}
28-
2919
/**
3020
* Convert form position to Android Gravity flags
3121
*/
@@ -147,7 +137,14 @@ internal data class FormLayout(
147137
val position: FormPosition,
148138
val width: Dimension,
149139
val height: Dimension,
150-
val offsets: Offsets = Offsets()
140+
val offsets: Offsets = Offsets(),
141+
/**
142+
* When true (default), the SDK adds safe-area insets to the provided [offsets]
143+
* when positioning the form. When false, the SDK uses [offsets] as-is and does
144+
* not account for safe-area at all — onsite is fully responsible for baking
145+
* any safe-area inset it wants into [offsets].
146+
*/
147+
val addSafeAreaInsetsToOffsets: Boolean = true
151148
) {
152149
/**
153150
* Returns true if this layout represents a fullscreen form
@@ -162,13 +159,16 @@ internal data class FormLayout(
162159
val position = FormPosition.fromString(json.optString("position"))
163160
val width = Dimension.fromJson(json.optJSONObject("width")) ?: return null
164161
val height = Dimension.fromJson(json.optJSONObject("height")) ?: return null
165-
val offsets = Offsets.fromJson(json.optJSONObject("margin"))
162+
163+
val offsets = Offsets.fromJson(json.optJSONObject("offsets"))
164+
val addSafeAreaInsetsToOffsets = json.optBoolean("addSafeAreaInsetsToOffsets", true)
166165

167166
return FormLayout(
168167
position = position,
169168
width = width,
170169
height = height,
171-
offsets = offsets
170+
offsets = offsets,
171+
addSafeAreaInsetsToOffsets = addSafeAreaInsetsToOffsets
172172
)
173173
}
174174
}

0 commit comments

Comments
 (0)