Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ef4609187
ℹ️ 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".
| if (bitmap.width <= 0 || bitmap.height <= 0) { | ||
| return null | ||
| } | ||
| val file = File(context.cacheDir, "status_share_${statusKey}_${System.currentTimeMillis()}.png") | ||
| FileOutputStream(file).use { |
There was a problem hiding this comment.
The share sheet builds filenames directly from statusKey.toString(), but MicroBlogKey.toString() does not escape / or :. For platforms like Bluesky where IDs include paths (e.g., at://.../app.bsky.feed.post/...), this produces filenames with path separators. That can make FileOutputStream fail (or write to unintended subdirectories) when saving/sharing images, so the share/download flow will error for those statuses. Consider normalizing the key (like UiMedia.getFileName does with replace("/", "_")) before composing the filename.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements a new “share sheet” flow for statuses to address #1673 by offering multiple share/link options (including FxEmbed and Fixvx) and screenshot sharing, replacing the prior “shareContent”-based ActionMenu sharing mechanism.
Changes:
- Introduces
DeeplinkRoute.Status.ShareSheetand routes (Android/iOS/Desktop) to open a dedicated share sheet UI. - Adds Fixvx link generation alongside existing FxEmbed links (where available) and consolidates share actions into a single menu entry.
- Adds cross-platform “share screenshot/save screenshot” UI and removes the legacy
PlatformShare/desktop share plumbing.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/route/DeeplinkRoute.kt | Adds new deeplink payload for status share sheet. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/XQT.kt | Generates Fixvx URL and routes Share action into the share sheet. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/VVO.kt | Routes Share action into the share sheet. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Misskey.kt | Routes Share action into the share sheet. |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Mastodon.kt | Routes Share action into the share sheet (guest/specific account handling). |
| shared/src/commonMain/kotlin/dev/dimension/flare/ui/model/mapper/Bluesky.kt | Routes Share action into the share sheet (replaces separate FxShare menu item). |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/xqt/XQTDataSource.kt | Adjusts cache source lookup for status-only paging key. |
| shared/src/commonMain/kotlin/dev/dimension/flare/data/datasource/microblog/ActionMenu.kt | Removes shareContent from ActionMenu items (share now via routing). |
| iosApp/flare/UI/Route/Router.swift | Allows .statusShareSheet to be presented. |
| iosApp/flare/UI/Route/Route.swift | Adds route case and mapping for share sheet screen. |
| iosApp/flare/UI/Component/ViewBox.swift | Adds a SwiftUI scaling container used for preview rendering. |
| iosApp/flare/UI/Component/Status/TimelineView.swift | Adds showTranslate flag to control translation UI presence. |
| iosApp/flare/UI/Component/Status/StatusView.swift | Gates translation UI and adds context menu actions. |
| iosApp/flare/UI/Component/Status/StatusShareSheet.swift | New iOS share sheet UI (share links + screenshot save/share + theme picker). |
| iosApp/flare/UI/Component/Status/StatusActionView.swift | Refactors iOS action rendering to rely on onClicked (no ShareLink from shareContent). |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/theme/FlareTheme.kt | Refactors theme/window provisioning (ProvideComposeWindow), adjusts sizing/background behavior. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/status/action/StatusShareSheet.kt | New desktop share sheet dialog (copy links, capture/save/share preview image). |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/screen/media/StatusMediaScreen.kt | Removes WindowScope receiver from the composable signature. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/WindowSceneStrategy.kt | Wraps window scenes with ProvideComposeWindow. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/Router.kt | Adds navigation entry for desktop status share sheet dialog route. |
| desktopApp/src/main/kotlin/dev/dimension/flare/ui/route/Route.kt | Adds desktop route + deeplink mapping for share sheet. |
| desktopApp/src/main/kotlin/dev/dimension/flare/di/DesktopModule.kt | Removes DI binding for legacy desktop share implementation. |
| desktopApp/src/main/kotlin/dev/dimension/flare/common/DesktopShareImpl.kt | Deletes legacy desktop share implementation. |
| desktopApp/src/main/kotlin/dev/dimension/flare/Main.kt | Wraps app content with ProvideComposeWindow. |
| desktopApp/src/main/composeResources/values/strings.xml | Adds desktop strings for share sheet actions. |
| desktopApp/build.gradle.kts | Minor formatting/whitespace change only. |
| compose-ui/src/jvmMain/kotlin/dev/dimension/flare/ui/common/PlatformShare.jvm.kt | Deletes legacy JVM platform sharing wrapper. |
| compose-ui/src/iosMain/kotlin/dev/dimension/flare/ui/common/PlatformShare.ios.kt | Deletes legacy iOS platform sharing wrapper. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/status/StatusMediaComponent.kt | Simplifies sensitive button logic. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/status/CommonStatusComponent.kt | Removes platform share usage; adds translate-button appearance gate; reorders visibility icon placement. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/ViewBox.kt | Adds Compose ViewBox scaling layout used for preview rendering. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/RichText.kt | Changes default color handling to use PlatformContentColor.current. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/ComponentAppearance.kt | Adds showTranslateButton appearance flag. |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/component/BuildContentAnnotatedString.kt | Restricts helper visibility (private). |
| compose-ui/src/commonMain/kotlin/dev/dimension/flare/ui/common/PlatformShare.kt | Deletes legacy expect/actual share API. |
| compose-ui/src/androidMain/kotlin/dev/dimension/flare/ui/common/PlatformShare.android.kt | Deletes legacy Android platform share implementation. |
| app/src/main/res/values/strings.xml | Updates share/save labels; adds share-via strings and share sheet theme title. |
| app/src/main/java/dev/dimension/flare/ui/screen/status/action/StatusShareSheet.kt | New Android share sheet UI (copy links, save/share screenshot, theme toggle). |
| app/src/main/java/dev/dimension/flare/ui/screen/status/StatusEntryBuilder.kt | Registers share sheet as a bottom sheet destination. |
| app/src/main/java/dev/dimension/flare/ui/route/Route.kt | Adds Android route + deeplink mapping for share sheet. |
| app/src/main/java/dev/dimension/flare/ui/component/BottomSheetSceneStrategy.kt | Adds expandFully support via wrapped bottom sheet properties. |
Comments suppressed due to low confidence (1)
desktopApp/src/main/kotlin/dev/dimension/flare/ui/theme/FlareTheme.kt:84
FlareThemeno longer appliesfillMaxSize(), so the mica backgroundBoxwill shrink-wrap its content. If a window scene's root content doesn't itself fill available space, you'll get uncovered/transparent areas. Consider restoringfillMaxSize()here, or add amodifierparameter (defaulting toModifier.fillMaxSize()) so call sites can opt out when embeddingFlareThemeinside smaller surfaces like dialogs.
internal fun FlareTheme(
isDarkTheme: Boolean = isDarkTheme(),
content: @Composable () -> Unit,
) {
FluentTheme(
compactMode = false,
colors =
if (isDarkTheme) {
darkColors()
} else {
lightColors()
},
) {
CompositionLocalProvider(
LocalIndication provides
FluentIndication(
hover = Color.Transparent,
pressed = Color.Transparent,
),
) {
Box(
modifier =
Modifier
.background(FluentTheme.colors.background.mica.base),
) {
content.invoke()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val placeable = measurable.measure(Constraints()) | ||
| val constraintWidth = | ||
| if (constraints.hasBoundedWidth) constraints.maxWidth else placeable.width | ||
| val constraintHeight = | ||
| if (constraints.hasBoundedHeight) constraints.maxHeight else placeable.height | ||
| val scaleX = constraintWidth.toFloat() / placeable.width | ||
| val scaleY = constraintHeight.toFloat() / placeable.height | ||
| var scale = min(scaleX, scaleY) | ||
| if (!stretch && scale > 1f) { | ||
| scale = 1f | ||
| } |
There was a problem hiding this comment.
ViewBox divides by placeable.width/placeable.height when computing scaleX/scaleY. If the measured content ends up 0×0 (possible for empty/Spacer content or during intermediate layouts), this will crash with a divide-by-zero. Consider guarding for zero dimensions (e.g., early layout(0,0){} or treat scale as 1f) before computing the scale.
| GeometryReader { childGeo in | ||
| let childSize = childGeo.size | ||
| let parentSize = geo.size | ||
| let scale = min( | ||
| parentSize.width / childSize.width, | ||
| parentSize.height / childSize.height | ||
| ) |
There was a problem hiding this comment.
scale is computed using parentSize.width / childSize.width and parentSize.height / childSize.height. If childSize is 0 in either dimension (can happen for views without an intrinsic size or before layout), this will produce inf/nan and can break rendering. Add a guard for zero sizes before dividing (e.g., fall back to scale=1 and/or avoid rendering the scaled overlay until a non-zero size is available).
| func snapshot(delay: TimeInterval = 0.1) async -> UIImage? { | ||
| let controller = UIHostingController(rootView: self.edgesIgnoringSafeArea(.top)) | ||
| let view = controller.view | ||
| let targetSize = controller.view.intrinsicContentSize | ||
| view?.bounds = CGRect(origin: CGPoint(x: 0, y: 0), size: targetSize) | ||
| view?.backgroundColor = .clear | ||
| try? await Task.sleep(nanoseconds: UInt64(delay * 1_000_000_000)) | ||
|
|
||
| let format = UIGraphicsImageRendererFormat() | ||
| format.scale = 3 | ||
| let renderer = UIGraphicsImageRenderer(size: targetSize, format: format) | ||
| return renderer.image { _ in | ||
| view?.drawHierarchy(in: controller.view.bounds, afterScreenUpdates: true) | ||
| } | ||
| } |
There was a problem hiding this comment.
The snapshot() helper uses controller.view.intrinsicContentSize as the render target size. For UIHostingController, intrinsicContentSize is often .noIntrinsicMetric/zero until the view has been laid out, which can yield empty or incorrectly sized screenshots. Prefer computing size via sizeThatFits(in:) (or controller.sizeThatFits(in:)) and force a layout pass (view.layoutIfNeeded()) before rendering.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd2ba03ed5
ℹ️ 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".
| .onSuccessOf(of: presenter.state.status) { data in | ||
| Task { | ||
| image = await renderImage(data: data) | ||
| } |
There was a problem hiding this comment.
The exported screenshot is generated only once in .onSuccessOf and then reused for both Save and Share, so it can drift from what the user is currently previewing. If the user switches the Theme picker (or media finishes loading after the initial render), the visible preview updates but image is not refreshed, and the shared/saved screenshot stays stale. Capture at action time or regenerate when theme/preview content changes so exports match the current preview.
Useful? React with 👍 / 👎.
| onClick = { | ||
| scope.launch { | ||
| val image = capturePreviewImage(previewGraphicsLayer) ?: return@launch | ||
| saveImageWithDialog(window, image, "status_$statusKey.png") |
There was a problem hiding this comment.
The default filename includes statusKey.toString() directly, but some keys (notably Bluesky at://...) contain / and :. On desktop, leaving this prefilled name unchanged can produce an invalid path (especially on Windows) and cause the subsequent ImageIO.write(...) save to fail. Normalize or sanitize the key before composing the filename.
Useful? React with 👍 / 👎.
1 participant
fix #1673