Add 300x250 ad placements and an ad placement priority system - #98
Open
arch10 wants to merge 8 commits into
Open
Add 300x250 ad placements and an ad placement priority system#98arch10 wants to merge 8 commits into
arch10 wants to merge 8 commits into
Conversation
Adds a second, larger ad placement to the two scrollable screens so the larger format can be measured against the existing 320x50 banners. - History: the ad is a real list row injected after the 3rd entry (clamped to the list size), so it scrolls with the history. HistoryAdapter becomes multi view type and maps adapter positions back to history indices for the share/delete context menu. - Settings: the ad sits between the Appearance and App sections inside the NestedScrollView. Both placements are gated behind a new enable_inline_ads Remote Config flag and only render when enable_ads is true as well. The new ad unit ids (history_inline_ad_id, settings_inline_ad_id) default to empty, so the placements stay off until the real units are published. A 300x250 AdView reserves its full height even on an unfilled request, so each placement is only surfaced from onAdLoaded, and both AdViews are destroyed with their activity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
Adds a PR Build workflow that runs the unit tests, lints, and attaches a debug APK to the run as an artifact. It never tags, releases, or touches Play, so a change can be installed and tested before it is merged. Debug builds need no secrets since google-services.json is committed and signing falls back to the debug keystore. Lint is reported but not blocking, since the project has no lint baseline yet and a first failing run would block every pull request. Also adds workflow_dispatch to App Pre-Release so a pre-release APK can be built from any branch without pushing to beta first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
setupObservables() only builds a HistoryAdapter when the list is non-empty; with no history it swaps to the noHistory empty-state view instead, so the loaded 300x250 ad had nowhere to attach and silently never showed. Adds a noHistoryAdContainer inside the empty state and routes the already-loaded ad there via attachInlineAd(), tracking isHistoryEmpty so the ad lands in the right place regardless of whether the ad or the history list finishes loading first. Also re-routes the ad on every list emission, since clearing history (or deleting the last item) via the toolbar/context menu flips the list to empty while this screen is still open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
Adds a 4th 320x50 banner placement (same size as main/history/settings) to AboutFragment, after the last content card inside the scrolling NestedScrollView so it appears once the user scrolls to the bottom. Gated the same way as the existing three banners: enable_ads plus a non-empty about_ad_id from Remote Config (new key, defaults to empty so it stays off until a real unit is published) and UMP consent. The AdView is destroyed in onDestroyView, since unlike an activity this fragment's view can be recreated on back-stack navigation to and from the Changelog/Open Source screens. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
CI failed to compile: AboutFragment.kt's enableBannerAd() calls
logEvent(ADS_DISABLED) { param("reason", ...) }, following the same
pattern History/SettingsActivity already use, but BaseFragment only
had the single-Bundle overload, not BaseActivity's lambda-based one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
Replaces the small 320x50 banner on About with the same 300x250 MREC used on History and Settings, and folds it into that same experiment instead of keeping it a separate always-on placement: it's now gated by enable_inline_ads (in addition to enable_ads) and reuses the shared resolveInlineAd()/createInlineAdView() helpers from util/InlineAd.kt, matching SettingsActivity's implementation. Drops the now-unused about_ad_id Remote Config key in favor of about_inline_ad_id, consistent with history_inline_ad_id / settings_inline_ad_id. The ad container is hidden until onAdLoaded, same as the other two placements, so an unfilled request never reserves a blank 250dp block. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
The About page's 300x250 ad is a separate placement from the enable_inline_ads experiment on History/Settings, not a third arm of it. Gates it on enable_ads alone again, and swaps the Remote Config key back to about_ad_id (dropping about_inline_ad_id). Still uses createInlineAdView() for the MREC creation/reveal-on-load behavior, since that part is just shared AdView boilerplate — only the gating decision (resolveInlineAd(), tied to enable_inline_ads) no longer applies here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
History and Settings were each requesting both an inline 300x250 and the anchored bottom banner, so both could show at once. Replaces the two independent ad paths with a single resolveAdPlacement() that picks one placement in priority order: the inline 300x250, then the bottom banner, then nothing. The choice is made from Remote Config before any request goes out, so a screen that picks the inline ad does not fall back to the banner when the inline request fails to fill — a session stays purely inline or purely banner, which keeps the experiment readable. An inline ad whose unit id is unset is not a usable placement, so it falls through to the banner rather than blocking it. Analytics stay mutually exclusive per screen: INLINE_ADS_ENABLED when the inline wins, ADS_ENABLED when the banner wins, ADS_DISABLED with a reason when neither is available. Drops the now-redundant INLINE_ADS_DISABLED event, and renames util/InlineAd.kt to util/AdPlacement.kt since it now owns the whole decision. With enable_inline_ads off — today's production config — every screen still resolves to the bottom banner, so behaviour is unchanged until the experiment is switched on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq
arch10
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds larger 300x250 ad placements so their revenue can be compared against the existing 320x50 banners, and makes each screen show at most one ad.
Inline 300x250 experiment (History + Settings) — gated behind a new
enable_inline_adsRemote Config flag, on top ofenable_ads. New unit id keyshistory_inline_ad_id/settings_inline_ad_iddefault to empty, so nothing renders until real units are published.HistoryAdapterbecomes multi-view-type and maps adapter positions back to history indices so the share/delete context menu still targets the right entry. When there is no history at all, the ad renders in the empty-state view instead, and relocates live if the list flips between empty and non-empty (e.g. clearing history while the screen is open).NestedScrollView.About page 300x250 — a separate placement, not part of the experiment: gated on
enable_adsplus its ownabout_ad_idkey, with no dependency onenable_inline_ads. Sits after the last content card. ItsAdViewis destroyed inonDestroyView, since unlike an activity this fragment's view is recreated when navigating back from Changelog / Open Source.Ad placement priority system — History and Settings previously requested both an inline ad and the bottom banner, so both could show at once.
resolveAdPlacement()now picks exactly one, in priority order:enable_adsandenable_inline_adsand inline unit id setenable_adsand banner unit id setThe choice is made from Remote Config before any request goes out, so a screen that picks the inline ad does not fall back to the banner when that request fails to fill — a session stays purely inline or purely banner, keeping the experiment readable. An inline ad whose unit id is unset is not a usable placement, so it falls through to the banner rather than blocking it. With
enable_inline_adsoff — today's production config — every screen still resolves to the bottom banner, so behaviour is unchanged until the experiment is switched on.A 300x250
AdViewreserves its full height even on an unfilled request, so every inline placement is only surfaced fromonAdLoaded, never leaving a blank 250dp block.Analytics stay mutually exclusive per screen:
INLINE_ADS_ENABLEDwhen the inline wins,ADS_ENABLEDwhen the banner wins,ADS_DISABLEDwith a reason when neither is available.CI — adds a
PR Buildworkflow (unit tests, lint, debug APK uploaded as a run artifact; no tag, release, or Play upload) andworkflow_dispatchonApp Pre-Releaseso a pre-release APK can be built from any branch on demand. Also adds the lambdalogEventoverload toBaseFragment, whichBaseActivityalready had.Test plan
./gradlew testandassembleDebugpass in CI./gradlew lintruns (reported, non-blocking — no lint baseline exists yet)enable_inline_adson, each screen shows only the inline ad and no bottom banner (and no leftover bottom margin); with it off, only the bottom bannerenable_adsoff, no ads anywhere🤖 Generated with Claude Code
https://claude.ai/code/session_01WW79CouiX84JP9fgbMhPwq