Conversation
4613e0d to
d81f195
Compare
This reverts commit b36ed89.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84b83fb84b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| protocol("Flare", "flare") | ||
|
|
||
| macOS { | ||
| val hasSigningProps = project.file("embedded.provisionprofile").exists() && project.file("runtime.provisionprofile").exists() |
There was a problem hiding this comment.
Guard macOS signing on entitlement plist presence
The new macOS signing gate only checks for embedded.provisionprofile and runtime.provisionprofile, but this commit also deletes desktopApp/entitlements.plist and desktopApp/runtime-entitlements.plist while still unconditionally wiring both files inside the if (hasSigningProps) block. In the tagged release path (.github/workflows/desktop.yml writes those provision profiles), hasSigningProps becomes true and packaging will fail when Gradle tries to consume missing entitlement files.
Useful? React with 👍 / 👎.
| -keep class androidx.credentials.playservices.** { | ||
| *; | ||
| } | ||
| -keep class app.bsky.** { *; } | ||
| -keep class de.jensklingenberg.ktorfit.** { *; } | ||
| -keepclassmembers class de.jensklingenberg.ktorfit.** { *; } | ||
|
|
||
| -keepnames class ** { *; } | ||
|
|
||
| # https://issuetracker.google.com/issues/442489402 | ||
| -keepclasseswithmembers class androidx.sqlite.driver.bundled.** { native <methods> ; } No newline at end of file | ||
| -keepnames class ** { *; } No newline at end of file |
There was a problem hiding this comment.
Restore BundledSQLite native keep rule for release builds
This change removes the ProGuard keep rule for native members in androidx.sqlite.driver.bundled, but the app still constructs BundledSQLiteDriver() in shared/.../ProvideDatabase.kt and Android release builds are minified (app/build.gradle.kts sets isMinifyEnabled = true). Dropping that keep rule can strip/rename JNI-linked native methods in the bundled driver, leading to runtime database initialization failures in release artifacts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates the desktop build/distribution pipeline and related runtime integrations, moving packaging to the Nucleus toolchain and removing legacy native bridge implementations for macOS/Windows.
Changes:
- Switch desktop packaging from
compose.desktopnative distributions (and C#/Swift bridges) toio.github.kdroidfilter.nucleus(Pkg/AppImage/AppX), including new AppX assets and updated CI workflows. - Update desktop window/titlebar theming and introduce a shared JVM video player/surface binding manager.
- Introduce a WebView-based login screen flow on desktop and refactor routing to support it.
Reviewed changes
Copilot reviewed 61 out of 118 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/commonMain/kotlin/dev/dimension/flare/data/network/xqt/model/FleetlineResponse.kt | Renames XQT audio space model type used in serialized response. |
| settings.gradle.kts | Adds Foojay toolchain resolver convention plugin. |
| gradle/libs.versions.toml | Adds Nucleus + webview/media deps and bumps Java toolchain version. |
| gradle/gradle-daemon-jvm.properties | Sets Gradle daemon toolchain vendor/version. |
| desktopApp/src/main/swift/macosBridge/WebViewBridge.swift | Removes macOS Swift WebView bridge implementation. |
| desktopApp/src/main/swift/macosBridge/ImageViewerBridge.swift | Removes macOS Swift image/video viewer bridge implementation. |
| desktopApp/src/main/swift/macosBridge.xcodeproj/xcuserdata/tlaster.xcuserdatad/xcschemes/xcschememanagement.plist | Removes user scheme management file. |
| desktopApp/src/main/swift/macosBridge.xcodeproj/project.xcworkspace/xcuserdata/tlaster.xcuserdatad/UserInterfaceState.xcuserstate | Workspace user state file updated/removed (user-specific). |
| desktopApp/src/main/swift/macosBridge.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved | Removes SwiftPM resolution file. |
| desktopApp/src/main/swift/macosBridge.xcodeproj/project.xcworkspace/contents.xcworkspacedata | Removes Xcode workspace contents file. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/theme/FlareTheme.kt | Integrates Nucleus decorated window theming + adjusts padding; changes autoplay/Wi‑Fi locals. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/settings/WebViewLoginScreen.kt | Removes unused/empty settings WebView login screen. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/serviceselect/WebViewLoginScreen.kt | Adds WebView login screen using composewebview with cookie polling. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/serviceselect/ServiceSelectScreen.kt | Refactors service login to navigate to WebView login screen. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/WindowSceneStrategy.kt | Adds platform window wrapper + dark theme control for overlay windows. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/Router.kt | Wires new WebView login route and updates window metadata calls. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/Route.kt | Changes WebViewLogin route shape and window/screen classification. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/FloatingWindowState.kt | Removes floating window state model. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/component/PlatformTitleBar.kt | Adds cross-platform title bar and platform window wrapper. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/component/AccountItem.kt | Simplifies AccountItem signature (removes generic). |
| desktopApp/src/main/kotlin/dev/dimension/flare/di/DesktopModule.kt | Removes old bridge bindings; adds SurfaceBindingManager singleton. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/windows/WindowsIPC.kt | Removes Windows IPC implementation. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/windows/WindowsBridge.kt | Removes Windows native bridge wrapper. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/macos/MacosBridge.kt | Removes macOS JNA bridge implementation. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/WebViewBridge.kt | Removes platform-specific WebView bridge abstraction. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/SandboxHelper.kt | Removes sandbox property helper. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/PlatformIPC.kt | Removes IPC abstractions and IPC event model. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/NativeWindowBridge.kt | Removes native media viewer bridge. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/ImageClipboardManager.kt | Moves clipboard image extraction onto IO dispatcher. |
| desktopApp/src/main/kotlin/dev/dimension/flare/Main.kt | Switches deep link + single instance handling to Nucleus runtime; adds titlebar back button wiring. |
| desktopApp/src/main/kotlin/dev/dimension/flare/App.kt | Removes native viewer routing; adds back-button state propagation for titlebar. |
| desktopApp/src/main/csharp/app.manifest | Removes legacy Windows WinUI/packaging manifest. |
| desktopApp/src/main/csharp/Properties/launchSettings.json | Removes legacy Windows launch settings. |
| desktopApp/src/main/csharp/Program.cs | Removes legacy Windows WinUI host program. |
| desktopApp/src/main/csharp/Package.appxmanifest | Removes legacy appxmanifest. |
| desktopApp/src/main/csharp/ImageEx2.cs | Removes legacy custom image control. |
| desktopApp/src/main/csharp/Flare.sln | Removes legacy Visual Studio solution. |
| desktopApp/src/main/csharp/Flare.csproj | Removes legacy Windows packaging project. |
| desktopApp/src/main/csharp/Common/DispatcherQueueExtensions.cs | Removes WinUI dispatcher helpers. |
| desktopApp/src/main/csharp/Cache/InMemoryStorageItem.cs | Removes legacy cache helper. |
| desktopApp/src/main/csharp/Cache/InMemoryStorage.cs | Removes legacy cache helper. |
| desktopApp/src/main/csharp/Cache/ImageCache.cs | Removes legacy WinUI image cache. |
| desktopApp/src/main/csharp/Assets/StoreLogo.backup.png | Leaves a legacy backup asset file (present in PR file list). |
| desktopApp/src/main/csharp/App.xaml | Removes legacy WinUI XAML resources. |
| desktopApp/src/main/csharp/.idea/.idea.Flare/.idea/.gitignore | Removes Rider/IDE ignore file under csharp subtree. |
| desktopApp/src/main/csharp/.gitignore | Removes Visual Studio .gitignore from csharp subtree. |
| desktopApp/runtime-entitlements.plist | Tightens macOS runtime entitlements (removes debug/audio/dyld env). |
| desktopApp/resources/appx/StoreLogo.scale-100.png | Adds AppX store logo asset. |
| desktopApp/resources/appx/Square44x44Logo.targetsize-48.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.targetsize-32.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.targetsize-24_altform-unplated.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.targetsize-24.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.targetsize-16.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.scale-150.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.scale-125.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.scale-100.png | Adds AppX logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-unplated_targetsize-48.png | Adds AppX altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-unplated_targetsize-32.png | Adds AppX altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-unplated_targetsize-16.png | Adds AppX altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-lightunplated_targetsize-48.png | Adds AppX lightunplated altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-lightunplated_targetsize-32.png | Adds AppX lightunplated altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-lightunplated_targetsize-24.png | Adds AppX lightunplated altform logo asset. |
| desktopApp/resources/appx/Square44x44Logo.altform-lightunplated_targetsize-16.png | Adds AppX lightunplated altform logo asset. |
| desktopApp/resources/appx/Square150x150Logo.scale-100.png | Adds AppX tile logo asset. |
| desktopApp/resources/appx/SmallTile.scale-125.png | Adds AppX small tile asset. |
| desktopApp/resources/appx/SmallTile.scale-100.png | Adds AppX small tile asset. |
| desktopApp/resources/appx/LockScreenLogo.scale-200.png | Adds AppX lock screen logo asset. |
| desktopApp/resources/appx/.gitignore | Ignores generated compose assets under appx resources. |
| desktopApp/proguard-rules.pro | Reworks ProGuard rules set (though release proguard currently disabled). |
| desktopApp/install-native-libs.gradle.kts | Removes native lib extraction tasks (sqlite/jna). |
| desktopApp/entitlements.plist | Tightens macOS entitlements (removes app identifier/team/debug/audio/dyld env). |
| desktopApp/build.gradle.kts | Migrates desktop distribution config to Nucleus plugin; configures AppX/AppImage/Pkg details. |
| desktopApp/build-swift.gradle.kts | Removes Swift bridge build task integration. |
| desktopApp/.gitignore | Stops ignoring resources/* to include packaged resources in VCS. |
| compose-ui/src/jvmMain/kotlin/dev/dimension/flare/ui/component/platform/PlatformVideoPlayer.jvm.kt | Implements JVM video player via new shared VideoPlayer composable. |
| compose-ui/src/jvmMain/kotlin/dev/dimension/flare/ui/component/VideoPlayer.kt | Adds JVM video player wrapper + SurfaceBindingManager for single-surface playback. |
| compose-ui/build.gradle.kts | Adds composemediaplayer dependency on JVM source set. |
| build.gradle.kts | Adds Nucleus plugin alias; enforces JetBrains toolchain vendor selection across Kotlin targets. |
| app/proguard-rules.pro | Simplifies Android ProGuard rules (keeps keepnames broad rule). |
| .github/workflows/server.yml | Updates CI to setup-java v5 and Java 25 (JetBrains). |
| .github/workflows/ios.yml | Updates CI to setup-java v5 and Java 25 (JetBrains). |
| .github/workflows/desktop.yml | Updates desktop CI (macos runner, Java 25, packaging tasks, Linux job, AppX bundle creation, artifact actions). |
| .github/workflows/android.yml | Updates Android CI to Java 25 (JetBrains) and setup-java v5 usage. |
Files not reviewed (2)
- desktopApp/src/main/csharp/.idea/.idea.Flare/.idea/.gitignore: Language not supported
- desktopApp/src/main/swift/macosBridge.xcodeproj/project.xcworkspace/contents.xcworkspacedata: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LaunchedEffect(visible, autoPlay, playerState) { | ||
| if (visible && autoPlay) { | ||
| playerState?.play() | ||
| } else { | ||
| playerState?.pause() | ||
| } | ||
| } | ||
|
|
||
| LaunchedEffect(playerState?.isPlaying, playerState?.sliderPos) { | ||
| isLoaded = playerState?.isPlaying == true && playerState.sliderPos > 0f | ||
| } | ||
|
|
||
| Box( | ||
| modifier = | ||
| modifier | ||
| .onLayoutRectChanged(debounceMillis = 300) { | ||
| currentRect = it.boundsInWindow | ||
| }.onVisibilityChanged(300, 0.66f) { | ||
| visible = it | ||
| }, | ||
| ) { | ||
| if (!isLoaded && LocalIsScrollingInProgress.current || !visible || playerState == null) { | ||
| idlePlaceholder() | ||
| } else { | ||
| AnimatedContent( | ||
| isLoaded, | ||
| transitionSpec = { |
There was a problem hiding this comment.
When autoPlay is false, LaunchedEffect(visible, autoPlay, playerState) will keep the player paused, so isLoaded stays false. With the current branching, a visible item will then fall into the AnimatedContent(isLoaded) path and show the loadingPlaceholder (including a progress ring) indefinitely, even though nothing is loading. Consider treating !autoPlay (or !playerState.isPlaying) as an idle state and showing idlePlaceholder instead of loadingPlaceholder.
| data class WebViewLogin( | ||
| val url: String, | ||
| val cookieCallback: ((cookies: String?) -> Unit)?, | ||
| ) : WindowRoute | ||
| val callback: (cookies: String?) -> Boolean, | ||
| ) : ScreenRoute |
There was a problem hiding this comment.
Route.WebViewLogin stores a function-typed callback inside a data class that implements NavKey. This makes route equality/hashCode depend on a lambda instance (often capturing presenters/UI state), which can break navigation key stability and can retain references longer than intended. Prefer passing a stable identifier (e.g., callbackId) and resolving the callback via a scoped registry/ViewModel, or model the login flow state in a presenter and have the screen read it instead of carrying lambdas in the route.
| @@ -31,9 +32,9 @@ internal fun ServiceSelectScreen(onBack: () -> Unit) { | |||
| ServiceSelectionScreenContent( | |||
| contentPadding = LocalWindowPadding.current, | |||
| onXQT = { | |||
| webviewBridge.openAndWaitCookies( | |||
| onWebViewLogin.invoke( | |||
| "https://${UiApplication.XQT.host}", | |||
| callback = { cookies -> | |||
| { cookies -> | |||
| if (cookies.isNullOrEmpty()) { | |||
| false | |||
| } else { | |||
| @@ -45,11 +46,25 @@ internal fun ServiceSelectScreen(onBack: () -> Unit) { | |||
| } | |||
| }, | |||
| ) | |||
| // webviewBridge.openAndWaitCookies( | |||
| // "https://${UiApplication.XQT.host}", | |||
| // callback = { cookies -> | |||
| // if (cookies.isNullOrEmpty()) { | |||
| // false | |||
| // } else { | |||
| // xqtLoginState.checkChocolate(cookies).also { | |||
| // if (it) { | |||
| // xqtLoginState.login(cookies) | |||
| // } | |||
| // } | |||
| // } | |||
| // }, | |||
| // ) | |||
| }, | |||
There was a problem hiding this comment.
There is a large block of commented-out code (old WebViewBridge path) left in this screen. Keeping dead code in-line makes it harder to maintain and review; it should be removed (git history retains it). If you still need both implementations, consider keeping them behind a feature flag/config instead of commented blocks.
| @@ -245,11 +245,48 @@ internal fun ProvideThemeSettings(content: @Composable () -> Unit) { | |||
| showPlatformLogo = appearanceSettings.showPlatformLogo, | |||
| ) | |||
| }, | |||
| LocalWifiState provides true, | |||
| content = { | |||
There was a problem hiding this comment.
ProvideThemeSettings now forces videoAutoplay = ALWAYS and also hardcodes LocalWifiState to true. This changes user-facing playback behavior (e.g., Wi‑Fi-only autoplay can never apply) and seems unrelated to the PR title (“update desktop packaging”). If the intent is to keep existing settings behavior on desktop, please use appearanceSettings.videoAutoplay and provide a real Wi‑Fi state (or leave the default false) instead of overriding both here.
No description provided.