fix: remove secondaryToolbarColor - #22
Conversation
There was a problem hiding this comment.
Review: remove secondaryToolbarColor
Clean, well-scoped deletion. I independently verified the premise, the completeness, and the "no release impact" claim rather than taking them on faith — all three hold up.
The premise checks out
CustomTabColorSchemeParams.setSecondaryToolbarColor() only paints the bottom bar created by setSecondaryToolbarViews(). I grepped the whole repo for SecondaryToolbarViews, RemoteViews, setActionButton, and addMenuItem — none of them appear anywhere. So there is genuinely no code path in which this color could have rendered, which matches the on-device result described. Removing rather than implementing is the right call: a functional secondary toolbar needs RemoteViews + click PendingIntents, which is a large API surface to expose inside a payment flow for cosmetic gain.
It is also consistent with precedent already set in this codebase — swift/Sources/DodoCheckout/BrowserCustomization.swift deliberately omits preferredBarTintColor/preferredControlTintColor for exactly the same "confirmed inert on the platform, not worth shipping" reason. Good to see the same standard applied to Android.
Completeness
- Case-insensitive repo-wide grep for
secondarytoolbarreturns zero hits on the head branch. Also checked prose (secondary,bottom bar,bottom toolbar) across.md/.dart/.kt/.tsx/.ts/.swift— clean, no dangling doc references. - Pigeon: schema and all three generated targets (
messages.g.dart,Messages.g.kt,Messages.g.swift) are regenerated in lockstep. I diffed the positional decode indices field-by-field — all three now read0..10in identical order, matching the schema declaration order.toList/fromList,deepEquals,hashCode, andtoStringwere all updated consistently in each target, which is what a real regeneration produces rather than a hand-edit. - Kotlin: the guard in
applyBrowserCustomizationwas narrowed correctly —colorSchemeParamsis still built only when at least one remaining color is non-null, so no emptysetDefaultColorSchemeParamscall is introduced. - Tests: the Kotlin
roundTripsThroughStringMapand the Flutter mapping test still assert every one of the 11 remaining fields, so coverage was trimmed rather than weakened.
Release-impact claim verified
Latest tags are flutter-v1.0.4, react-native-v1.1.2, kotlin-v1.0.2, against in-repo versions 1.1.0 / 1.2.0 / 1.1.0 respectively. The field only ever existed in unreleased code, so this is correctly handled as a pre-release trim of the feature summary — no version bump and no "Removed" changelog entry needed. Editing the existing ## 1.1.0 / ## 1.2.0 entries in place is the right treatment here.
Verification run
React Native tsc --noEmit is clean and all 31 Jest tests pass. I could not run the Kotlin or Flutter suites (no JDK or Dart toolchain in this environment) — relying on CI and the author's reported results for those.
Non-blocking follow-up
All three READMEs delegate the full option list to the hosted docs (docs.dodopayments.com/.../appearance-customization) rather than enumerating fields inline. Those pages are outside this repo but likely still list secondaryToolbarColor, so they will need the same trim around merge/release time — worth tracking separately so the published docs don't advertise a field the SDK no longer accepts.
Also worth noting (no action needed): on React Native the customization object is forwarded via JSON.stringify, so a consumer building it dynamically could still include a stale secondaryToolbarColor key. The Android bridge simply ignores unknown keys, and the Kotlin Bundle decoder documents unrecognized entries as decoding to null, so this degrades gracefully rather than throwing — including across saved-instance-state restore.
Approving.
637a8e0 to
e9c4f0d
Compare
secondaryToolbarColorsetsCustomTabColorSchemeParams.setSecondaryToolbarColor(), but this SDK never callssetSecondaryToolbarViewsto actually create a secondary (bottom) toolbar — so the color has nothing to paint, unconditionally, regardless of any other setting. Confirmed live on a real device (set the field, checked the rendered Custom Tab — no visible effect in any configuration), unlike the rest of the customization surface which was verified working field-by-field on both Android and iOS.Removed from
BrowserCustomizationin kotlin, react-native, and flutter (Pigeon schema + regenerated codegen). None of these packages have been tagged/released yet, so this is a clean pre-release removal — no version bump or changelog "removed" note needed, just trimmed from the still-unreleased 1.1.0/1.2.0 feature summaries.Verified: kotlin unit tests, RN tsc + jest + Android bridge compile, flutter unit tests + real Android/iOS demo builds — all green.