Merged
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
[REDACTED]
[REDACTED]
Suggested cleanup
[REDACTED]
🤖 Prompt for AI Agents
[REDACTED]
sample-ios/Configuration/Config.xcconfig (1)
1-1: Optional: add a short comment forTEAM_IDplaceholder.A brief note that it should be set locally for physical-device signing can reduce contributor setup friction.
Suggested tweak
+// Set your Apple Developer Team ID locally when running on a physical device. TEAM_ID=🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-ios/Configuration/Config.xcconfig` at line 1, Add a short inline comment next to the TEAM_ID placeholder explaining that TEAM_ID must be set locally for signing on physical devices (e.g., "set your Apple Developer Team ID for device signing; leave blank for CI/simulators"), so update the TEAM_ID line in Config.xcconfig to include that note; reference the TEAM_ID placeholder so contributors know where to set their local value and that it isn't required for simulator builds or CI.sample-ios/iosApp/Info.plist (1)
32-35: Remove manual architecture capability declaration for modern iOS targets.The
UIRequiredDeviceCapabilitiesentry listingarmv7is obsolete for modern iOS applications. Current guidance from Apple recommends omitting botharmv7andarm64entries for apps targeting iOS 15+, as modern deployment configurations automatically target arm64 and Xcode handles architecture settings through build configuration. Keeping this entry unnecessarily restricts device compatibility. Either remove the key entirely or replace with an empty array.Proposed update
- <key>UIRequiredDeviceCapabilities</key> - <array> - <string>armv7</string> - </array>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-ios/iosApp/Info.plist` around lines 32 - 35, Remove the manual architecture requirement from Info.plist by deleting the UIRequiredDeviceCapabilities key or replacing its array so it no longer contains the "armv7" entry; locate the UIRequiredDeviceCapabilities key and the string "armv7" in Info.plist and either remove the entire key/value pair or set the array to empty to avoid restricting modern iOS targets.sample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.kt (1)
11-19: Unusedstateparameter.The
stateparameter is accepted but not used in this stub implementation. Consider prefixing with underscore to suppress unused parameter warnings.🔧 Suggested fix
`@Composable` actual fun IntentPickerSheetContent( - state: BottomSheetState, + `@Suppress`("UNUSED_PARAMETER") state: BottomSheetState, modifier: Modifier, ) {Or alternatively:
`@Composable` actual fun IntentPickerSheetContent( - state: BottomSheetState, + state: BottomSheetState, // TODO: Use state when implementing iOS intent picker modifier: Modifier, ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.kt` around lines 11 - 19, The IntentPickerSheetContent actual implementation declares a parameter state: BottomSheetState that is unused; update the function signature or parameter name to suppress the warning by prefixing the parameter with an underscore (e.g., _state) or otherwise using state in the body, keeping the function IntentPickerSheetContent, the BottomSheetState type and Modifier parameter intact so callers remain compatible.sample-ios/iosApp.xcodeproj/project.pbxproj (1)
133-151: Consider specifying input/output paths for incremental builds.The "Compile Kotlin Framework" build phase has empty
inputPathsandoutputPaths, causing the Gradle task to run on every build. For faster incremental builds, consider specifying paths so Xcode can skip the phase when inputs haven't changed.Example input/output specification:
inputPaths = ( "$(SRCROOT)/../sample/src/", ); outputPaths = ( "$(SRCROOT)/../sample/build/xcode-frameworks/$(CONFIGURATION)/$(SDK_NAME)/SheetsSample.framework", );Note: This optimization can be deferred since the current setup is functional and the KMP Gradle plugin manages framework generation reliably.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sample-ios/iosApp.xcodeproj/project.pbxproj` around lines 133 - 151, The "Compile Kotlin Framework" PBXShellScriptBuildPhase currently has empty inputPaths/outputPaths which forces the Gradle task to run every build; update the PBXShellScriptBuildPhase entry named "Compile Kotlin Framework" to supply appropriate inputPaths (e.g., the sample module source and any build.gradle/Kotlin sources) and outputPaths (the generated framework path for the current configuration/SDK) so Xcode can perform incremental checks before running the shellScript "./gradlew :sample:embedAndSignAppleFrameworkForXcode"; ensure you reference the same phase by its name ("Compile Kotlin Framework") and populate the inputPaths and outputPaths arrays with the SRCROOT-relative paths that match your project layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sample-ios/Configuration/Config.xcconfig`:
- Line 1: Add a short inline comment next to the TEAM_ID placeholder explaining
that TEAM_ID must be set locally for signing on physical devices (e.g., "set
your Apple Developer Team ID for device signing; leave blank for
CI/simulators"), so update the TEAM_ID line in Config.xcconfig to include that
note; reference the TEAM_ID placeholder so contributors know where to set their
local value and that it isn't required for simulator builds or CI.
In `@sample-ios/iosApp.xcodeproj/project.pbxproj`:
- Around line 133-151: The "Compile Kotlin Framework" PBXShellScriptBuildPhase
currently has empty inputPaths/outputPaths which forces the Gradle task to run
every build; update the PBXShellScriptBuildPhase entry named "Compile Kotlin
Framework" to supply appropriate inputPaths (e.g., the sample module source and
any build.gradle/Kotlin sources) and outputPaths (the generated framework path
for the current configuration/SDK) so Xcode can perform incremental checks
before running the shellScript "./gradlew
:sample:embedAndSignAppleFrameworkForXcode"; ensure you reference the same phase
by its name ("Compile Kotlin Framework") and populate the inputPaths and
outputPaths arrays with the SRCROOT-relative paths that match your project
layout.
[REDACTED]
In `@sample-ios/iosApp/Info.plist`:
- Around line 32-35: Remove the manual architecture requirement from Info.plist
by deleting the UIRequiredDeviceCapabilities key or replacing its array so it no
longer contains the "armv7" entry; locate the UIRequiredDeviceCapabilities key
and the string "armv7" in Info.plist and either remove the entire key/value pair
or set the array to empty to avoid restricting modern iOS targets.
In `@sample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.kt`:
- Around line 11-19: The IntentPickerSheetContent actual implementation declares
a parameter state: BottomSheetState that is unused; update the function
signature or parameter name to suppress the warning by prefixing the parameter
with an underscore (e.g., _state) or otherwise using state in the body, keeping
the function IntentPickerSheetContent, the BottomSheetState type and Modifier
parameter intact so callers remain compatible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d8adee34-060d-4101-a64a-cf1b839a6d2a
📥 Commits
Reviewing files that changed from the base of the PR and between 82931af and 55bde16ad699376005011f96b65750278b7d79eb.
📒 Files selected for processing (17)
[REDACTED]
sample-ios/Configuration/Config.xcconfigsample-ios/iosApp.xcodeproj/project.pbxprojsample-ios/iosApp/ContentView.swiftsample-ios/iosApp/Info.plistsample-ios/iosApp/iOSApp.swiftsample/build.gradle.ktssample/src/iosMain/kotlin/com/dokar/sheets/sample/MainViewController.ktsample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.ktsample/src/iosMain/kotlin/com/dokar/sheets/sample/SampleNavigation.ktsheets-core/build.gradle.ktssheets-core/src/commonMain/kotlin/com/dokar/sheets/BackgroundWithInsetsModifier.ktsheets-core/src/commonMain/kotlin/com/dokar/sheets/CoreBottomSheetLayout.ktsheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.ktsheets-m3/build.gradle.ktssheets/build.gradle.kts
💤 Files with no reviewable changes (1)
- sheets-core/src/commonMain/kotlin/com/dokar/sheets/BackgroundWithInsetsModifier.kt
📝 WalkthroughWalkthroughThis pull request adds comprehensive iOS platform support to a Kotlin Multiplatform project. Changes include iOS Kotlin/Native targets in build configurations across modules, a new Xcode project setup with Swift entry points, iOS-specific Kotlin implementations, IME visibility handling adjustments, and documentation updates reflecting iOS as a now-supported target. Changes
Sequence DiagramsequenceDiagram
actor User
participant SwiftUI as SwiftUI<br/>(ContentView)
participant Bridge as ComposeViewController<br/>(UIViewControllerRepresentable)
participant Kotlin as Kotlin/Native<br/>(MainViewController)
participant Compose as Compose UI<br/>(SampleApp)
User->>SwiftUI: Launch App
SwiftUI->>Bridge: Create ContentView with<br/>ComposeViewController
Bridge->>Kotlin: makeUIViewController()<br/>→ MainViewController()
Kotlin->>Compose: ComposeUIViewController renders<br/>SampleApp composable
Compose->>Compose: Initialize dark theme state<br/>remember { mutableStateOf(false) }
Compose-->>Kotlin: onUpdateDarkTheme callback
Kotlin-->>Compose: isDarkTheme state updates
Compose-->>User: Render themed UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.kt (1)
32-35: Use a one-time effect for IME state initialization.
SideEffectruns after every recomposition; here the state values are constant. Prefer a one-shot effect to avoid redundant writes.♻️ Suggested simplification
+import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.Composable import androidx.compose.runtime.SideEffect @@ - SideEffect { + LaunchedEffect(Unit) { state.imeVisible = false state.hasImeVisibilityUpdated = true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.kt` around lines 32 - 35, Replace the recurring SideEffect with a one-time effect so you don't repeatedly write the same values: change the SideEffect block that sets state.imeVisible and state.hasImeVisibilityUpdated to a one-shot effect such as LaunchedEffect keyed to Unit (or another stable key) so that state.imeVisible = false and state.hasImeVisibilityUpdated = true run only once during initialization rather than on every recomposition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.kt`:
- Around line 25-42: The iOS SheetHost currently ignores the
DialogSheetBehaviors parameter; update the SheetHost function to inspect the
passed-in behaviors (DialogSheetBehaviors) and either map supported flags into
PopupProperties (e.g., set focusable/usePlatformInsets on the Popup) or, as a
minimal fail-fast guard, detect non-default/unsupported behavior flags and throw
a clear UnsupportedOperationException (or log and call onDismissRequest) so
callers get immediate feedback instead of silent no-ops; adjust the code around
the Popup/PopupProperties creation to use the computed properties and reference
SheetHost, DialogSheetBehaviors, PopupProperties, and Popup when implementing
this check/mapping.
---
Nitpick comments:
In `@sheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.kt`:
- Around line 32-35: Replace the recurring SideEffect with a one-time effect so
you don't repeatedly write the same values: change the SideEffect block that
sets state.imeVisible and state.hasImeVisibilityUpdated to a one-shot effect
such as LaunchedEffect keyed to Unit (or another stable key) so that
state.imeVisible = false and state.hasImeVisibilityUpdated = true run only once
during initialization rather than on every recomposition.
🪄 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: 0158c90e-f1e1-4cb0-b3c8-afbe70f10856
📥 Commits
Reviewing files that changed from the base of the PR and between 55bde16ad699376005011f96b65750278b7d79eb and 1908a46.
📒 Files selected for processing (17)
README.mdsample-ios/.gitignoresample-ios/Configuration/Config.xcconfigsample-ios/iosApp.xcodeproj/project.pbxprojsample-ios/iosApp/ContentView.swiftsample-ios/iosApp/Info.plistsample-ios/iosApp/iOSApp.swiftsample/build.gradle.ktssample/src/iosMain/kotlin/com/dokar/sheets/sample/MainViewController.ktsample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.ktsample/src/iosMain/kotlin/com/dokar/sheets/sample/SampleNavigation.ktsheets-core/build.gradle.ktssheets-core/src/commonMain/kotlin/com/dokar/sheets/BackgroundWithInsetsModifier.ktsheets-core/src/commonMain/kotlin/com/dokar/sheets/CoreBottomSheetLayout.ktsheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.ktsheets-m3/build.gradle.ktssheets/build.gradle.kts
💤 Files with no reviewable changes (1)
- sheets-core/src/commonMain/kotlin/com/dokar/sheets/BackgroundWithInsetsModifier.kt
✅ Files skipped from review due to trivial changes (11)
- sample-ios/.gitignore
- README.md
- sample-ios/iosApp/iOSApp.swift
- sheets-core/build.gradle.kts
- sheets-core/src/commonMain/kotlin/com/dokar/sheets/CoreBottomSheetLayout.kt
- sheets-m3/build.gradle.kts
- sample-ios/Configuration/Config.xcconfig
- sample/src/iosMain/kotlin/com/dokar/sheets/sample/Platform.ios.kt
- sample-ios/iosApp/Info.plist
- sheets/build.gradle.kts
- sample-ios/iosApp.xcodeproj/project.pbxproj
🚧 Files skipped from review as they are similar to previous changes (4)
- sample-ios/iosApp/ContentView.swift
- sample/src/iosMain/kotlin/com/dokar/sheets/sample/MainViewController.kt
- sample/build.gradle.kts
- sample/src/iosMain/kotlin/com/dokar/sheets/sample/SampleNavigation.kt
Comment on lines
+25
to
+42
| internal actual fun SheetHost( | ||
| state: BottomSheetState, | ||
| behaviors: DialogSheetBehaviors, | ||
| showAboveKeyboard: Boolean, | ||
| onDismissRequest: () -> Unit, | ||
| content: @Composable () -> Unit, | ||
| ) { | ||
| SideEffect { | ||
| state.imeVisible = false | ||
| state.hasImeVisibilityUpdated = true | ||
| } | ||
|
|
||
| Popup( | ||
| onDismissRequest = onDismissRequest, | ||
| properties = PopupProperties( | ||
| focusable = true, | ||
| usePlatformInsets = false, | ||
| ), |
There was a problem hiding this comment.
DialogSheetBehaviors is currently ignored in iOS SheetHost.
On Line 27, behaviors is accepted, but Lines 37-42 hardcode popup behavior. This makes behavior flags effectively no-op on iOS (e.g., dismissal controls), which creates cross-platform inconsistencies for callers.
💡 Minimal fail-fast guard (until full behavior mapping is implemented)
`@Composable`
internal actual fun SheetHost(
state: BottomSheetState,
behaviors: DialogSheetBehaviors,
showAboveKeyboard: Boolean,
onDismissRequest: () -> Unit,
content: `@Composable` () -> Unit,
) {
+ require(behaviors.collapseOnBackPress && behaviors.collapseOnClickOutside) {
+ "iOS SheetHost currently does not support custom collapse behavior flags."
+ }
+
SideEffect {
state.imeVisible = false
state.hasImeVisibilityUpdated = true
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sheets-core/src/iosMain/kotlin/com/dokar/sheets/Platform.ios.kt` around lines
25 - 42, The iOS SheetHost currently ignores the DialogSheetBehaviors parameter;
update the SheetHost function to inspect the passed-in behaviors
(DialogSheetBehaviors) and either map supported flags into PopupProperties
(e.g., set focusable/usePlatformInsets on the Popup) or, as a minimal fail-fast
guard, detect non-default/unsupported behavior flags and throw a clear
UnsupportedOperationException (or log and call onDismissRequest) so callers get
immediate feedback instead of silent no-ops; adjust the code around the
Popup/PopupProperties creation to use the computed properties and reference
SheetHost, DialogSheetBehaviors, PopupProperties, and Popup when implementing
this check/mapping.
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
1 participant
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 by CodeRabbit
New Features
Bug Fixes
Documentation