Migrate mobile builds from Cordova to Capacitor#421
Conversation
Introduce an isolated `capacitor/` package with basic Capacitor config so we can migrate off Cordova without impacting the legacy Ember/Node 12 root install.
Update the Android lanes to accept either AAB_FILE_PATH or APK_FILE_PATH, enabling a safe transition to Play Store AAB uploads without breaking existing APK-based CI.
Switch the CircleCI Android production build to generate an AAB and expose an AAB_FILE_PATH for fastlane uploads, while keeping staging builds on debug APK output.
Generate native `ios/` and `android/` projects under the new `capacitor/` wrapper so the Ember app can be packaged with Capacitor (web assets from ../dist) while Cordova-based CI remains unchanged for now.
Keep the existing Cordova www output but also persist the Ember dist folder to the workspace so future Capacitor-based mobile builds can reuse the same web build artifacts.
Introduce a Capacitor-side rename script to set app id/name and align Android/iOS versioning from the root package version and CI build number, matching the existing Cordova release behavior.
Add a Gradle release signing config that reads the keystore path, alias, and passwords from CI environment variables so we can produce Play Store-ready AABs from the Capacitor Android project.
Allow Android fastlane lanes to find the service account key in either fastlane/google-play-key.json (current CI download path) or the repo root.
Build Android artifacts from the new Capacitor native project (syncing Ember dist into the wrapper), produce AABs for production and APKs for staging, and keep fastlane deployment unchanged.
Build and export the iOS IPA from the Capacitor Xcode project (syncing Ember dist into the wrapper) so fastlane can continue uploading to TestFlight/TestFairy without relying on Cordova.
Replace Cordova-focused mobile build documentation with Capacitor workflows (syncing Ember dist into the native projects), and update related Android Studio/WSL guidance.
Support overriding API_HOST_URL and SOCKETIO_WEBSERVICE_URL in development so mobile simulators/devices can run dev-mode builds against staging or other remote backends.
Use padding-top (not padding shorthand) for iOS safe-area inset, and set base html/body styles to avoid unexpected horizontal scrolling and the white bar at the top of the Capacitor WebView.
Set the UIWindow, bridge controller, and WKWebView backgrounds to the app blue so iOS safe-area insets don't render as a white bar above the web content.
Detect Capacitor/Cordova at runtime and add an html class to suppress ember-smart-banner, which is only relevant for Safari and should not display inside the native WebView.
Ember builds produce .gz variants of static assets; exclude *.gz from Android packaged assets to prevent duplicate-resource merge failures when syncing web assets into the Capacitor project.
Add a Capacitor wrapper script that picks the first running emulator/device from adb and runs the Android build against it. Also ignore Capacitor-generated Gradle include files.
Enable fitsSystemWindows on the root layout and WebView so content does not render under the status bar on Android emulators/devices.
Use edge-to-edge with WindowInsets and pad the root container by system bar insets so the Capacitor WebView content does not render under the status/navigation bars.
BridgeActivity uses Capacitor’s internal layout, so apply system bar + cutout insets padding to android.R.id.content once the view hierarchy exists. This prevents WebView underlap across devices.
Apply the app blue to status/navigation bars and force light system icons so the status bar text remains readable after enabling edge-to-edge + inset padding.
Set postSplashScreenTheme and explicit system bar colors/appearance so the status bar does not remain white under the splash theme, and apply bar styling early in MainActivity.
Set window background to the app blue and explicitly color the decor view so the system bar inset padding region doesn’t render as white when using edge-to-edge with manual inset padding.
Create iOS and Android launcher icons from public/assets/images/logo-art.png, composited onto the GoodCity blue background (#0B2B4D).
Drop the ember-cli-cordova dependency and related EMBER_CLI_CORDOVA configuration, and replace it with a small cordova service shim compatible with Capacitor so existing controllers continue to function.
Build Ember dist once in www_build and reuse it for Capacitor Android/iOS builds; drop the legacy ember_cordova_build job and all cordova/www handling.
Drop legacy EMBER_CLI_CORDOVA usage in README and remove Cordova-specific ignore patterns now that Cordova packaging has been removed.
Delete the Cordova wrapper project and Docker build file now that Capacitor is the sole mobile packaging path.
Bump @capacitor/* to 8.3.1, add a capacitor-scoped .nvmrc, and update docs/CI to use Node 22 for running Capacitor CLI and sync steps.
Declare the GET / handler payload with var so it does not leak as a global when the mock server runs in strict mode.
Resolve btn1Callback via RSVP so returned promises rejections are caught, the dialog can still close safely, and close() respects isDestroyed / isDestroying.
- Dedupe register_device POSTs using lastRegisteredPushPosted; replay when session.authToken becomes available after token capture. - Log Capacitor platform check failures in isIOS instead of swallowing. - Prefer capacitor-native-settings on iOS; gate AppLauncher app-settings: on canOpenUrl when NativeSettings is absent.
Avoid re-running edge-to-edge setup on every onWindowFocusChanged by guarding the focused-path apply with a one-shot flag.
Add a small Node script that rewrites Xcode MARKETING_VERSION build settings from the Ember app package version, and run it before cap sync so iOS marketing version tracks releases without manual pbxproj edits.
Rely on Dir glob results for AAB/APK paths and drop redundant File.exist? checks and nil guards before end_with? and TestFairy APK selection.
update_fastlane upgrades the installed gem and removes the version pinned in Gemfile.lock, so bundle exec fails when Fastlane restarts (e.g. on CircleCI).
Capacitor 8 native modules (e.g. :capacitor-android) target Java 21; CircleCI had JAVA_HOME pinned to JDK 17, which caused "invalid source release: 21". Rely on cimg/android's default JDK 21, align the app module and docs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
app/components/message-box.js (2)
42-47: Optional: mirrorbtn1Click's async-safety inbtn2Click.
btn2Clickinvokesbtn2Callback()synchronously and unconditionally callsclose()immediately after. If a caller ever wires a promise-returning callback tobtn2Callback(nothing in the component's contract prevents it), a rejection would become an unhandled promise rejection, andclose()would run before the callback settles. Current callers (closeDialoginapp/controllers/offer/ios_notification.js) are synchronous, so this isn't a live bug — just a consistency/defensive-coding nit now thatbtn1Clickhandles this correctly.♻️ Proposed refactor to mirror btn1Click
btn2Click() { - if (this.btn2Callback) { - this.btn2Callback(); - } - this.close(); + const promise = Ember.RSVP.resolve().then(() => + this.btn2Callback ? this.btn2Callback() : undefined + ); + promise + .then(() => { + if (!this.isDestroyed && !this.isDestroying) { + this.close(); + } + }) + .catch(err => { + if (!this.isDestroyed && !this.isDestroying) { + this.close(); + } + throw err; + }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/message-box.js` around lines 42 - 47, btn2Click currently calls btn2Callback() synchronously and immediately calls this.close(), which can cause unhandled promise rejections and closes before async callbacks finish; change btn2Click to mirror btn1Click by making it async, invoke the callback via await (e.g. await Promise.resolve(this.btn2Callback && this.btn2Callback())), wrap the await in try/catch to swallow or log errors appropriately, and call this.close() in a finally block so the dialog closes only after the callback settles; refer to btn2Click, btn2Callback, this.close, and btn1Click when locating the change.
24-40: LGTM — async-rejection concern properly resolved, and behavior change is acceptable.The rewrite correctly converts sync throws into promise rejections via
Ember.RSVP.resolve().then(...), guardsclose()withisDestroyed/isDestroyingon both success and error paths, and rethrows errors so they surface throughEmber.onerror.The code introduces an intentional behavior change: where the prior implementation caught synchronous throws and kept the modal open, the new implementation closes the modal on any failure (sync throw or async rejection). This change is acceptable—the only identified caller with a
btn1CallbackisopenSettings()in the iOS notification template, which delegates to a native Cordova API to open iOS app settings. For this one-time system action, closing the modal on error (whether the Cordova call rejects or throws) is the correct UX—users should not remain stuck on the prompt if the system action fails.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/message-box.js` around lines 24 - 40, Summary: The btn1Click rewrite changes error-handling semantics to close the modal on any failure; this is intentional and acceptable. No code change required: leave btn1Click as implemented (the Ember.RSVP.resolve().then(...) wrapper, the guards using this.isDestroyed/this.isDestroying in both .then and .catch, and the rethrow of errors) because the only caller btn1Callback (openSettings) should close the modal on failure; keep the current behavior and tests as-is.capacitor/android/app/build.gradle (1)
62-73: LGTM — robust conditional application ofgoogle-servicesplugin.The three-way check (missing file / empty file / applied with error trap) cleanly handles the common CI scenarios where
google-services.jsonis injected only for release builds, and avoids opaque Gradle failures when the file is intentionally absent. The explicitGradleExceptionwrapping on Line 71 will surface real misconfigurations (malformed JSON, etc.) rather than letting them silently degrade push notifications.Minor: consider
logger.warn(orlogger.lifecycle) instead oflogger.infoon Lines 64/66 — by default Gradle suppressesinfounless run with--info, so a developer who forgot to placegoogle-services.jsonwon't see the message and will wonder why push notifications are broken at runtime.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/android/app/build.gradle` around lines 62 - 73, Change the two logger.info calls that report missing/empty google-services.json to a higher-visibility Gradle log level so the message is shown by default; update the logging for the servicesJSON checks (the two branches that currently call logger.info) to use logger.warn or logger.lifecycle so developers will see the message without --info, leaving the apply plugin: 'com.google.gms.google-services' try/catch and GradleException behavior unchanged.capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java (1)
17-45: Minor: redundant chrome re-application on first focus.
applySystemBarChrome(false)inonCreate(Line 27) already performssetDecorFitsSystemWindows, decor background, API<35 bar color, and controller appearance. On the first focus event,applySystemBarChrome(true)repeats all of this just to attach the insets listener. Functionally harmless, but you can split the "one-time chrome" setup from the "attach insets listener" step to make the intent clearer and avoid duplicate window calls.♻️ Optional refactor
`@Override` protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - - // Edge-to-edge + decor background so `@color/goodcity_blue` shows behind transparent - // system bars on Android 15+ (API 35), where setStatusBarColor/setNavigationBarColor - // are no longer applied for the default edge-to-edge window. - applySystemBarChrome(false); + applySystemBarChrome(); } `@Override` public void onWindowFocusChanged(boolean hasFocus) { super.onWindowFocusChanged(hasFocus); - - // BridgeActivity uses Capacitor's internal layout (not our activity_main.xml), - // so apply insets to the real content root once the view hierarchy exists. - if (!hasFocus) { - return; - } - if (systemBarChromeApplied) { - return; - } - - applySystemBarChrome(true); - systemBarChromeApplied = true; + if (!hasFocus || systemBarChromeApplied) { + return; + } + attachInsetsListener(); + systemBarChromeApplied = true; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java` around lines 17 - 45, MainActivity currently calls applySystemBarChrome(false) in onCreate and then calls applySystemBarChrome(true) again on first focus which repeats one-time window setup; refactor by extracting the one-time chrome setup (setDecorFitsSystemWindows, decor background, API<35 bar color, controller appearance) into a dedicated method (e.g., initializeSystemBarChromeOnce or keep applySystemBarChromeOneTime) and call that from onCreate, and change the onWindowFocusChanged path to only attach the insets listener (e.g., attachInsetsListener) without reapplying window/window-decoration changes; update systemBarChromeApplied usage to reflect only the listener attachment to avoid duplicate calls to applySystemBarChrome..circleci/config.yml (1)
275-289: DuplicatedDEVELOPMENT_TEAMliteral.The team ID
6B8FS8W94Mis hardcoded both as anxcodebuildargument (line 275) and inside the ExportOptions plist (line 289). Rotating or re-using this config from a fork requires editing both sites and will silently desync if one is missed. Consider pulling it to a single shell variable (or an env var in the jobenvironment:block) and referencing it in both places.♻️ Suggested change
+ TEAM_ID="6B8FS8W94M" xcodebuild \ -workspace ios/App/App.xcworkspace \ -scheme App \ -configuration Release \ -archivePath ios/build/App.xcarchive \ -destination "generic/platform=iOS" \ archive \ - DEVELOPMENT_TEAM="6B8FS8W94M" \ + DEVELOPMENT_TEAM="${TEAM_ID}" \ CODE_SIGN_IDENTITY="${CODE_SIGN_IDENTITY}" \ PROVISIONING_PROFILE="${UUID}" \ IPHONEOS_DEPLOYMENT_TARGET=15 ... <key>teamID</key> - <string>6B8FS8W94M</string> + <string>${TEAM_ID}</string>(Paired with the unquoted-heredoc fix above so
${TEAM_ID}expands.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.circleci/config.yml around lines 275 - 289, The hardcoded team ID appears twice (DEVELOPMENT_TEAM="6B8FS8W94M" and the <key>teamID</key> value in the ExportOptions.plist); replace the literal with a single shell variable (e.g., TEAM_ID) or env var and reference that variable in both places (DEVELOPMENT_TEAM="${TEAM_ID}" and substitute ${TEAM_ID} into the ExportOptions.plist generation) and ensure the heredoc is unquoted so ${TEAM_ID} expands at runtime; update any uses around EXPORT_METHOD/PROVISIONING_PROFILE to reference the same TEAM_ID variable.capacitor/scripts/sync-ios-marketing-version.js (1)
29-41: Silent no-op when noMARKETING_VERSIONentries match.
replacereturns the original string both when the file is already in sync and when the pattern doesn't match (e.g. Xcode project layout changes, a rename, or an accidentally empty pbxproj). In the latter case this script exits 0 with no output, which will only surface later as a mismatched store submission.Consider distinguishing the two cases by testing the regex first, and logging / failing when nothing matched:
♻️ Suggested change
let pbx = fs.readFileSync(pbxPath, "utf8"); +if (!/MARKETING_VERSION = [^;]+;/.test(pbx)) { + console.error( + "sync-ios-marketing-version: no MARKETING_VERSION entries found in", + pbxPath + ); + process.exit(1); +} const next = pbx.replace( /MARKETING_VERSION = [^;]+;/g, `MARKETING_VERSION = ${version};` ); if (next !== pbx) { fs.writeFileSync(pbxPath, next); console.log( "sync-ios-marketing-version: set MARKETING_VERSION to", version, "in project.pbxproj" ); +} else { + console.log( + "sync-ios-marketing-version: MARKETING_VERSION already", + version + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/scripts/sync-ios-marketing-version.js` around lines 29 - 41, The script silently no-ops when the regex doesn't match; first run the regex against pbx (e.g., const matches = pbx.match(/MARKETING_VERSION = [^;]+;/g)) and if matches is null or length === 0 log an explicit error including pbxPath and exit non-zero so CI fails, otherwise perform the existing replace into next; then keep the current change-detection (if next !== pbx) to write the file and log the update, and if next === pbx log a clear "already in sync" message referencing version so you distinguish "no matches" from "already up-to-date".fastlane/Fastfile (1)
152-152: Guard is fragile —""is truthy in Ruby.
(ENV["SUPPLY_AAB"] || ENV["SUPPLY_APK"] || "").empty?relies on the invariant (held inbefore_all) that neither env var is ever set to an empty string. If some future caller setsSUPPLY_AAB=""whileSUPPLY_APKholds a real path, the short-circuit returns""and this raises despite a valid APK being present.♻️ Suggested fix
- raise "Please specify ENV VAR [AAB_FILE_PATH] or [APK_FILE_PATH]" if (ENV["SUPPLY_AAB"] || ENV["SUPPLY_APK"] || "").empty? + if ENV["SUPPLY_AAB"].to_s.empty? && ENV["SUPPLY_APK"].to_s.empty? + raise "Please specify ENV VAR [AAB_FILE_PATH] or [APK_FILE_PATH]" + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@fastlane/Fastfile` at line 152, The guard at the raise line incorrectly treats a set-but-empty ENV value as truthy; replace the current condition so it checks both SUPPLY_AAB and SUPPLY_APK for non-empty strings (e.g. treat nil and "" the same) before raising. Specifically, update the raise check that references ENV["SUPPLY_AAB"] and ENV["SUPPLY_APK"] to use a string-safe emptiness test (like calling to_s.empty? on each) and raise only if both are empty, ensuring the logic around SUPPLY_AAB and SUPPLY_APK correctly allows one valid path to satisfy the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Around line 279-292: The heredoc used to write ExportOptions.plist uses a
single-quoted delimiter (<<'EOF') so ${EXPORT_METHOD} is written literally;
change the heredoc to an unquoted delimiter (<<EOF) so ${EXPORT_METHOD} expands,
or keep the quoted heredoc and perform an explicit substitution (e.g., replace a
placeholder like __METHOD__ with ${EXPORT_METHOD} via sed) before calling
xcodebuild -exportArchive; also remove the hardcoded team ID (6B8FS8W94M) from
both the xcodebuild arguments and the ExportOptions.plist generation and instead
reference a single shared variable (e.g., EXPORT_TEAM_ID) so the same value is
used in the xcodebuild invocation and the plist creation.
In `@capacitor/android/app/build.gradle`:
- Around line 19-25: The hardcoded default values versionCode and versionName in
defaultConfig (symbols: versionCode, versionName, applicationId
"hk.goodcity.app") can conflict with CI's rename:package script which sets
versionCode = 270000000 + CIRCLE_BUILD_NUM and versionName from package.json
(CircleCI jobs: android_build_and_deploy and ios_build_and_deploy); update the
repository docs or build config to explicitly state the migration strategy and
verify the existing Cordova Play Store app's versionCode is below the 270000000
seed or implement logic to read the live Play Store versionCode and adjust the
CI seed accordingly; ensure the rename:package step and any Gradle placeholders
are coordinated with package.json and add a short check in CI to fail the build
with a clear error if a versionCode collision risk is detected.
---
Nitpick comments:
In @.circleci/config.yml:
- Around line 275-289: The hardcoded team ID appears twice
(DEVELOPMENT_TEAM="6B8FS8W94M" and the <key>teamID</key> value in the
ExportOptions.plist); replace the literal with a single shell variable (e.g.,
TEAM_ID) or env var and reference that variable in both places
(DEVELOPMENT_TEAM="${TEAM_ID}" and substitute ${TEAM_ID} into the
ExportOptions.plist generation) and ensure the heredoc is unquoted so ${TEAM_ID}
expands at runtime; update any uses around EXPORT_METHOD/PROVISIONING_PROFILE to
reference the same TEAM_ID variable.
In `@app/components/message-box.js`:
- Around line 42-47: btn2Click currently calls btn2Callback() synchronously and
immediately calls this.close(), which can cause unhandled promise rejections and
closes before async callbacks finish; change btn2Click to mirror btn1Click by
making it async, invoke the callback via await (e.g. await
Promise.resolve(this.btn2Callback && this.btn2Callback())), wrap the await in
try/catch to swallow or log errors appropriately, and call this.close() in a
finally block so the dialog closes only after the callback settles; refer to
btn2Click, btn2Callback, this.close, and btn1Click when locating the change.
- Around line 24-40: Summary: The btn1Click rewrite changes error-handling
semantics to close the modal on any failure; this is intentional and acceptable.
No code change required: leave btn1Click as implemented (the
Ember.RSVP.resolve().then(...) wrapper, the guards using
this.isDestroyed/this.isDestroying in both .then and .catch, and the rethrow of
errors) because the only caller btn1Callback (openSettings) should close the
modal on failure; keep the current behavior and tests as-is.
In `@capacitor/android/app/build.gradle`:
- Around line 62-73: Change the two logger.info calls that report missing/empty
google-services.json to a higher-visibility Gradle log level so the message is
shown by default; update the logging for the servicesJSON checks (the two
branches that currently call logger.info) to use logger.warn or logger.lifecycle
so developers will see the message without --info, leaving the apply plugin:
'com.google.gms.google-services' try/catch and GradleException behavior
unchanged.
In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java`:
- Around line 17-45: MainActivity currently calls applySystemBarChrome(false) in
onCreate and then calls applySystemBarChrome(true) again on first focus which
repeats one-time window setup; refactor by extracting the one-time chrome setup
(setDecorFitsSystemWindows, decor background, API<35 bar color, controller
appearance) into a dedicated method (e.g., initializeSystemBarChromeOnce or keep
applySystemBarChromeOneTime) and call that from onCreate, and change the
onWindowFocusChanged path to only attach the insets listener (e.g.,
attachInsetsListener) without reapplying window/window-decoration changes;
update systemBarChromeApplied usage to reflect only the listener attachment to
avoid duplicate calls to applySystemBarChrome.
In `@capacitor/scripts/sync-ios-marketing-version.js`:
- Around line 29-41: The script silently no-ops when the regex doesn't match;
first run the regex against pbx (e.g., const matches =
pbx.match(/MARKETING_VERSION = [^;]+;/g)) and if matches is null or length === 0
log an explicit error including pbxPath and exit non-zero so CI fails, otherwise
perform the existing replace into next; then keep the current change-detection
(if next !== pbx) to write the file and log the update, and if next === pbx log
a clear "already in sync" message referencing version so you distinguish "no
matches" from "already up-to-date".
In `@fastlane/Fastfile`:
- Line 152: The guard at the raise line incorrectly treats a set-but-empty ENV
value as truthy; replace the current condition so it checks both SUPPLY_AAB and
SUPPLY_APK for non-empty strings (e.g. treat nil and "" the same) before
raising. Specifically, update the raise check that references ENV["SUPPLY_AAB"]
and ENV["SUPPLY_APK"] to use a string-safe emptiness test (like calling
to_s.empty? on each) and raise only if both are empty, ensuring the logic around
SUPPLY_AAB and SUPPLY_APK correctly allows one valid path to satisfy the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50d880c2-5c1b-4558-a902-e4434c64aa3b
📒 Files selected for processing (11)
.circleci/config.ymlREADME.mdapp/components/message-box.jsapp/services/cordova.jscapacitor/android/app/build.gradlecapacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.javacapacitor/android/gradle.propertiescapacitor/package.jsoncapacitor/scripts/sync-ios-marketing-version.jsfastlane/Fastfileserver/mocks/territories.js
✅ Files skipped from review due to trivial changes (2)
- capacitor/android/gradle.properties
- capacitor/package.json
🚧 Files skipped from review as they are similar to previous changes (3)
- README.md
- server/mocks/territories.js
- app/services/cordova.js
Stop passing PROVISIONING_PROFILE to xcodebuild archive so it does not apply to Pod targets or conflict with automatic signing on App. Copy the downloaded mobileprovision to UUID.mobileprovision so Xcode can resolve it for automatic signing.
rename_package no longer rewrites namespace to match staging applicationId. R and BuildConfig are generated under namespace; Java sources stay in hk.goodcity.app, so R must remain hk.goodcity.app.R.
Export App Store Connect API key env vars for xcodebuild and pass -allowProvisioningUpdates on archive and exportArchive so automatic signing can resolve hk.goodcity.appstaging without a global profile.
Env-only APP_STORE_CONNECT vars still produced "No Accounts" on archive. Use -authenticationKeyPath/ID/IssuerID (same .p8 as fastlane) for archive and exportArchive.
…guard Write ExportOptions.plist with printf so EXPORT_METHOD and team expand. Use TEAM_ID (IOS_DEVELOPMENT_TEAM override) for archive and export. After Android Gradle, fail if CIRCLE_BUILD_NUM set but versionCode below rename_package ANDROID_BUILD_VERSION_SEED.
Ruby treats "" as truthy in || chains; require non-empty SUPPLY_AAB or SUPPLY_APK after strip so a blank AAB path does not hide a valid APK path.
Exit non-zero if MARKETING_VERSION assignments are missing; log when already aligned with package.json version.
…sions Use logger.warn for visibility without --info. Note that rename:package overwrites versionCode, versionName, and applicationId on CI.
Split window chrome from content inset listener so decor is not reapplied when the view hierarchy becomes ready.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
capacitor/scripts/sync-ios-marketing-version.js (2)
40-40: Escape$in the replacement string to avoid special-pattern interpretation.
String.prototype.replacetreats$&,$1,$', etc. in the replacement as special tokens. For conventional semver values frompackage.jsonthis is effectively a non-issue, but ifversionever contains a$(e.g., a custom prerelease tag), the writtenMARKETING_VERSIONwill be corrupted silently. A small defensive tweak avoids that footgun:♻️ Proposed defensive fix
-const next = pbx.replace(marketingRe, `MARKETING_VERSION = ${version};`); +const replacement = `MARKETING_VERSION = ${version};`.replace(/\$/g, "$$$$"); +const next = pbx.replace(marketingRe, replacement);Or use the function form, which does not interpret
$tokens:-const next = pbx.replace(marketingRe, `MARKETING_VERSION = ${version};`); +const next = pbx.replace(marketingRe, () => `MARKETING_VERSION = ${version};`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/scripts/sync-ios-marketing-version.js` at line 40, The replacement currently uses pbx.replace(marketingRe, `MARKETING_VERSION = ${version};`) which treats any $ in version as replacement tokens; change it to use the function form or escape $ characters so they are not interpreted. For example, replace the call to pbx.replace(marketingRe, `MARKETING_VERSION = ${version};`) with pbx.replace(marketingRe, () => `MARKETING_VERSION = ${version};`) or pre-escape version (e.g., const safeVersion = version.replace(/\$/g, '$$$$') and use that) so MARKETING_VERSION is written exactly as the version string.
29-38: Consider validating that allMARKETING_VERSIONentries agree before overwriting.The regex replaces every
MARKETING_VERSION = …;occurrence regardless of its current value or build configuration (Debug/Release). If the project ever intentionally uses distinct values per configuration or target (e.g., an extension target), this script would silently flatten them. Given the current single-targetApp.xcodeproj, this is fine today, but a quick sanity check or log of the distinct values found would make drift easier to notice.♻️ Optional: log distinct existing values
const matches = pbx.match(marketingRe); if (!matches || matches.length === 0) { console.error( "sync-ios-marketing-version: no MARKETING_VERSION = …; assignments found in", pbxPath ); process.exit(1); } +const distinct = [...new Set(matches)]; +if (distinct.length > 1) { + console.warn( + "sync-ios-marketing-version: found multiple distinct MARKETING_VERSION values, flattening:", + distinct + ); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/scripts/sync-ios-marketing-version.js` around lines 29 - 38, Before replacing MARKETING_VERSION entries, extract the current values from pbx using the existing marketingRe and check for consistency: use pbx.match(marketingRe) to get matches, map each match to the version token (the portion after "MARKETING_VERSION = " and before ";"), build a Set of distinct values and if the Set size > 1 log a clear warning including pbxPath and the distinct values (or exit non-zero to avoid silently flattening differing per-configuration values); if the Set size == 1 proceed with the replacement as before and optionally log the single existing value for audit. Ensure this logic touches the existing symbols pbx, marketingRe, matches and pbxPath and runs before performing any overwrite.capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java (1)
30-42: Consider attaching the insets listener inonCreateinstead of firstonWindowFocusChanged.
BridgeActivity.onCreatecallssetContentViewviasuper.onCreate, sofindViewById(android.R.id.content)is already valid by the end of youronCreate. Gating this on the first focus event works, but it delays the first inset application until the user actually sees the window and adds a statefulsystemBarInsetsListenerAttachedflag. Attaching inonCreate(aftersuper.onCreate) and relying onViewCompat.requestApplyInsets(content)would be simpler and avoid a brief pre-focus frame without correct padding.Not blocking — current implementation is functionally correct.
♻️ Optional simplification
public class MainActivity extends BridgeActivity { - private boolean systemBarInsetsListenerAttached; - `@Override` protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); // Edge-to-edge + decor background so `@color/goodcity_blue` shows behind transparent // system bars on Android 15+ (API 35), where setStatusBarColor/setNavigationBarColor // are no longer applied for the default edge-to-edge window. applyWindowSystemBarDecorOnce(); + attachSystemBarInsetsListener(); } - `@Override` - public void onWindowFocusChanged(boolean hasFocus) { - super.onWindowFocusChanged(hasFocus); - - // BridgeActivity uses Capacitor's internal layout (not our activity_main.xml), - // so attach insets to the real content root once the view hierarchy exists. - if (!hasFocus || systemBarInsetsListenerAttached) { - return; - } - - attachSystemBarInsetsListener(); - systemBarInsetsListenerAttached = true; - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java` around lines 30 - 42, Move the insets attachment out of onWindowFocusChanged into onCreate: after calling super.onCreate(...) call attachSystemBarInsetsListener() (instead of waiting for focus) and remove or stop using the systemBarInsetsListenerAttached gating flag and the early-return logic in onWindowFocusChanged; after attaching, call ViewCompat.requestApplyInsets(content) to force the initial inset application. This keeps the existing attachSystemBarInsetsListener() implementation and ensures findViewById(android.R.id.content) is valid because BridgeActivity.setContentView runs in super.onCreate.capacitor/android/app/build.gradle (1)
28-33:aaptOptionsDSL is deprecated in AGP 8 — migrate toandroidResourcesblock.AGP 8.13.0 deprecated
aaptOptionsin favor of theandroidResourcesblock. The recommended replacement should sit directly under theandroid { … }block (as a sibling todefaultConfigandbuildTypes), not nested withindefaultConfig.♻️ Proposed refactor
aaptOptions { // Files and dirs to omit from the packaged assets dir, modified to accommodate modern web apps. // Default: https://android.googlesource.com/platform/frameworks/base/+/282e181b58cf72b6ca770dc7ca5f91f135444502/tools/aapt/AaptAssets.cpp#61 // Ignore Ember-generated gzip artifacts to avoid duplicate assets (foo.js + foo.js.gz). ignoreAssetsPattern '!.svn:!.git:!.ds_store:!*.scc:.*:!CVS:!thumbs.db:!picasa.ini:!*~:*.gz' - } + } + androidResources { + // Files and dirs to omit from the packaged assets dir, modified to accommodate modern web apps. + // Default: https://android.googlesource.com/platform/frameworks/base/+/282e181b58cf72b6ca770dc7ca5f91f135444502/tools/aapt/AaptAssets.cpp#61 + // Ignore Ember-generated gzip artifacts to avoid duplicate assets (foo.js + foo.js.gz). + ignoreAssetsPattern '!.svn:!.git:!.ds_store:!*.scc:.*:!CVS:!thumbs.db:!picasa.ini:!*~:*.gz' }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@capacitor/android/app/build.gradle` around lines 28 - 33, The build.gradle uses the deprecated aaptOptions block (aaptOptions { ignoreAssetsPattern ... }) which must be migrated to the androidResources DSL in AGP 8; move the ignoreAssetsPattern setting out of aaptOptions and into an androidResources block at the same level as defaultConfig/buildTypes (e.g., androidResources { excludes += 'pattern' } or the AGP-appropriate property for excluding assets) so replace the aaptOptions/ignoreAssetsPattern usage with the equivalent androidResources configuration while keeping the same ignore pattern semantics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Line 271: The EXPORT_METHOD assignment uses deprecated Xcode 26 values; update
the conditional that sets EXPORT_METHOD (the shell variable computed from
ENVIRONMENT) to use "app-store-connect" for production and "debugging" (or
"release-testing" if staging requires ad-hoc distribution) instead of
"app-store" and "development", and ensure any downstream usage expecting the old
strings is adjusted accordingly.
In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java`:
- Around line 80-85: The Android WebView can return 0 for CSS
env(safe-area-inset-top), so keep the top inset handling in native code: update
the window inset handler around v.setPadding to either include bars.top (use
v.setPadding(bars.left, bars.top, bars.right, bars.bottom)) or, if you want to
avoid double-spacing, call the WebView's evaluateJavascript from the same inset
handler to set a CSS custom property (e.g.,
document.documentElement.style.setProperty('--safe-area-inset-top',
'<bars.top>px')) so the app CSS reads the native top inset; reference
v.setPadding, bars.top, insets and the WebView.evaluateJavascript call in
MainActivity.java to locate where to implement the native padding or JS
injection and test on Android 15.
- Around line 67-88: attachSystemBarInsetsListener currently overwrites the
content view padding and doesn't handle IME or gesture insets; update the
WindowInsets handling in the ViewCompat.setOnApplyWindowInsetsListener callback
(inside attachSystemBarInsetsListener) to: compute separate Insets for
systemBars, systemGestures (WindowInsetsCompat.Type.systemBars() vs
Type.systemGestures()), and ime (Type.ime()); when ime is visible zero or reduce
the bottom padding so the keyboard doesn't overlap; preserve existing padding by
using max(current padding, computed inset) for each side (use
v.getPaddingLeft()/Top()/Right()/Bottom()); apply insets to an inner container
if possible rather than android.R.id.content, and return
WindowInsetsCompat.CONSUMED from the listener to avoid duplicate handling by
children.
---
Nitpick comments:
In `@capacitor/android/app/build.gradle`:
- Around line 28-33: The build.gradle uses the deprecated aaptOptions block
(aaptOptions { ignoreAssetsPattern ... }) which must be migrated to the
androidResources DSL in AGP 8; move the ignoreAssetsPattern setting out of
aaptOptions and into an androidResources block at the same level as
defaultConfig/buildTypes (e.g., androidResources { excludes += 'pattern' } or
the AGP-appropriate property for excluding assets) so replace the
aaptOptions/ignoreAssetsPattern usage with the equivalent androidResources
configuration while keeping the same ignore pattern semantics.
In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java`:
- Around line 30-42: Move the insets attachment out of onWindowFocusChanged into
onCreate: after calling super.onCreate(...) call attachSystemBarInsetsListener()
(instead of waiting for focus) and remove or stop using the
systemBarInsetsListenerAttached gating flag and the early-return logic in
onWindowFocusChanged; after attaching, call
ViewCompat.requestApplyInsets(content) to force the initial inset application.
This keeps the existing attachSystemBarInsetsListener() implementation and
ensures findViewById(android.R.id.content) is valid because
BridgeActivity.setContentView runs in super.onCreate.
In `@capacitor/scripts/sync-ios-marketing-version.js`:
- Line 40: The replacement currently uses pbx.replace(marketingRe,
`MARKETING_VERSION = ${version};`) which treats any $ in version as replacement
tokens; change it to use the function form or escape $ characters so they are
not interpreted. For example, replace the call to pbx.replace(marketingRe,
`MARKETING_VERSION = ${version};`) with pbx.replace(marketingRe, () =>
`MARKETING_VERSION = ${version};`) or pre-escape version (e.g., const
safeVersion = version.replace(/\$/g, '$$$$') and use that) so MARKETING_VERSION
is written exactly as the version string.
- Around line 29-38: Before replacing MARKETING_VERSION entries, extract the
current values from pbx using the existing marketingRe and check for
consistency: use pbx.match(marketingRe) to get matches, map each match to the
version token (the portion after "MARKETING_VERSION = " and before ";"), build a
Set of distinct values and if the Set size > 1 log a clear warning including
pbxPath and the distinct values (or exit non-zero to avoid silently flattening
differing per-configuration values); if the Set size == 1 proceed with the
replacement as before and optionally log the single existing value for audit.
Ensure this logic touches the existing symbols pbx, marketingRe, matches and
pbxPath and runs before performing any overwrite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a80ba095-8b8f-450f-b287-fa010436d3fe
📒 Files selected for processing (7)
.circleci/config.ymlapp/components/message-box.jscapacitor/android/app/build.gradlecapacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.javacapacitor/rename_package.jscapacitor/scripts/sync-ios-marketing-version.jsfastlane/Fastfile
🚧 Files skipped from review as they are similar to previous changes (3)
- fastlane/Fastfile
- capacitor/rename_package.js
- app/components/message-box.js
| # Xcode discovers profiles by UUID filename; do not pass PROVISIONING_PROFILE to xcodebuild — it | ||
| # applies to every target (Pods/Capacitor) and conflicts with CODE_SIGN_STYLE=Automatic on App. | ||
| cp "/Users/distiller/Library/MobileDevice/Provisioning Profiles/app-${ENVIRONMENT}.mobileprovision" "/Users/distiller/Library/MobileDevice/Provisioning Profiles/${UUID}.mobileprovision" | ||
| EXPORT_METHOD=$(if [ "$ENVIRONMENT" == "production" ]; then echo app-store; else echo development; fi) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Xcode 26 exportOptionsPlist method values: is "app-store" still accepted or must it be "app-store-connect"? Same question for "development" vs "debugging".
💡 Result:
In Xcode 26, "app-store" and "development" are deprecated for the method key in exportOptionsPlist. Use "app-store-connect" instead of "app-store", and "debugging" instead of "development". Older values like "app-store" and "development" may still be accepted for backward compatibility but should be updated to avoid future issues.
Citations:
- 1: https://gist.github.com/jessedc/12a74aff88d06e669cf1c9999408c62c
- 2: https://gist.github.com/shakemno/8a48eb935c2e9cfc995ef0d7942e50e8
Update EXPORT_METHOD values for Xcode 26 compatibility.
In Xcode 26, the legacy exportOptionsPlist method values (app-store, development) are deprecated. Use app-store-connect and debugging instead. While backward compatibility may exist, updating now prevents future pipeline failures:
Suggested change
- EXPORT_METHOD=$(if [ "$ENVIRONMENT" == "production" ]; then echo app-store; else echo development; fi)
+ EXPORT_METHOD=$(if [ "$ENVIRONMENT" == "production" ]; then echo app-store-connect; else echo debugging; fi)Also verify that debugging-signed IPAs are appropriate for staging distribution. Development-signed builds only install on UDIDs registered in the provisioning profile. If staging needs broader tester access without re-signing, release-testing (ad-hoc) with an iPhone Distribution identity may be more suitable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.circleci/config.yml at line 271, The EXPORT_METHOD assignment uses
deprecated Xcode 26 values; update the conditional that sets EXPORT_METHOD (the
shell variable computed from ENVIRONMENT) to use "app-store-connect" for
production and "debugging" (or "release-testing" if staging requires ad-hoc
distribution) instead of "app-store" and "development", and ensure any
downstream usage expecting the old strings is adjusted accordingly.
| private void attachSystemBarInsetsListener() { | ||
| View content = findViewById(android.R.id.content); | ||
| if (content == null) { | ||
| return; | ||
| } | ||
|
|
||
| ViewCompat.setOnApplyWindowInsetsListener( | ||
| content, | ||
| (v, insets) -> { | ||
| Insets bars = | ||
| insets.getInsets( | ||
| WindowInsetsCompat.Type.systemBars() | ||
| | WindowInsetsCompat.Type.displayCutout()); | ||
| // Do not apply bars.top here: the WebView already gets top inset via | ||
| // CSS env(safe-area-inset-top) on body (see app/styles/app.scss). Applying | ||
| // native top padding as well stacks and looks like double top spacing on Android. | ||
| v.setPadding(bars.left, 0, bars.right, bars.bottom); | ||
| return insets; | ||
| }); | ||
|
|
||
| ViewCompat.requestApplyInsets(content); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Capacitor 6 Android WebView padding on android.R.id.content and IME inset handling best practices
💡 Result:
In Capacitor 6 for Android, best practices for WebView padding on android.R.id.content and IME (keyboard) inset handling involve configuring capacitor.config.ts and optionally customizing MainActivity for edge-to-edge support, especially relevant for Android 15+ (API 35+). Capacitor 6 introduced adjustMarginsForEdgeToEdge to handle Android's edge-to-edge enforcement. ## Key Configuration in capacitor.config.ts { android: { adjustMarginsForEdgeToEdge: 'auto' // or 'force' | 'disable'. 'auto' checks Android 15+ and windowOptOutEdgeToEdgeEnforcement; defaults to 'disable' in v6, changes to 'auto' in v8. }, plugins: { Keyboard: { resizeOnFullScreen: false // Recommended to prevent WebView resize issues in fullscreen/overlay modes. } } } - adjustMarginsForEdgeToEdge: 'auto' | 'force' applies margins to the WebView (via CapacitorWebView) matching system bar insets (status/nav bars), preventing content overlap. This uses ViewCompat.setOnApplyWindowInsetsListener on the WebView, setting MarginLayoutParams for top/bottom/left/right insets from WindowInsetsCompat.Type.systemBars and consuming insets. For IME, modern WebViews (M139+) auto-resize visual viewport; set bottom margin to 0 when keyboard visible to avoid double-padding. ## Custom MainActivity Handling (Recommended for Full Control) Extend BridgeActivity and apply insets to android.R.id.content (activity root): @Override protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); ViewCompat.setOnApplyWindowInsetsListener(findViewById(android.R.id.content), (v, windowInsets) -> { Insets systemInsets = windowInsets.getInsets(WindowInsetsCompat.Type.systemBars); Insets gestureInsets = windowInsets.getInsets(WindowInsetsCompat.Type.systemGestures); Insets imeInsets = windowInsets.getInsets(WindowInsetsCompat.Type.ime); int bottomInset = gestureInsets.bottom; if (imeInsets.bottom > systemInsets.bottom) { bottomInset = 0; // Ignore nav/gesture when IME visible; WebView handles resize. } v.setPadding(0, systemInsets.top, 0, bottomInset); return WindowInsetsCompat.CONSUMED; }); } - Use systemBars for status/nav, systemGestures for gesture nav bottom, ime for keyboard. - Zero bottom padding when keyboard visible (imeInsets.bottom > 0) as WebView visual viewport resizes automatically (since Chromium M139). - Inject CSS vars: document.documentElement.style.setProperty('--ion-safe-area-top', top + 'px') etc., for web CSS like padding-top: var(--ion-safe-area-top). - Avoid CONSUMED without zeroing specific insets to prevent WebView "ghost padding". ## IME Handling Best Practices 1. Config: Keyboard.resizeOnFullScreen: false + captureInput: true (if needed for input reactivity). 2. WebView Auto-Handles: Since Chromium M139 (WebView 139+), IME resizes visual viewport based on WebView-window intersection (bottom only). No manual resize needed unless opting out. 3. Avoid Double-Padding: After applying padding to parent/container, pass modified insets with handled types zeroed via WindowInsetsCompat.Builder.setInsets(Type.ime, Insets.NONE) to WebView. 4. Plugins: @capawesome/capacitor-android-edge-to-edge-support for margins + EdgeToEdge.getInsets; disable Capacitor SystemBars insets if conflicting. 5. Legacy WebView (<140): Apply native bottom margins; force --safe-area-inset-bottom: 0 on keyboard show. 6. AndroidManifest: android:windowSoftInputMode="adjustPan" or "adjustResize" for legacy control, but edge-to-edge overrides this. ## Notes for Capacitor 6 - Defaults safer than later versions; test on Android 15+ emulators/devices. - Update System WebView for env(safe-area-inset-*) support (M136+). - For Ionic: IonicModule.forRoot({ inputShims: true }) enables Android scroll assist. This synthesizes official Capacitor source, docs, and Android WebView guidelines. Run npx cap sync android after config changes.
Citations:
- 1: https://github.com/ionic-team/capacitor/blob/53a47dd9/android/capacitor/src/main/java/com/getcapacitor/CapacitorWebView.java
- 2: https://forum.ionicframework.com/t/edge-to-edge-android/245973
- 3: https://developer.android.com/develop/ui/views/layout/webapps/understand-window-insets
- 4: fix(android): implement automatic edge-to-edge and universal keyboard handling ionic-team/capacitor#8366
- 5: https://registry.npmjs.org/%40capawesome%2Fcapacitor-android-edge-to-edge-support
- 6: fix(android): add EdgeToEdge compatibility ionic-team/capacitor#7871
- 7: https://www.capacitorjs.com/docs/apis/keyboard
🏁 Script executed:
# Find and examine MainActivity.java to understand its class hierarchy
find . -name "MainActivity.java" -type f | head -5Repository: crossroads/app.goodcity
Length of output: 138
🏁 Script executed:
# Check if BridgeActivity is involved and look for other inset listeners
rg -B5 -A15 "attachSystemBarInsetsListener" --type javaRepository: crossroads/app.goodcity
Length of output: 4690
🏁 Script executed:
# Search for other ViewCompat.setOnApplyWindowInsetsListener calls to understand if there are conflicts
rg "setOnApplyWindowInsetsListener" --type java -B2 -A5Repository: crossroads/app.goodcity
Length of output: 833
🏁 Script executed:
# Get full MainActivity.java to see class declaration and imports
cat -n capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java | head -100Repository: crossroads/app.goodcity
Length of output: 3858
🏁 Script executed:
# Check if Capacitor's BridgeActivity or CapacitorWebView registers inset listeners
rg -A10 "class.*BridgeActivity|setOnApplyWindowInsetsListener" $(find . -path "*/capacitor/android/*" -name "*.java" 2>/dev/null | head -20) 2>/dev/null || echo "Searching in capacitor package..."Repository: crossroads/app.goodcity
Length of output: 2733
🏁 Script executed:
# Look for CapacitorWebView in the gradle dependencies and source
fd -e gradle -e kt -e java -path "*capacitor*" | head -10 | xargs rg "CapacitorWebView" 2>/dev/null | head -20Repository: crossroads/app.goodcity
Length of output: 235
🏁 Script executed:
# Check app/styles/app.scss for IME-related handling
find . -name "app.scss" -o -name "app.css" | xargs cat 2>/dev/null | head -100Repository: crossroads/app.goodcity
Length of output: 1798
🏁 Script executed:
# Check capacitor.config.ts for keyboard/IME configuration
find . -name "capacitor.config.*" | xargs cat 2>/dev/nullRepository: crossroads/app.goodcity
Length of output: 257
🏁 Script executed:
# Look for keyboard-related configuration in gradle or manifest
rg "adjustPan|adjustResize|resizeOnFullScreen|windowSoftInputMode" --type java --type gradle -B2 -A2Repository: crossroads/app.goodcity
Length of output: 49
Address IME (keyboard) inset handling and padding application to prevent overlap on gesture-navigation devices.
setPadding unconditionally overwrites the content view's padding with (bars.left, 0, bars.right, bars.bottom). The current code does not distinguish between system navigation bars and gesture navigation, and lacks handling for the input method editor (IME). On devices with gesture navigation enabled, bars.bottom includes the gesture inset; when the keyboard appears, the WebView automatically resizes its visual viewport (Chromium M139+), but the unconsumed bottom padding remains, causing the keyboard to overlap the WebView content.
Consider:
- Separating gesture navigation insets from system bar insets using
Type.systemGestures()andType.ime(), zeroing bottom padding when the keyboard is visible. - Preserving the view's existing padding by using
max(v.getPaddingLeft(), bars.left)for each side, or applying insets to a dedicated inner container rather thanandroid.R.id.content. - Returning
WindowInsetsCompat.CONSUMEDinstead of unmodified insets to prevent duplicate handling by child views.
Capacitor 6 recommends the configuration Keyboard.resizeOnFullScreen: false and separate handling of IME insets to avoid double-padding issues.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java` around
lines 67 - 88, attachSystemBarInsetsListener currently overwrites the content
view padding and doesn't handle IME or gesture insets; update the WindowInsets
handling in the ViewCompat.setOnApplyWindowInsetsListener callback (inside
attachSystemBarInsetsListener) to: compute separate Insets for systemBars,
systemGestures (WindowInsetsCompat.Type.systemBars() vs Type.systemGestures()),
and ime (Type.ime()); when ime is visible zero or reduce the bottom padding so
the keyboard doesn't overlap; preserve existing padding by using max(current
padding, computed inset) for each side (use
v.getPaddingLeft()/Top()/Right()/Bottom()); apply insets to an inner container
if possible rather than android.R.id.content, and return
WindowInsetsCompat.CONSUMED from the listener to avoid duplicate handling by
children.
| // Do not apply bars.top here: the WebView already gets top inset via | ||
| // CSS env(safe-area-inset-top) on body (see app/styles/app.scss). Applying | ||
| // native top padding as well stacks and looks like double top spacing on Android. | ||
| v.setPadding(bars.left, 0, bars.right, bars.bottom); | ||
| return insets; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does env(safe-area-inset-top) work in Capacitor Android WebView out of the box with setDecorFitsSystemWindows(false), or does it require a plugin?
💡 Result:
No, env(safe-area-inset-top) does not work reliably out of the box in Capacitor Android WebView with setDecorFitsSystemWindows(false). It requires additional configuration or a plugin due to bugs in Android WebView (Chromium versions <140), where the env values are often 0px instead of correct insets, causing content overlap with status/navigation bars in edge-to-edge mode.
Citations:
- 1: It seems that Android WebView actually does implement `safe-area-inset-*` environment variables ionic-team/capacitor#6456
- 2: bug: env(safe-area-inset-*) not working ionic-team/capacitor#6692
- 3: https://www.npmjs.com/package/@capacitor-community/safe-area
- 4: https://github.com/capacitor-community/safe-area
- 5: https://github.com/capacitor-community/safe-area/blob/master/README.md
- 6: fix(SystemBars): use native safe area insets on Android ionic-team/capacitor#8384
- 7: ionic-team/capacitor@4e99598
- 8: [Bug]: Header is behind the statusbar on Android 13 ionic-team/capacitor#8103
- 9: https://github.com/ionic-team/capacitor/blob/main/core/system-bars.md
- 10: https://capacitorjs.com/docs/apis/system-bars
🏁 Script executed:
# Find the app styles file mentioned in the comment
find . -name "app.scss" -o -name "app.css" | head -20Repository: crossroads/app.goodcity
Length of output: 88
🏁 Script executed:
# Check if safe-area plugin is in dependencies
find . -name "package.json" -type f | head -5 | xargs grep -l "safe-area" 2>/dev/null || echo "Checking all package.json files..."
find . -name "package.json" -type f -exec grep -l "safe-area\|@capacitor-community" {} \;Repository: crossroads/app.goodcity
Length of output: 101
🏁 Script executed:
# Search for setDecorFitsSystemWindows in the MainActivity
grep -n "setDecorFitsSystemWindows\|setPadding" capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java 2>/dev/null || echo "File not found or pattern not present"Repository: crossroads/app.goodcity
Length of output: 194
🏁 Script executed:
# Search for env(safe-area-inset usage in CSS/SCSS files
find . -type f \( -name "*.scss" -o -name "*.css" \) -exec grep -l "safe-area-inset\|env(" {} \;Repository: crossroads/app.goodcity
Length of output: 88
🏁 Script executed:
# Read the app.scss file to check for env(safe-area-inset-top) usage
cat app/styles/app.scss | grep -n "safe-area\|env(\|viewport-fit" -A 2 -B 2Repository: crossroads/app.goodcity
Length of output: 661
🏁 Script executed:
# Search for viewport-fit or safe-area configuration in HTML/index files
find . -name "index.html" -o -name "*.html" | xargs grep -l "viewport" 2>/dev/nullRepository: crossroads/app.goodcity
Length of output: 102
🏁 Script executed:
# Search for any JS code that might inject safe-area CSS variables
rg "safe-area-inset|evaluateJavascript|setDecorFitsSystemWindows" -t js -t ts 2>/dev/null | head -30Repository: crossroads/app.goodcity
Length of output: 49
🏁 Script executed:
# Check full context of MainActivity.java around the setPadding call
sed -n '40,95p' capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.javaRepository: crossroads/app.goodcity
Length of output: 1950
🏁 Script executed:
# Check index.html for viewport-fit and safe-area configuration
cat app/index.html | grep -i "viewport\|safe-area\|content" | head -20Repository: crossroads/app.goodcity
Length of output: 505
🏁 Script executed:
# Search for any Capacitor plugins or safe-area handling in config/index files
find . -name "capacitor.config.*" -o -name "ionic.config.*" | xargs cat 2>/dev/nullRepository: crossroads/app.goodcity
Length of output: 257
🏁 Script executed:
# Check if there's any manual implementation of CSS variable injection in JS
rg "document\.|window\." -t js -t ts -A 2 | grep -i "safe-area\|inset\|style" | head -20Repository: crossroads/app.goodcity
Length of output: 158
Implement safe-area inset handling for Android or apply top padding workaround.
The code intentionally skips native bars.top padding and relies on CSS env(safe-area-inset-top) for top spacing. However, env(safe-area-inset-top) does not work reliably in Capacitor Android WebView with setDecorFitsSystemWindows(false) without additional configuration. Android WebView (Chromium <140) has a known bug where these environment variables often return 0px instead of correct insets, causing header content to render behind the status bar on Android 15+.
The codebase does not install the @capacitor-community/safe-area plugin or provide a manual implementation (e.g., injecting the top inset as a CSS custom property via evaluateJavascript). To fix this, either:
- Install and configure a safe-area plugin, or
- Apply
bars.topas native padding here (accepting the "double top spacing" tradeoff on Android, or subtract it from CSS), or - Manually inject the top inset as a CSS variable from the native layer.
Test on a physical Android 15 device to confirm the app header is not clipped by the status bar.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@capacitor/android/app/src/main/java/hk/goodcity/app/MainActivity.java` around
lines 80 - 85, The Android WebView can return 0 for CSS
env(safe-area-inset-top), so keep the top inset handling in native code: update
the window inset handler around v.setPadding to either include bars.top (use
v.setPadding(bars.left, bars.top, bars.right, bars.bottom)) or, if you want to
avoid double-spacing, call the WebView's evaluateJavascript from the same inset
handler to set a CSS custom property (e.g.,
document.documentElement.style.setProperty('--safe-area-inset-top',
'<bars.top>px')) so the app CSS reads the native top inset; reference
v.setPadding, bars.top, insets and the WebView.evaluateJavascript call in
MainActivity.java to locate where to implement the native padding or JS
injection and test on Android 15.
Mirror async safety: await btn2Callback via RSVP, log errors, close in finally with destroy guards to avoid races and unhandled rejections.
Write provisioningProfiles into ExportOptions.plist using the downloaded .mobileprovision's Name so exportArchive re-signs with a push-capable profile. Use debugging export method for staging.
Stop forcing provisioningProfiles at export time; Xcode can then select a profile that matches the signing certificate (including push capability) when running headless with -authenticationKey* and -allowProvisioningUpdates.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
app/components/message-box.js (1)
46-48: Useconsole.warninstead of deprecatedEmber.Logger
Ember.Loggerwas deprecated in Ember 3.2.0 (not Ember 2.x) and removed in Ember 4.0.0. While the code works in Ember 2.11.0, modernize the logging for consistency and future-proofing when upgrading. Replace withconsole.warnorimport { warn } from '@ember/debug';.🛠️ Optional fix
- .catch(e => { - if (Ember.Logger && Ember.Logger.warn) { - Ember.Logger.warn("message-box btn2Callback", e); - } - }) + .catch(e => { + // eslint-disable-next-line no-console + console.warn("message-box btn2Callback", e); + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/message-box.js` around lines 46 - 48, Replace the deprecated Ember.Logger.warn usage in the btn2Callback handler with modern logging: locate the Ember.Logger.warn("message-box btn2Callback", e) call inside the btn2Callback function and change it to either console.warn("message-box btn2Callback", e) or import and use warn from '@ember/debug' (e.g., import { warn } from '@ember/debug' and call warn(`message-box btn2Callback: ${e}`, false)). Ensure the chosen replacement preserves the message and error details..circleci/config.yml (1)
218-224: Nice safeguard; consider moving the check before the Gradle build to fail fast.The check correctly matches
ANDROID_BUILD_VERSION_SEED = 270000000fromcapacitor/rename_package.jsand will catch a silent rename failure before Play Store submission. One minor refinement: running this before./gradlew bundleRelease/assembleDebugwould save several minutes on a rename regression. Non-blocking.Proposed reshuffle
ENVIRONMENT=$ENVIRONMENT CIRCLE_BUILD_NUM=$CIRCLE_BUILD_NUM yarn run rename:package npx cap sync android + if [ -n "${CIRCLE_BUILD_NUM:-}" ]; then + vc="$(awk '/versionCode /{print $2; exit}' android/app/build.gradle | tr -d '\r')" + if ! [ "${vc:-0}" -ge 270000000 ] 2>/dev/null; then + echo "Android versionCode after rename:package must be >= 270000000 (see rename_package.js ANDROID_BUILD_VERSION_SEED); got: ${vc:-empty}" + exit 1 + fi + fi if [ "$ENVIRONMENT" == "production" ]; then cd android && ./gradlew bundleRelease else cd android && ./gradlew assembleDebug fi - if [ -n "${CIRCLE_BUILD_NUM:-}" ]; then - vc="$(awk '/versionCode /{print $2; exit}' app/build.gradle | tr -d '\r')" - if ! [ "${vc:-0}" -ge 270000000 ] 2>/dev/null; then - echo "Android versionCode after rename:package must be >= 270000000 (see rename_package.js ANDROID_BUILD_VERSION_SEED); got: ${vc:-empty}" - exit 1 - fi - fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.circleci/config.yml around lines 218 - 224, Move the Android versionCode sanity check earlier in the CI workflow so it runs before any Gradle build step (e.g., before commands like ./gradlew bundleRelease or ./gradlew assembleDebug). Specifically, relocate the block that extracts vc via awk (vc="$(awk '/versionCode /{print $2; exit}' app/build.gradle | tr -d '\r')") and the guard that checks "${vc:-0}" -ge 270000000 (wrapped in the CIRCLE_BUILD_NUM conditional) to a step that precedes the Gradle invocation so rename failures fail fast and avoid wasting time on the build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Line 345: Update the CircleCI branch-filter regex used in the "only" keys so
it no longer includes the feature branch; replace occurrences of only:
/^(master|live|ionic-capacitor)$/ with only: /^(master|live)$/ in
.circleci/config.yml (there are three occurrences of that exact regex) to stop
triggering full mobile/web pipelines for the stale ionic-capacitor branch.
- Line 294: The CI config is forcibly setting IPHONEOS_DEPLOYMENT_TARGET=15 on
the xcodebuild archive invocation which overrides the project/Podfile source of
truth; remove the hardcoded environment override from the xcodebuild archive
command (the occurrence of IPHONEOS_DEPLOYMENT_TARGET=15) so the build uses the
deployment target declared in the Xcode project/Podfile instead and avoid adding
any replacement defaults in the command itself; keep the xcodebuild archive
invocation but let it inherit the project settings.
In `@app/components/message-box.js`:
- Around line 34-39: In app/components/message-box.js update the .catch handler
so it does not rethrow the error (which causes an unhandled promise rejection);
instead log the error and swallow it to match btn2Click's behavior (use
Ember.Logger.warn or the component's logger and include err), keeping the
existing close() call when this.isDestroyed/isDestroying are false; if you
intended to preserve the prior synchronous behavior of keeping the dialog open
on error, explicitly document and implement that alternative branch rather than
rethrowing.
---
Nitpick comments:
In @.circleci/config.yml:
- Around line 218-224: Move the Android versionCode sanity check earlier in the
CI workflow so it runs before any Gradle build step (e.g., before commands like
./gradlew bundleRelease or ./gradlew assembleDebug). Specifically, relocate the
block that extracts vc via awk (vc="$(awk '/versionCode /{print $2; exit}'
app/build.gradle | tr -d '\r')") and the guard that checks "${vc:-0}" -ge
270000000 (wrapped in the CIRCLE_BUILD_NUM conditional) to a step that precedes
the Gradle invocation so rename failures fail fast and avoid wasting time on the
build.
In `@app/components/message-box.js`:
- Around line 46-48: Replace the deprecated Ember.Logger.warn usage in the
btn2Callback handler with modern logging: locate the
Ember.Logger.warn("message-box btn2Callback", e) call inside the btn2Callback
function and change it to either console.warn("message-box btn2Callback", e) or
import and use warn from '@ember/debug' (e.g., import { warn } from
'@ember/debug' and call warn(`message-box btn2Callback: ${e}`, false)). Ensure
the chosen replacement preserves the message and error details.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ccbecd6-4c4f-4921-93ba-b4666ea975aa
📒 Files selected for processing (2)
.circleci/config.ymlapp/components/message-box.js
| archive \ | ||
| DEVELOPMENT_TEAM="${TEAM_ID}" \ | ||
| CODE_SIGN_IDENTITY="${CODE_SIGN_IDENTITY}" \ | ||
| IPHONEOS_DEPLOYMENT_TARGET=15 |
There was a problem hiding this comment.
Avoid overriding IPHONEOS_DEPLOYMENT_TARGET from CI.
Hardcoding IPHONEOS_DEPLOYMENT_TARGET=15 on the xcodebuild archive line overrides what's configured in the Xcode project and Podfile. If those move to a different minimum (Capacitor 8's iOS 14 baseline, or pods that require iOS 16+), this override silently drifts from the source of truth and can cause Pod/app target mismatch warnings or link-time issues. Prefer letting the project/Podfile govern the deployment target.
Proposed change
archive \
DEVELOPMENT_TEAM="${TEAM_ID}" \
- CODE_SIGN_IDENTITY="${CODE_SIGN_IDENTITY}" \
- IPHONEOS_DEPLOYMENT_TARGET=15
+ CODE_SIGN_IDENTITY="${CODE_SIGN_IDENTITY}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.circleci/config.yml at line 294, The CI config is forcibly setting
IPHONEOS_DEPLOYMENT_TARGET=15 on the xcodebuild archive invocation which
overrides the project/Podfile source of truth; remove the hardcoded environment
override from the xcodebuild archive command (the occurrence of
IPHONEOS_DEPLOYMENT_TARGET=15) so the build uses the deployment target declared
in the Xcode project/Podfile instead and avoid adding any replacement defaults
in the command itself; keep the xcodebuild archive invocation but let it inherit
the project settings.
| filters: | ||
| branches: | ||
| only: /^(master|live)$/ | ||
| only: /^(master|live|ionic-capacitor)$/ |
There was a problem hiding this comment.
Remove the ionic-capacitor branch from these filters before merging to master.
This filter is useful for validating the migration on the feature branch (the PR notes CI hasn't been exercised yet), but once merged, master|live already covers production flows. Leaving ionic-capacitor in the regex will keep triggering full mobile/web pipelines — including paid macOS minutes for ios_build_and_deploy — on any future push to that stale branch.
Cleanup to apply before merge
- www_build:
requires:
- tests
filters:
branches:
- only: /^(master|live|ionic-capacitor)$/
+ only: /^(master|live)$/
- www_deploy:
requires:
- www_build
filters:
branches:
- only: /^(master|live|ionic-capacitor)$/
+ only: /^(master|live)$/
- android_build_and_deploy:
requires:
- tests
- www_build
filters:
branches:
- only: /^(master|live|ionic-capacitor)$/
+ only: /^(master|live)$/
- ios_build_and_deploy:
requires:
- tests
- www_build
filters:
branches:
- only: /^(master|live|ionic-capacitor)$/
+ only: /^(master|live)$/Also applies to: 351-351, 358-358, 365-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.circleci/config.yml at line 345, Update the CircleCI branch-filter regex
used in the "only" keys so it no longer includes the feature branch; replace
occurrences of only: /^(master|live|ionic-capacitor)$/ with only:
/^(master|live)$/ in .circleci/config.yml (there are three occurrences of that
exact regex) to stop triggering full mobile/web pipelines for the stale
ionic-capacitor branch.
Stop requesting push permissions on app start. Trigger Capacitor push registration after auth so we can post the token to /auth/register_device. Harden token extraction and skip POST when registration_id is empty, and re-check permissions after requestPermissions on Android.
Observe authToken directly on the session service so post-login push registration fires reliably, while keeping appLoad as a no-op compatibility hook. Add an acceptance test covering permission request and push registration after auth.
Keep environment-specific Firebase config out of git while allowing CI/local builds to place it under capacitor/android/app.
Send register_device payloads using the shared-goodcity handle/platform contract, add notification tap routing for Capacitor push events, and cover post-login registration plus tapped notification routing in acceptance tests.
1 participant
Ticket Link:
#419
What does this PR do?
Migrate mobile builds from Cordova to Capacitor
Impacted Areas
Changes mainly affect iOS and Android builds; the web build should also be checked
Any Open question(s) or challenge(s):
Builds have been tested locally but not yet on CI.
Summary by CodeRabbit
New Features
Improvements
Chores