[SDK-338] Inbox Header Customization

[franco-zalamena-iterable]

🔹 Jira Ticket(s) if any

• SDK-338

✏️ Description

This Pr is meant to add easy out of the box customization for the Inbox Header.
Currently we only have a modifiable title in the inbox, which is a bit limited, if someone just wants a back button that requires a whole wrapper on the fragment or recreating the whole fragment just for that.
To address this concern it is now possible to opt-in two pre built headers and customizable one.

• Default: Just title
• WithBackButton: Title and a left back button
• Custom: You can pass a layout resource to be rendered in the header, this layout can contain Iterable specific ids to add actions and automatic customization from the fragment

Default	WithBackButton	None (Same as before)

---

Things to consider:

• Backward compatible: None is the default for every existing entry point, current integrators see no change.
• Tests: 13 unit tests in iterableapi-ui (Robolectric, new module test harness). Covers all four variants and apply() state transitions.

[sumeruchat]

Thanks Franco — the sealed InboxToolbarOption API shape with None / Default / WithBackButton / Custom(@LayoutRes) and the opt-in id convention for custom layouts feel right, and the test suite passes locally. Two blockers below that need to land before merge, then some non-blockers, then a separate API-design section I'd love @davidtruong input on per your note before we lock the public surface.

---

🚫 Blockers

1. (P0) Default None path renders a blank inbox.

iterable_inbox_fragment.xml changed the RecyclerView from layout_height="match_parent" to:

android:layout_height="0dp"
android:layout_below="@id/iterable_inbox_toolbar"
android:layout_alignParentBottom="true"

When InboxToolbarOption is None (the default for every existing entry point), IterableInboxToolbarView.apply sets the toolbar to View.GONE. RelativeLayout.getRelatedView() walks the dependency chain looking for a non-GONE substitute; the toolbar has no layout_below of its own, so the lookup returns null and the layout_below rule on the RecyclerView is effectively dropped. That leaves the RecyclerView with only alignParentBottom=true and 0dp height with no top constraint → it collapses to a 0-height strip at the bottom.

This hits all pre-existing callers — newInstance(), the (InboxMode, layoutId) overload, the (InboxMode, layoutId, noMsgTitle, noMsgBody) overload, and IterableInboxActivity launched with no toolbar extras. Directly contradicts the stated backward-compat goal.

Two clean fixes:

• Keep match_parent on the list and only add the layout_below constraint when the toolbar is actually visible (e.g., in code when applying a non-None option), or
• Restructure the root as a vertical LinearLayout where a GONE toolbar takes no space and the list gets layout_weight="1" / match_parent.

Either way, please add a Robolectric test that inflates IterableInboxFragment.newInstance() and asserts the list measures to non-zero height under None — this would have caught the regression.

2. (P1) MaterialToolbar is inflated for every host, even when None is selected.

IterableInboxToolbarView is always present in iterable_inbox_fragment.xml, and its constructor unconditionally inflates iterable_inbox_toolbar.xml containing a MaterialToolbar (IterableInboxToolbarView.kt:35-38) — apply(None) then hides the result. So every existing host now needs an AppCompat-descendant theme just to inflate the fragment, even if they never opt into the toolbar.

The view's KDoc says AppCompat is required "when the opt-in toolbar is enabled" — that's not what the code does; the requirement is paid at fragment inflation regardless. IterableInboxActivity is unaffected (already AppCompatActivity), but customers embedding IterableInboxFragment in non-AppCompat hosts will start getting InflateException at runtime with no migration path.

Fix: the None path should not inflate MaterialToolbar. Lazy-inflate the toolbar layout on first non-None apply(...) call, or split the layout so None doesn't include the view at all.

---

Non-blocking (P2)

3. Standalone IterableInboxToolbarView back-fallback breaks through ContextThemeWrapper.

dispatchBackClick falls back via (context as? ComponentActivity)?.onBackPressedDispatcher (IterableInboxToolbarView.kt:85-92). Views are commonly inflated under a ContextThemeWrapper (including, FWIW, exactly the pattern the new tests use), so that cast often fails even when the base context is a ComponentActivity. Fragment usage is unaffected because the fragment always installs an override, but the public standalone-view docs promise this fallback works. Either unwrap ContextWrappers until you find a ComponentActivity, or drop the fallback from the docs.

4. Test coverage misses the at-risk paths.

The new tests cover serialization round-tripping and direct apply(...) state transitions on the view. They don't inflate IterableInboxFragment.newInstance() (or the old overloads) or IterableInboxActivity with no extras to assert the list still measures/fills under None. Adding those would have caught blocker #1, and similar fragment-host tests would have caught #2. Also missing: Bundle/Intent saved-state restore, missing/extra custom ids, parent-fragment listener discovery, and the standalone fallback under ContextThemeWrapper.

---

API design — would love @david Truong's input before this freezes

(All non-blocking; calling out because the public surface is hard to change once shipped.)

5. data object variants don't preserve singleton identity across Bundle/Intent restore.

There's no generated readResolve on None/Default/WithBackButton (verifiable with javap), so Bundle/Intent round-trips return non-identical instances. Kotlin equality + when keep working, and Custom(layoutRes) round-trips the layoutRes intact, so the SDK code is fine. The risk is Java consumers writing option == InboxToolbarOption.None.INSTANCE after restore — that silently breaks. Either add readResolve() overrides on the data objects, or document explicitly that consumers should use .equals() / instanceof.

6. SDK-reserved ids in custom layouts.

@id/iterable_inbox_back_button and @id/iterable_inbox_title are looked up via findViewById scoped to the inflated custom toolbar — so this isn't a broad app-wide collision risk, but any view the integrator names with those ids inside their custom layout gets auto-wired whether they meant it or not. Should be documented as reserved opt-in ids in the public KDoc. Same goes for the deliberate findViewById<View>(R.id.iterable_inbox_title) as? TextView casting accepting any TextView subclass (Button, EditText, etc.) — defensible, but document as "any TextView subclass."

7. Toolbar back-press semantics for embedded fragments.

The fragment's default back listener delegates to requireActivity().getOnBackPressedDispatcher().onBackPressed(), which triggers the full back-press chain (any registered OnBackPressedCallback, fragment pop, activity finish, etc.). For IterableInboxActivity that's fine — back === finish. For embedded fragments it's opinionated; a toolbar back arrow usually means "up" (popBackStack / NavigationUI.navigateUp), not "system back."

Two reasonable directions:

• Document that integrators embedding the fragment with WithBackButton should implement IterableInboxToolbarBackListener if they want anything other than full system-back semantics, or
• Make IterableInboxToolbarBackListener required (rather than optional) when WithBackButton is used in an embedded fragment, and throw / log if missing.

8. Public surface is wider than the option API needs.

InboxToolbarOption (sealed), IterableInboxToolbarView (class), and IterableInboxToolbarBackListener (interface) are all public. Custom's data-class copy/component1 are also on the public JVM surface. If the supported API is only "pass an option to the fragment/activity," consider @RestrictTo(LIBRARY) on IterableInboxToolbarView (or document standalone use as a supported pattern). Also: public sealed InboxToolbarOption means adding a new variant later breaks exhaustive Kotlin when consumers at runtime — worth a deliberate decision.

---

Verified clean / context

• AppCompat-only (no full Material Components) hosts inflate the toolbar fine after the colorSurface drop — Robolectric test passes under Theme.AppCompat.Light.NoActionBar. The issue is that hosts pay the AppCompat cost even when they didn't opt in (see blocker #2).
• Listener discovery (onAttach parent-first → context-second) is the standard pattern; survives config change. When both parent fragment and host activity implement the interface, the parent silently wins — fine, just document.
• apply(Custom(...)) always removeAllViews() + re-inflates, dropping any state/listeners on previously-inflated custom views. Probably fine for fragment-init usage; worth documenting for standalone/subclass use.
• 13 unit tests pass locally (testDebugUnitTest --tests InboxToolbarOptionTest --tests IterableInboxToolbarViewTest).

[franco-zalamena-iterable]

I think regarding

7. Toolbar back-press semantics for embedded fragments.

This is the most straight forward implementation, there is an option to override it if the developer wants to, but the default behavior being "goes back" makes sense for me, if someone is implementing it in a root level (navigation bar entry) they probably won't use the top bar with the back button anyway

8. Public surface is wider than the option API needs.
   I think this is not an issue, the developers need access to the properties to choose what they want to use and they can use it in different places also, let me know what you think

I addressed the readResolve part, i tried to keep it out because if you check with assertEquals it works currently it only fails with assertsSame(), but it's better to be more safe just in case, i just wanted to keep the class less bloated

Regarding the ids, i tried to give a naming that makes it really hard for it to accidentally be overwriten

[franco-zalamena-iterable]

I think regarding

7. Toolbar back-press semantics for embedded fragments.

This is the most straight forward implementation, there is an option to override it if the developer wants to, but the default behavior being "goes back" makes sense for me, if someone is implementing it in a root level (navigation bar entry) they probably won't use the top bar with the back button anyway

8. Public surface is wider than the option API needs.
   I think this is not an issue, the developers need access to the properties to choose what they want to use and they can use it in different places also, let me know what you think

I addressed the readResolve part, i tried to keep it out because if you check with assertEquals it works currently it only fails with assertsSame(), but it's better to be more safe just in case, i just wanted to keep the class less bloated

Regarding the ids, i tried to give a naming that makes it really hard for it to accidentally be overwriten

[sumeruchat]

Thanks Franco — the rework is thorough and the P0/P1 blockers from the previous round are cleanly resolved. Approving.

Important — please do not merge until @davidtruong has signed off on the public API shape (items #4 and #5 below). The blockers are fixed, but the items I deferred to David's review last round are still untouched in these commits, and they're hard to change once shipped.

✅ Verified fixed

• P0 layout collapse: layout migrated from RelativeLayout (with the layout_below/alignParentBottom/0dp combo that collapsed when toolbar was GONE) to vertical LinearLayout + FrameLayout content area with layout_weight="1". Under None the toolbar takes 0 space and the RecyclerView fills correctly. All four option transitions (Default → Custom → Default, Default → None → Default, etc.) verified clean — showCustomLayout() nulls materialToolbar so the next showDefaultLayout() correctly re-inflates the SDK toolbar.
• P1 unconditional MaterialToolbar inflation: init {} block removed; materialToolbar is nullable and only inflated on first non-None apply(...). Plain AppCompat hosts using InboxToolbarOption.None no longer pay the Material inflate cost.
• API #5 data object identity across serialization: private fun readResolve(): Any = X added to None / Default / WithBackButton. javap confirms generated; unit tests round-trip each through ObjectOutputStream + assertSame. Custom(layoutRes) round-trips with layoutRes intact.
• API #6 SDK-reserved id collision risk: iterable_inbox_back_button → iterable_reserved_inbox_toolbar_action, iterable_inbox_title → iterable_reserved_inbox_toolbar_title. Repo-wide grep finds zero remaining references to the old names. KDoc updated to call them out as "Reserved opt-in ids ... deliberately namespaced; do not reuse them on unrelated views."
• Standalone ContextThemeWrapper fallback (P2 from previous round): documented as a limitation rather than code-fixed. Fragment hosting unaffected since the fragment always installs its own listener. Consistent.

All 15 iterableapi-ui unit tests pass locally.

---

Non-blocking follow-ups

1. (P2) Test coverage for backward-compat default path is partial.
IterableInboxFragmentTest covers IterableInboxFragment.newInstance() under None and asserts recyclerView.measuredHeight > 0, which catches the P0 regression. Still uncovered (and the original P0 would still slip past these gaps if it re-regressed in a subtler way):

• Measured height equality with the fragment/content height (not just > 0)
• Old newInstance(InboxMode, layoutId) and newInstance(InboxMode, layoutId, noMsgTitle, noMsgBody) overloads
• IterableInboxActivity launched with no toolbar extras
• Custom layout with both reserved ids present (auto-wired)
• Parent-fragment listener discovery (only host-activity case is exercised)

Manual code review confirms the old overloads still land on InboxToolbarOption.None (they don't set TOOLBAR_OPTION so the field default applies), and IterableInboxActivity defaults a missing extra to None before calling the 6-arg overload — so the runtime should be correct. The gap is in regression coverage, not in the fix.

2. (P3) Empty-state isn't pixel-identical to master.
The pre-PR layout centered emptyInboxTitle in the parent and placed emptyInboxMessage directly below it. The new layout wraps both in a centered vertical LinearLayout. Likely fine, but if anyone has screenshot tests on the empty-inbox screen, this would be a small observable shift. Worth a note in the changelog if so.

---

🚦 Awaiting @davidtruong before merge (API-design items from previous round, untouched in this rework)

3. Embedded-fragment back-press semantics.
IterableInboxFragment's default toolbar back action still calls requireActivity().getOnBackPressedDispatcher().onBackPressed() — the full system-back chain (any registered OnBackPressedCallback, fragment pop, activity finish, etc.). Fine for IterableInboxActivity; opinionated for embedded fragments where the conventional toolbar back is an "up" nav (NavigationUI.navigateUp / popBackStack). David, would you weigh in on whether this default is the right one, or whether we should require integrators to wire IterableInboxToolbarBackListener when WithBackButton is used in an embedded fragment?

4. Public API surface width.
InboxToolbarOption (sealed), IterableInboxToolbarView (class), and IterableInboxToolbarBackListener (interface) are all in the public package. Custom(@LayoutRes layoutRes: Int) is a data class so copy/component1 are also in public JVM bytecode. If the supported API is only "pass an option to the fragment/activity," consider @RestrictTo(LIBRARY) on IterableInboxToolbarView (or explicitly support standalone usage as a documented pattern). Also worth a deliberate decision: public sealed InboxToolbarOption means adding future variants can break exhaustive Kotlin when consumers at runtime. David — your call here matters because it's hard to narrow once shipped.

---

cc @davidtruong — please weigh in on (3) and (4) above before this merges. The implementation is solid and the integrator-facing API surface (InboxToolbarOption + the reserved-id convention for Custom) feels right to me, but the back-press default semantics and the @RestrictTo decision are forward-incompatible once we ship.

Static review + targeted local unit-test run only.