Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds SettingsActivity for OBS connection settings, routes MainActivity to it, replaces alert-based update prompts with custom dialogs, updates StreamConfig presets, and restyles the Android app to a minimalist glass visual system. ChangesSettings and updater flow
Estimated code review effort: 4 (Complex) | ~60 minutes Minimalist glass UI restyle
Estimated code review effort: 3 (Moderate) | ~35 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![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: 36d1c18e9e
ℹ️ 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".
| btnSettings.setOnClickListener { | ||
| val intent = Intent(this, SettingsActivity::class.java) | ||
| startActivity(intent) |
There was a problem hiding this comment.
When OBS discovery is unavailable or the user needs a direct host/port connection, replacing the ADV toggle with a settings launcher leaves no visible control that calls startStream(connectionTargetFromManualFields()): SettingsActivity only saves preferences and finishes, while btnManualConnect remains inside manualContainer, which is always gone. The saved OBS host/port values therefore cannot be used to initiate a manual caller connection.
Useful? React with 👍 / 👎.
| <View | ||
| android:id="@+id/screenOffOverlay" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" |
There was a problem hiding this comment.
In display-off mode, toggleDisplayOff() still only makes screenOffOverlay visible, but this redesign moved the overlay inside previewContainer; the bottom control panel is now a sibling below it, so DISPLAY mode blacks out only the preview while leaving the controls visible and interactive. On OLED devices this defeats the intended hide-all-UI/power-saving behavior during streaming.
Useful? React with 👍 / 👎.
| val dialog = android.app.Dialog(this, R.style.BrutalistDialogTheme) | ||
| dialog.setContentView(R.layout.dialog_custom_update) |
There was a problem hiding this comment.
On first launch after each installed version, this confirmation reuses dialog_custom_update, whose title is @string/update_title (UPDATE AVAILABLE), and only changes the message/action button. That shows users a false update-available dialog even though the message says they are already running the installed version.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/dev/openstream/app/MainActivity.kt (1)
859-896: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOverride the dialog title for the installed-version flow
dialog_custom_updatestill rendersUPDATE AVAILABLEfrom@string/update_title, so this "you are running vX" dialog shows an update prompt title unless the title view is changed here or the layout is split.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt` around lines 859 - 896, The installed-version dialog in showVersionInstalledDialogOnce() still uses the default title from dialog_custom_update, so override that title explicitly for this flow. Update the title TextView after inflating the dialog (or use a separate layout) so the message matches the “You are running OpenStream vX” content instead of showing the update prompt; locate the change in showVersionInstalledDialogOnce(), dialog_custom_update, and the title/text view setup.
🧹 Nitpick comments (13)
android/app/src/main/res/layout/activity_main.xml (1)
26-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
@color/os_blackinstead of hardcoded#FF000000.Rest of the file consistently references color resources; this hardcoded literal duplicates
os_black.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_main.xml` around lines 26 - 33, The screenOffOverlay view in activity_main.xml is using a hardcoded black literal instead of the existing color resource. Replace the android:background value on the screenOffOverlay View with the shared `@color/os_black` resource so the layout stays consistent with the rest of the file and avoids duplicating the same color definition.android/app/src/main/res/drawable/bg_brutalist_btn_danger.xml (1)
1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer color/dimen resource references over hardcoded literals.
This drawable duplicates values already defined as resources (
#FF000000==os_shadow,#FFEF4444==os_destructive,4dp/3dp==os_brutalist_shadow_x/y/os_brutalist_border). Hardcoding them here (and presumably across the otherbg_brutalist_*drawables) means future palette/spacing tweaks require touching every drawable file individually instead of one source of truth.♻️ Suggested fix
<layer-list xmlns:android="http://schemas.android.com/apk/res/android"> <!-- Shadow layer --> <item - android:left="4dp" - android:top="4dp"> + android:left="`@dimen/os_brutalist_shadow_x`" + android:top="`@dimen/os_brutalist_shadow_y`"> <shape android:shape="rectangle"> - <solid android:color="`#FF000000`" /> + <solid android:color="`@color/os_shadow`" /> <corners android:radius="4dp" /> </shape> </item> <!-- Foreground layer --> <item - android:right="4dp" - android:bottom="4dp"> + android:right="`@dimen/os_brutalist_shadow_x`" + android:bottom="`@dimen/os_brutalist_shadow_y`"> <shape android:shape="rectangle"> - <solid android:color="`#FFEF4444`" /> + <solid android:color="`@color/os_destructive`" /> <stroke - android:width="3dp" - android:color="`#FF000000`" /> + android:width="`@dimen/os_brutalist_border`" + android:color="`@color/os_shadow`" /> <corners android:radius="4dp" /> </shape> </item> </layer-list>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_brutalist_btn_danger.xml` around lines 1 - 24, The drawable bg_brutalist_btn_danger is hardcoding colors and dimensions that already exist as shared resources. Update this XML to reference the existing color and dimen resources used by the brutalist theme instead of inline literals, and apply the same pattern to the other bg_brutalist_* drawables so the shared values stay centralized. Use the resource names already defined for shadow, destructive color, border width, and brutalist shadow offsets when editing this layer-list.android/app/src/main/res/values/dimens.xml (2)
43-44: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
os_preview_heightis unused and its value contradicts its comment.The comment describes a "roughly 9:16 portrait ratio" but the value is
0dp, andpreviewContainerinactivity_main.xmluses a hardcoded0dp+layout_weight="1"instead of this dimen. This looks like a stale/orphaned token from an earlier fixed-height design.🧹 Suggested cleanup
- <!-- ── Preview height: roughly 9:16 portrait ratio on 360dp-wide phone ── --> - <dimen name="os_preview_height">0dp</dimen> + <!-- os_preview_height removed: previewContainer now uses layout_weight for sizing -->🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/values/dimens.xml` around lines 43 - 44, Remove the stale os_preview_height dimen from dimens.xml or wire it into the actual preview sizing logic so it is no longer orphaned; the current value conflicts with its “9:16 portrait ratio” comment, and previewContainer in activity_main.xml is already using a hardcoded 0dp with layout_weight instead of this token. Update the related layout and any references to use a single source of truth, or delete the unused symbol if the fixed-height design is no longer needed.
38-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNew brutalist tokens (
os_brutalist_border,os_brutalist_shadow_x/y) aren't referenced by the brutalist drawables.
bg_brutalist_btn_danger.xmlhardcodes4dp/3dpliterals matching these values instead of using@dimen/os_brutalist_borderetc. Referencing the tokens would keep shadow offset/border width consistent if adjusted later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/values/dimens.xml` around lines 38 - 42, The brutalist dimension tokens are defined but not used in the brutalist drawable, so update bg_brutalist_btn_danger.xml to reference os_brutalist_border and os_brutalist_shadow_x/y instead of hardcoded dp literals. Keep the drawable values driven by the existing dimen resources so border width and shadow offsets stay consistent with future token changes.android/app/src/main/res/drawable/bg_btn_accent.xml (1)
2-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRegression: theme/resource-based fill replaced with hardcoded hex, losing theming flexibility.
Per the line-range change details, the prior version used a theme/resource-based fill color and corner radius; this now hardcodes
#FFFFAB00/#FF000000/4dp. If the app supports any light/dark or dynamic theming, this button (used live forbtnTorchperMainActivity.kt:163) will no longer respond to it. Confirm whether theme-driven colors are still needed elsewhere, and if so, route this back through@color/@dimentokens instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_btn_accent.xml` around lines 2 - 24, The bg_btn_accent drawable now hardcodes the foreground, stroke, and corner radius values, which removes the theme/resource-based styling previously used. Update the layer-list in bg_btn_accent.xml to reference the existing color and dimension resources again (or equivalent theme attributes) so btnTorch in MainActivity continues to adapt to light/dark or dynamic themes. Keep the structure of the drawable the same, but replace the literal hex and dp values with the app’s resource tokens used for the accent button styling.android/app/src/main/res/drawable/bg_brutalist_btn_ghost.xml (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded colors bypass the new color tokens; inconsistent with sibling brutalist drawables.
Colors (
#FF1E1E28,#FFFFFFFF) and the4dpradius are inlined here and duplicated verbatim acrossbg_brutalist_btn_primary.xml,bg_brutalist_btn_secondary.xml,bg_brutalist_card.xml, andbg_brutalist_input.xml. This cohort's summary states colors.xml/dimens.xml were extended specifically to back this theme — referencing@color/...and@dimen/...instead would keep the palette/radius centrally editable.Also, unlike
bg_brutalist_btn_primary.xml/bg_brutalist_btn_secondary.xml, this ghost button uses a plain<shape>with no shadow-offset layer, breaking the shadow+foreground "brutalist" visual consistency used by sibling buttons.♻️ Suggested direction
<shape xmlns:android="http://schemas.android.com/apk/res/android" android:shape="rectangle"> - <solid android:color="`#FF1E1E28`" /> + <solid android:color="`@color/os_surface_alt`" /> <stroke android:width="3dp" - android:color="`#FFFFFFFF`" /> - <corners android:radius="4dp" /> + android:color="`@color/os_border`" /> + <corners android:radius="`@dimen/os_corner_radius`" /> </shape>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_brutalist_btn_ghost.xml` around lines 1 - 9, Update bg_brutalist_btn_ghost to stop inlining the solid, stroke, and corner values and instead reference the shared theme tokens already added in colors.xml and dimens.xml via `@color/`... and `@dimen/`... like the other brutalist drawables. Also align this drawable with bg_brutalist_btn_primary and bg_brutalist_btn_secondary by wrapping the shape in the same shadow-offset/layered structure so the ghost button keeps the brutalist shadow-and-foreground treatment rather than a plain single shape.android/app/src/main/res/drawable/bg_btn_ghost.xml (1)
1-9: 📐 Maintainability & Code Quality | 🔵 TrivialHardcoded values instead of theme tokens.
Same pattern as the other brutalist drawables — colors/stroke width/corner radius are hardcoded rather than referencing the new
colors.xml/dimens.xmltokens.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_btn_ghost.xml` around lines 1 - 9, The bg_btn_ghost drawable uses hardcoded color, stroke width, and corner radius values instead of the shared theme tokens. Update this shape resource to reference the existing color and dimension resources from colors.xml and dimens.xml, matching the pattern used by the other brutalist drawables, so the styling stays centralized and consistent.android/app/src/main/res/drawable/bg_btn_danger.xml (1)
2-24: 📐 Maintainability & Code Quality | 🔵 TrivialHardcoded colors/dimens duplicate values instead of referencing tokens.
This PR's cohort also updates
colors.xml/dimens.xmlwith brutalist tokens (corner radii, colors), but this drawable hardcodes#FF000000,#FFEF4444,4dpinline instead of referencing@color/@dimen. The same magic values are duplicated across all the brutalist drawables in this PR, making future theme tweaks require edits in many files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_btn_danger.xml` around lines 2 - 24, The bg_btn_danger drawable is still using inline color and dimension values instead of the new brutalist resource tokens. Update the layer-list items in bg_btn_danger.xml to reference the matching `@color` and `@dimen` entries from colors.xml and dimens.xml for the shadow, foreground, stroke, and corner radius values, and apply the same token-based pattern used by the other brutalist drawables so the shared values stay centralized.android/app/src/main/res/drawable/bg_lens_selector.xml (1)
3-20: 📐 Maintainability & Code Quality | 🔵 TrivialSelected-state color no longer references any resource.
The selected item's amber fill (
#FFFFAB00) is hardcoded and no longer tied to a named color token, unlike the other brutalist tokens introduced elsewhere in this PR. Consider aligning withcolors.xmlfor consistency with the rest of the restyle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_lens_selector.xml` around lines 3 - 20, The selected-state fill in bg_lens_selector is hardcoded instead of using a shared color token. Update the selected <item> in this selector drawable to reference the appropriate named color from colors.xml, matching the token-based approach used elsewhere in the restyle, and keep the existing selector structure in place.android/app/src/main/res/layout/activity_settings.xml (2)
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the newly added string resources instead of hardcoded labels.
strings.xmladdssection_settings,settings_obs_host,settings_obs_port,settings_latency, andsettings_listening_portspecifically for this screen, but this layout hardcodes the equivalent text directly instead of referencing them — breaking localization and leaving those new resources unused.🛠️ Proposed fix
- android:text="SETTINGS" + android:text="`@string/section_settings`" ... - android:text="OBS HOST" + android:text="`@string/settings_obs_host`" ... - android:text="OBS PORT" + android:text="`@string/settings_obs_port`" ... - android:text="LATENCY (ms)" + android:text="`@string/settings_latency`" ... - android:text="LISTENING PORT" + android:text="`@string/settings_listening_port`"Also applies to: 32-32, 59-59, 87-87, 115-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_settings.xml` at line 20, The settings layout is still hardcoding labels instead of using the new string resources, which leaves localization broken and the added resources unused. Update activity_settings.xml to replace the text values in the affected views with the corresponding references to section_settings, settings_obs_host, settings_obs_port, settings_latency, and settings_listening_port, keeping the rest of the layout unchanged.
40-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider linking labels to inputs via
android:labelFor.Each EditText has a preceding label TextView but no
labelForassociation, which reduces the usefulness of screen readers on this settings form.Also applies to: 67-81, 95-109, 123-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_settings.xml` around lines 40 - 53, The settings form EditText fields in activity_settings.xml have preceding TextView labels but no accessibility association. Add android:labelFor on each corresponding label TextView so it points to the matching input id (for example the settingsObsHost field and the other similar pairs in this layout), keeping the label/input relationship intact for screen readers.android/app/src/main/res/layout/dialog_custom_permission.xml (1)
15-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded shadow color instead of a theme token.
Every other color in this layout uses a named resource (
@color/os_accent,@color/os_text_primary, etc.), but the shadow uses a raw#FF000000. Extracting this to a shared color (e.g.,@color/os_shadow) would keep it consistent with the rest of the brutalist token system and easier to retheme later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/dialog_custom_permission.xml` at line 15, The shadow color in the dialog layout is hardcoded instead of using the shared token system. Update the background in dialog_custom_permission.xml to reference a named color resource, and add or reuse a token such as os_shadow so it matches the existing resource-based styling used by the layout.android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt (1)
127-146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the shared dialog-creation boilerplate.
showUpdateAvailableandshowInstallPermissionPromptboth repeat the sameDialog(activity, R.style.BrutalistDialogTheme)→setContentView→setCancelable(true)→ dismiss-wiring pattern. A small helper (e.g.,createBrutalistDialog(layoutRes): Dialog) would reduce duplication for this and any future custom dialogs.♻️ Example helper extraction
+ private fun createBrutalistDialog(`@LayoutRes` layoutRes: Int): Dialog = + Dialog(activity, R.style.BrutalistDialogTheme).apply { + setContentView(layoutRes) + setCancelable(true) + } + private fun showUpdateAvailable(release: ReleaseUpdate) { - val dialog = Dialog(activity, R.style.BrutalistDialogTheme) - dialog.setContentView(R.layout.dialog_custom_update) - dialog.setCancelable(true) + val dialog = createBrutalistDialog(R.layout.dialog_custom_update)Also applies to: 200-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt` around lines 127 - 146, Extract the repeated custom-dialog setup shared by showUpdateAvailable and showInstallPermissionPrompt into a small helper such as createBrutalistDialog(layoutRes), and use it from AppUpdater. Keep the helper responsible for constructing the Dialog with R.style.BrutalistDialogTheme, setting the content view, and making it cancelable, so the two dialog methods only handle their specific view wiring and button callbacks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt`:
- Around line 207-216: The port value in MainActivity.onResume is being
overwritten by a stale persisted setting because the stepper changes only update
currentPort in memory. Update the port-change flow in MainActivity, especially
changePort and the btnPortUp/btnPortDown handlers, so any successful port change
is also saved to SettingsActivity.PREFS_NAME with KEY_LISTENING_PORT. Keep the
in-memory currentPort and shared preference value synchronized so onResume does
not revert a stepper-adjusted port.
In `@android/app/src/main/java/dev/openstream/app/SettingsActivity.kt`:
- Around line 61-88: Store blank OBS port/latency as unset instead of defaulting
them in SettingsActivity. Update saveSettings() so empty inputObsPort and
inputLatency are persisted as 0, and keep the existing coercion only for
non-empty values; then adjust loadSettings() to treat 0 as unset and leave those
fields blank. This should align with
MainActivity.connectionTargetFromManualFields() and preserve the manual override
path after saving.
In `@android/app/src/main/res/drawable/bg_btn_danger.xml`:
- Around line 2-24: The danger button drawable currently uses a static
layer-list with no pressed-state handling, so touch feedback is lost. Update
bg_btn_danger.xml by restoring state-aware behavior around the existing
layer-list shape: add a selector for the default and pressed appearances, or
wrap this drawable with a ripple/pressed variant so tapping still shows a visual
change. Keep the current look defined by the layer-list, but make sure the
button’s pressed state is handled explicitly.
In `@android/app/src/main/res/drawable/bg_btn_ghost.xml`:
- Around line 1-9: Restore pressed-state feedback in bg_btn_ghost by replacing
the static shape with a stateful drawable selector so the torch toggle can show
different visuals when pressed versus default. Keep the existing ghost styling
as the default item and add a pressed item with a visibly distinct appearance,
then ensure MainActivity.kt’s torch-off button continues to reference
R.drawable.bg_btn_ghost so the state change is picked up automatically.
In `@android/app/src/main/res/layout/dialog_custom_permission.xml`:
- Around line 9-15: The shadow layer `LinearLayout` in the custom
permission/update dialog layouts is using a collapsible height, so it can
disappear instead of filling the dialog. Update the shadow view in both
`dialog_custom_permission` and `dialog_custom_update` so it uses a full-height
layout setting, and keep the rest of the dialog structure unchanged to preserve
the intended offset shadow effect.
---
Outside diff comments:
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt`:
- Around line 859-896: The installed-version dialog in
showVersionInstalledDialogOnce() still uses the default title from
dialog_custom_update, so override that title explicitly for this flow. Update
the title TextView after inflating the dialog (or use a separate layout) so the
message matches the “You are running OpenStream vX” content instead of showing
the update prompt; locate the change in showVersionInstalledDialogOnce(),
dialog_custom_update, and the title/text view setup.
---
Nitpick comments:
In `@android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt`:
- Around line 127-146: Extract the repeated custom-dialog setup shared by
showUpdateAvailable and showInstallPermissionPrompt into a small helper such as
createBrutalistDialog(layoutRes), and use it from AppUpdater. Keep the helper
responsible for constructing the Dialog with R.style.BrutalistDialogTheme,
setting the content view, and making it cancelable, so the two dialog methods
only handle their specific view wiring and button callbacks.
In `@android/app/src/main/res/drawable/bg_brutalist_btn_danger.xml`:
- Around line 1-24: The drawable bg_brutalist_btn_danger is hardcoding colors
and dimensions that already exist as shared resources. Update this XML to
reference the existing color and dimen resources used by the brutalist theme
instead of inline literals, and apply the same pattern to the other
bg_brutalist_* drawables so the shared values stay centralized. Use the resource
names already defined for shadow, destructive color, border width, and brutalist
shadow offsets when editing this layer-list.
In `@android/app/src/main/res/drawable/bg_brutalist_btn_ghost.xml`:
- Around line 1-9: Update bg_brutalist_btn_ghost to stop inlining the solid,
stroke, and corner values and instead reference the shared theme tokens already
added in colors.xml and dimens.xml via `@color/`... and `@dimen/`... like the other
brutalist drawables. Also align this drawable with bg_brutalist_btn_primary and
bg_brutalist_btn_secondary by wrapping the shape in the same
shadow-offset/layered structure so the ghost button keeps the brutalist
shadow-and-foreground treatment rather than a plain single shape.
In `@android/app/src/main/res/drawable/bg_btn_accent.xml`:
- Around line 2-24: The bg_btn_accent drawable now hardcodes the foreground,
stroke, and corner radius values, which removes the theme/resource-based styling
previously used. Update the layer-list in bg_btn_accent.xml to reference the
existing color and dimension resources again (or equivalent theme attributes) so
btnTorch in MainActivity continues to adapt to light/dark or dynamic themes.
Keep the structure of the drawable the same, but replace the literal hex and dp
values with the app’s resource tokens used for the accent button styling.
In `@android/app/src/main/res/drawable/bg_btn_danger.xml`:
- Around line 2-24: The bg_btn_danger drawable is still using inline color and
dimension values instead of the new brutalist resource tokens. Update the
layer-list items in bg_btn_danger.xml to reference the matching `@color` and
`@dimen` entries from colors.xml and dimens.xml for the shadow, foreground,
stroke, and corner radius values, and apply the same token-based pattern used by
the other brutalist drawables so the shared values stay centralized.
In `@android/app/src/main/res/drawable/bg_btn_ghost.xml`:
- Around line 1-9: The bg_btn_ghost drawable uses hardcoded color, stroke width,
and corner radius values instead of the shared theme tokens. Update this shape
resource to reference the existing color and dimension resources from colors.xml
and dimens.xml, matching the pattern used by the other brutalist drawables, so
the styling stays centralized and consistent.
In `@android/app/src/main/res/drawable/bg_lens_selector.xml`:
- Around line 3-20: The selected-state fill in bg_lens_selector is hardcoded
instead of using a shared color token. Update the selected <item> in this
selector drawable to reference the appropriate named color from colors.xml,
matching the token-based approach used elsewhere in the restyle, and keep the
existing selector structure in place.
In `@android/app/src/main/res/layout/activity_main.xml`:
- Around line 26-33: The screenOffOverlay view in activity_main.xml is using a
hardcoded black literal instead of the existing color resource. Replace the
android:background value on the screenOffOverlay View with the shared
`@color/os_black` resource so the layout stays consistent with the rest of the
file and avoids duplicating the same color definition.
In `@android/app/src/main/res/layout/activity_settings.xml`:
- Line 20: The settings layout is still hardcoding labels instead of using the
new string resources, which leaves localization broken and the added resources
unused. Update activity_settings.xml to replace the text values in the affected
views with the corresponding references to section_settings, settings_obs_host,
settings_obs_port, settings_latency, and settings_listening_port, keeping the
rest of the layout unchanged.
- Around line 40-53: The settings form EditText fields in activity_settings.xml
have preceding TextView labels but no accessibility association. Add
android:labelFor on each corresponding label TextView so it points to the
matching input id (for example the settingsObsHost field and the other similar
pairs in this layout), keeping the label/input relationship intact for screen
readers.
In `@android/app/src/main/res/layout/dialog_custom_permission.xml`:
- Line 15: The shadow color in the dialog layout is hardcoded instead of using
the shared token system. Update the background in dialog_custom_permission.xml
to reference a named color resource, and add or reuse a token such as os_shadow
so it matches the existing resource-based styling used by the layout.
In `@android/app/src/main/res/values/dimens.xml`:
- Around line 43-44: Remove the stale os_preview_height dimen from dimens.xml or
wire it into the actual preview sizing logic so it is no longer orphaned; the
current value conflicts with its “9:16 portrait ratio” comment, and
previewContainer in activity_main.xml is already using a hardcoded 0dp with
layout_weight instead of this token. Update the related layout and any
references to use a single source of truth, or delete the unused symbol if the
fixed-height design is no longer needed.
- Around line 38-42: The brutalist dimension tokens are defined but not used in
the brutalist drawable, so update bg_brutalist_btn_danger.xml to reference
os_brutalist_border and os_brutalist_shadow_x/y instead of hardcoded dp
literals. Keep the drawable values driven by the existing dimen resources so
border width and shadow offsets stay consistent with future token changes.
🪄 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: 21f55e03-2424-4d90-8e48-c560562986a6
📒 Files selected for processing (27)
android/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/dev/openstream/app/MainActivity.ktandroid/app/src/main/java/dev/openstream/app/SettingsActivity.ktandroid/app/src/main/java/dev/openstream/app/update/AppUpdater.ktandroid/app/src/main/res/drawable/bg_brutalist_btn_danger.xmlandroid/app/src/main/res/drawable/bg_brutalist_btn_ghost.xmlandroid/app/src/main/res/drawable/bg_brutalist_btn_primary.xmlandroid/app/src/main/res/drawable/bg_brutalist_btn_secondary.xmlandroid/app/src/main/res/drawable/bg_brutalist_card.xmlandroid/app/src/main/res/drawable/bg_brutalist_input.xmlandroid/app/src/main/res/drawable/bg_btn_accent.xmlandroid/app/src/main/res/drawable/bg_btn_danger.xmlandroid/app/src/main/res/drawable/bg_btn_ghost.xmlandroid/app/src/main/res/drawable/bg_card.xmlandroid/app/src/main/res/drawable/bg_chip.xmlandroid/app/src/main/res/drawable/bg_controls_panel.xmlandroid/app/src/main/res/drawable/bg_lens_selector.xmlandroid/app/src/main/res/drawable/live_dot.xmlandroid/app/src/main/res/drawable/scrim_bottom.xmlandroid/app/src/main/res/layout/activity_main.xmlandroid/app/src/main/res/layout/activity_settings.xmlandroid/app/src/main/res/layout/dialog_custom_permission.xmlandroid/app/src/main/res/layout/dialog_custom_update.xmlandroid/app/src/main/res/values/colors.xmlandroid/app/src/main/res/values/dimens.xmlandroid/app/src/main/res/values/strings.xmlandroid/app/src/main/res/values/styles.xml
| override fun onResume() { | ||
| super.onResume() | ||
| appUpdater.resumePendingInstallIfAllowed() | ||
| // Reload listening port from settings if it changed | ||
| val settingsPrefs = getSharedPreferences(SettingsActivity.PREFS_NAME, MODE_PRIVATE) | ||
| val savedPort = settingsPrefs.getInt(SettingsActivity.KEY_LISTENING_PORT, currentPort) | ||
| if (savedPort != currentPort && savedPort in 1024..65535) { | ||
| changePort(savedPort) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Port stepper changes can be silently reverted by stale Settings-persisted value.
The +/- port stepper buttons (btnPortUp/btnPortDown) update currentPort only in memory and never persist to SettingsActivity.PREFS_NAME. If a user adjusts the port via the stepper and later opens/saves the Settings screen (even without touching the listening-port field), onResume will read the now-stale persisted value and call changePort(), silently discarding the stepper's change.
Keep both sources in sync by persisting the port whenever it changes:
🛠️ Proposed fix
private fun changePort(newPort: Int) {
val clamped = newPort.coerceIn(1024, 65535)
if (clamped == currentPort) return
currentPort = clamped
portLabel.text = currentPort.toString()
renderConnectionInfo()
+ getSharedPreferences(SettingsActivity.PREFS_NAME, MODE_PRIVATE).edit()
+ .putInt(SettingsActivity.KEY_LISTENING_PORT, currentPort)
+ .apply()
// Restart the phone server on the new port🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt` around lines
207 - 216, The port value in MainActivity.onResume is being overwritten by a
stale persisted setting because the stepper changes only update currentPort in
memory. Update the port-change flow in MainActivity, especially changePort and
the btnPortUp/btnPortDown handlers, so any successful port change is also saved
to SettingsActivity.PREFS_NAME with KEY_LISTENING_PORT. Keep the in-memory
currentPort and shared preference value synchronized so onResume does not revert
a stepper-adjusted port.
| private fun loadSettings() { | ||
| val prefs = getSharedPreferences(PREFS_NAME, MODE_PRIVATE) | ||
| inputObsHost.setText(prefs.getString(KEY_OBS_HOST, "")) | ||
| val port = prefs.getInt(KEY_OBS_PORT, ConnectionTarget.DEFAULT_PORT) | ||
| if (port != ConnectionTarget.DEFAULT_PORT) inputObsPort.setText(port.toString()) | ||
| val latency = prefs.getInt(KEY_LATENCY, ConnectionTarget.DEFAULT_LATENCY_MS) | ||
| if (latency != ConnectionTarget.DEFAULT_LATENCY_MS) inputLatency.setText(latency.toString()) | ||
| val listenPort = prefs.getInt(KEY_LISTENING_PORT, ConnectionTarget.DEFAULT_PORT) | ||
| inputListeningPort.setText(listenPort.toString()) | ||
| } | ||
|
|
||
| private fun saveSettings() { | ||
| val host = inputObsHost.text.toString().trim() | ||
| val port = inputObsPort.text.toString().toIntOrNull() ?: ConnectionTarget.DEFAULT_PORT | ||
| val latency = inputLatency.text.toString().toIntOrNull() ?: ConnectionTarget.DEFAULT_LATENCY_MS | ||
| val listenPort = inputListeningPort.text.toString().toIntOrNull() ?: ConnectionTarget.DEFAULT_PORT | ||
|
|
||
| getSharedPreferences(PREFS_NAME, MODE_PRIVATE).edit() | ||
| .putString(KEY_OBS_HOST, host) | ||
| .putInt(KEY_OBS_PORT, port.coerceIn(1, 65535)) | ||
| .putInt(KEY_LATENCY, latency.coerceIn(20, 2000)) | ||
| .putInt(KEY_LISTENING_PORT, listenPort.coerceIn(1024, 65535)) | ||
| .apply() | ||
|
|
||
| Toast.makeText(this, "Settings saved", Toast.LENGTH_SHORT).show() | ||
| setResult(RESULT_OK) | ||
| finish() | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant files and symbols
git ls-files | rg 'android/app/src/main/java/dev/openstream/app/(SettingsActivity|MainActivity|ConnectionTarget|AppUpdater)\.kt$'
echo '--- SettingsActivity outline ---'
ast-grep outline android/app/src/main/java/dev/openstream/app/SettingsActivity.kt --view expanded || true
echo '--- MainActivity outline ---'
ast-grep outline android/app/src/main/java/dev/openstream/app/MainActivity.kt --view expanded || true
echo '--- ConnectionTarget outline ---'
ast-grep outline android/app/src/main/java/dev/openstream/app/ConnectionTarget.kt --view expanded || true
echo '--- AppUpdater outline ---'
ast-grep outline android/app/src/main/java/dev/openstream/app/AppUpdater.kt --view expanded || trueRepository: YashasVM/OpenStream
Length of output: 7197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SettingsActivity.kt (1-170) ---'
sed -n '1,170p' android/app/src/main/java/dev/openstream/app/SettingsActivity.kt
echo '--- MainActivity.kt (880-940) ---'
sed -n '880,940p' android/app/src/main/java/dev/openstream/app/MainActivity.kt
echo '--- Search ConnectionTarget / AppUpdater symbols ---'
rg -n "class ConnectionTarget|object ConnectionTarget|data class ConnectionTarget|class AppUpdater|object AppUpdater" android/app/src/main/javaRepository: YashasVM/OpenStream
Length of output: 7472
Store empty OBS Port/Latency as unset
saveSettings() writes ConnectionTarget.DEFAULT_PORT/DEFAULT_LATENCY_MS when those fields are blank, but MainActivity.connectionTargetFromManualFields() only falls back to the inline inputs when the saved value is 0. That makes the settings values sticky and disables the manual override path after the first save. Store 0 for empty fields and treat > 0 as set in loadSettings().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/java/dev/openstream/app/SettingsActivity.kt` around
lines 61 - 88, Store blank OBS port/latency as unset instead of defaulting them
in SettingsActivity. Update saveSettings() so empty inputObsPort and
inputLatency are persisted as 0, and keep the existing coercion only for
non-empty values; then adjust loadSettings() to treat 0 as unset and leave those
fields blank. This should align with
MainActivity.connectionTargetFromManualFields() and preserve the manual override
path after saving.
| <layer-list xmlns:android="http://schemas.android.com/apk/res/android"> | ||
| <!-- Shadow layer --> | ||
| <item | ||
| android:left="4dp" | ||
| android:top="4dp"> | ||
| <shape android:shape="rectangle"> | ||
| <solid android:color="#33FF1744" /> | ||
| <corners android:radius="@dimen/os_corner_sm" /> | ||
| <stroke android:width="1dp" android:color="@color/os_live_red" /> | ||
| <solid android:color="#FF000000" /> | ||
| <corners android:radius="4dp" /> | ||
| </shape> | ||
| </item> | ||
| <item> | ||
| <!-- Foreground layer --> | ||
| <item | ||
| android:right="4dp" | ||
| android:bottom="4dp"> | ||
| <shape android:shape="rectangle"> | ||
| <solid android:color="#1AFF1744" /> | ||
| <corners android:radius="@dimen/os_corner_sm" /> | ||
| <stroke android:width="1dp" android:color="@color/os_live_red_dim" /> | ||
| <solid android:color="#FFEF4444" /> | ||
| <stroke | ||
| android:width="3dp" | ||
| android:color="#FF000000" /> | ||
| <corners android:radius="4dp" /> | ||
| </shape> | ||
| </item> | ||
| </selector> | ||
| </layer-list> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Lost pressed-state feedback on danger button.
The previous <selector> provided a distinct pressed-state visual (per the diff summary). The new layer-list is a static shape with no state_pressed handling, so tapping this button now gives no visual feedback. Consider re-adding a pressed state (e.g., wrap in a <selector> with a pressed variant, or use a <ripple> foreground) to preserve touch feedback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/res/drawable/bg_btn_danger.xml` around lines 2 - 24, The
danger button drawable currently uses a static layer-list with no pressed-state
handling, so touch feedback is lost. Update bg_btn_danger.xml by restoring
state-aware behavior around the existing layer-list shape: add a selector for
the default and pressed appearances, or wrap this drawable with a ripple/pressed
variant so tapping still shows a visual change. Keep the current look defined by
the layer-list, but make sure the button’s pressed state is handled explicitly.
| <?xml version="1.0" encoding="utf-8"?> | ||
| <selector xmlns:android="http://schemas.android.com/apk/res/android"> | ||
| <item android:state_pressed="true"> | ||
| <shape android:shape="rectangle"> | ||
| <solid android:color="#33FFFFFF" /> | ||
| <corners android:radius="@dimen/os_corner_sm" /> | ||
| </shape> | ||
| </item> | ||
| <item> | ||
| <shape android:shape="rectangle"> | ||
| <solid android:color="#1AFFFFFF" /> | ||
| <corners android:radius="@dimen/os_corner_sm" /> | ||
| </shape> | ||
| </item> | ||
| </selector> | ||
| <shape xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:shape="rectangle"> | ||
| <solid android:color="#FF1E1E28" /> | ||
| <stroke | ||
| android:width="3dp" | ||
| android:color="#FFFFFFFF" /> | ||
| <corners android:radius="4dp" /> | ||
| </shape> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Lost pressed-state feedback on ghost/torch-off button.
Per the change summary, the previous <selector> had separate pressed/default items; this is now a static <shape> with no pressed state. Since this drawable is used for the torch toggle button, tapping it no longer shows any press feedback.
As shown in the context snippet, MainActivity.kt uses R.drawable.bg_btn_ghost for the "torch off" state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/res/drawable/bg_btn_ghost.xml` around lines 1 - 9,
Restore pressed-state feedback in bg_btn_ghost by replacing the static shape
with a stateful drawable selector so the torch toggle can show different visuals
when pressed versus default. Keep the existing ghost styling as the default item
and add a pressed item with a visibly distinct appearance, then ensure
MainActivity.kt’s torch-off button continues to reference
R.drawable.bg_btn_ghost so the state change is picked up automatically.
| <LinearLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:orientation="vertical" | ||
| android:layout_marginStart="4dp" | ||
| android:layout_marginTop="4dp" | ||
| android:background="#FF000000" /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== dialog_custom_permission.xml ==\n'
sed -n '1,220p' android/app/src/main/res/layout/dialog_custom_permission.xml | cat -n
printf '\n== dialog_custom_update.xml ==\n'
sed -n '1,220p' android/app/src/main/res/layout/dialog_custom_update.xml | cat -nRepository: YashasVM/OpenStream
Length of output: 8588
🌐 Web query:
Android FrameLayout measure children match_parent second measurement pass wrap_content zero height background does not affect measured size
💡 Result:
In Android's FrameLayout, the behavior where children set to match_parent appear to have zero size when the FrameLayout is set to wrap_content occurs because FrameLayout performs two measurement passes under specific conditions [1]. When a FrameLayout is constrained (e.g., its parent does not provide an exact size), it cannot know the final dimensions of its children in the first pass [2][3]. If a child is set to match_parent, it effectively asks the FrameLayout, "How big is the parent?" [4]. If the FrameLayout itself is set to wrap_content, it initially measures that child with a measurement specification of UNSPECIFIED or AT_MOST, which often leads the child to report a measured size of zero [1][4]. The "second measurement pass" mechanism is a specific optimization in FrameLayout to resolve this [1][3]. When the FrameLayout detects that it has children with match_parent dimensions while it is not under an exact constraint, it collects these children into a list [2][5]. After the first pass determines the FrameLayout's own size based on non-match_parent children (or defaults), it performs a second pass [2][5]. In this second pass, it forces the collected match_parent children to measure themselves again, this time providing the FrameLayout's newly determined dimensions as an EXACT constraint [2][3]. Regarding the background, the FrameLayout's measured size is determined by its largest child plus padding [6][7]. If no children have intrinsic size (e.g., they are all empty or match_parent in a wrap_content container), the background's minimum dimensions (if any, like a drawable's intrinsic size) are taken into account [7]. If the background has no intrinsic size, the container may collapse to zero size despite the background being set [8]. Note that simply setting a background does not automatically force a container to expand if the child layout parameters are effectively zero-sized [8]. The background drawable's own minimum requirements (e.g., via setMinimumWidth/Height) are only considered by the FrameLayout during its final measurement calculation, but they do not override the fundamental logic that requires valid child dimensions to determine the final size [7].
Citations:
- 1: https://blog.danlew.net/2014/03/24/an-android-view-measurement-case-study/
- 2: https://android.googlesource.com/platform/frameworks/base/+/08a79197da3ceff52f8dbd82ac62710c229f6e19/core/java/android/widget/FrameLayout.java
- 3: https://android.googlesource.com/platform/prebuilts/fullsdk/sources/android-30/+/refs/heads/androidx-appcompat-release/android/widget/FrameLayout.java
- 4: https://developer.android.com/guide/topics/ui/how-android-draws
- 5: https://android.googlesource.com/platform/frameworks/base/%2B/master/core/java/android/widget/FrameLayout.java
- 6: https://developer.android.com/reference/kotlin/android/widget/FrameLayout
- 7: https://android.googlesource.com/platform/frameworks/base/+/ace8addce47efc03be5038eef48d7fb066b14aae/core/java/android/widget/FrameLayout.java
- 8: https://stackoverflow.com/questions/45483900/wrap-content-doesnt-work-with-background
Make the shadow layer fill the dialog in both layouts
android/app/src/main/res/layout/dialog_custom_permission.xml and android/app/src/main/res/layout/dialog_custom_update.xml both use an empty shadow LinearLayout with layout_height="wrap_content", so it can collapse to 0 and the offset shadow never appears. Set it to match_parent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/res/layout/dialog_custom_permission.xml` around lines 9
- 15, The shadow layer `LinearLayout` in the custom permission/update dialog
layouts is using a collapsible height, so it can disappear instead of filling
the dialog. Update the shadow view in both `dialog_custom_permission` and
`dialog_custom_update` so it uses a full-height layout setting, and keep the
rest of the dialog structure unchanged to preserve the intended offset shadow
effect.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39abb11c89
ℹ️ 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 (release.isNewerThan(currentVersionName(), currentVersionCode())) { | ||
| showUpdateAvailable(release) | ||
| downloadApk(release) |
There was a problem hiding this comment.
When the startup update check runs from MainActivity after UPDATE_CHECK_DELAY_MS, any newer release now goes straight into downloadApk(release) instead of showing the previous confirmation dialog. This means simply opening the app can enqueue an APK download over metered data with no chance to decline, and the new custom update dialog layout is bypassed for actual update availability.
Useful? React with 👍 / 👎.
|
@codex the latest commits fix the choppy video and slot switching, please review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9caba1b236
ℹ️ 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".
| encoder.start() | ||
| runCatching { audioEncoder.start() }.onFailure { e -> | ||
| Log.w("OpenStream", "Audio encoder start failed", e) | ||
| } | ||
| camera.startStreaming(encoder.inputSurface()) |
There was a problem hiding this comment.
Starting a stream now only starts the video encoder; the previous audioEncoder.start() call was removed in both the direct caller path and the listener path, while MediaCodecAudioEncoder is still the only code that feeds sendAudioAccessUnit. Any successful stream from this build will therefore be silent in OBS despite the app still requesting microphone permission and advertising AAC audio.
Useful? React with 👍 / 👎.
| .onSuccess { release -> | ||
| if (release.isNewerThan(currentVersionName(), currentVersionCode())) { | ||
| showUpdateAvailable(release) | ||
| downloadApk(release) |
There was a problem hiding this comment.
When MainActivity runs its silent startup check (checkForUpdates() with the default showAlreadyCurrent = false), this branch now immediately enqueues the APK download instead of showing the update prompt that existed before. Because downloadApk() allows metered connections, users can get an unsolicited update download just by opening the app; keep the explicit confirmation path for background checks and only download after user action.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/src/main/java/dev/openstream/app/MainActivity.kt (1)
901-923: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winLatency clamp range diverges from
SettingsActivity's validation range.
SettingsActivity.saveSettings()persists latency withcoerceIn(20, 2000), but here the effective connection target re-clamps tocoerceIn(80, 2000). A user who saves e.g. 40ms in Settings will see it silently bumped to 80ms when actually connecting, with no indication the persisted value differs from the applied value.🔧 Proposed fix
return ConnectionTarget( name = ConnectionTarget.DEFAULT_NAME, host = host, port = port.coerceIn(1, 65535), - latencyMs = latencyMs.coerceIn(80, 2000), + latencyMs = latencyMs.coerceIn(20, 2000), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt` around lines 901 - 923, The latency handling in connectionTargetFromManualFields is re-clamping to a different minimum than SettingsActivity.saveSettings, so the applied ConnectionTarget can differ from the persisted preference. Update connectionTargetFromManualFields to use the same latency bounds as SettingsActivity’s validation/save path, and keep the clamp logic aligned when building the ConnectionTarget so saved and applied latency values match.
♻️ Duplicate comments (1)
android/app/src/main/java/dev/openstream/app/MainActivity.kt (1)
961-980: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPort stepper changes still not persisted — stale Settings value can revert them on resume.
changePort()updatescurrentPortin memory only; it never writes toSettingsActivity.PREFS_NAME. SinceonResume()(Lines 214-219) reads the persisted listening port and callschangePort(savedPort)whenever it differs fromcurrentPort, a stepper-adjusted port that was never saved to Settings will be silently reverted the next time the activity resumes.🛠️ Proposed fix
private fun changePort(newPort: Int) { val clamped = newPort.coerceIn(1024, 65535) if (clamped == currentPort) return currentPort = clamped portLabel.text = currentPort.toString() renderConnectionInfo() + getSharedPreferences(SettingsActivity.PREFS_NAME, MODE_PRIVATE).edit() + .putInt(SettingsActivity.KEY_LISTENING_PORT, currentPort) + .apply() // Restart the phone server on the new port🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt` around lines 961 - 980, changePort() only updates the in-memory port, so stepper changes can be lost when onResume() reloads the persisted value and re-applies it. Update MainActivity.changePort() to also persist the clamped port into SettingsActivity.PREFS_NAME using the same key that onResume() reads before restarting the phone server and recreating PhoneDiscoveryAdvertiser. Keep the existing currentPort, portLabel, renderConnectionInfo, stopPhoneServer, and phoneAdvertiser refresh flow unchanged.
🧹 Nitpick comments (8)
android/app/src/main/res/layout/dialog_custom_permission.xml (1)
14-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded dialog text instead of string resources.
Title, message, and button labels ("Permission Required", "Cancel", "Settings") are hardcoded rather than pulled from
strings.xml, blocking localization for this new dialog.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/dialog_custom_permission.xml` around lines 14 - 63, The dialog in dialog_custom_permission.xml uses hardcoded text for the title, message, and button labels instead of string resources, which prevents localization. Update the TextView fields in this layout to reference existing or new entries in strings.xml for the permission title, body copy, Cancel, and Settings labels, and keep the IDs such as dialogPermissionDismiss and dialogPermissionAction unchanged so the code wiring still works.android/app/src/main/res/layout/activity_settings.xml (1)
16-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExternalize hardcoded UI text to string resources.
Hints (
@string/hint_obs_host, etc.) are externalized, but the screen title, field labels, and all three button labels ("Settings", "OBS Host", "OBS Port", "Latency (ms)", "Listening Port", "Check for Updates", "Save Settings", "Back") are hardcoded. This is inconsistent within the same file and blocks localization.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_settings.xml` around lines 16 - 163, Externalize the remaining hardcoded UI text in activity_settings.xml into string resources to keep the screen localizable and consistent with the existing hint strings. Update the TextView labels and button texts used by the settings screen, including the title, field labels, and the btnCheckUpdates, btnSaveSettings, and btnBackSettings texts, to reference `@string` values. Keep the existing layout structure and resource naming aligned with the current settings UI identifiers so the referenced views remain easy to locate.android/app/src/main/res/layout/dialog_custom_update.xml (1)
14-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded title/button text instead of string resources.
"Update Ready", "Later", and "Install" are hardcoded. Same localization gap as the sibling
dialog_custom_permission.xml.Also applies to: 38-63
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/dialog_custom_update.xml` around lines 14 - 21, The custom update dialog still hardcodes user-facing text instead of using string resources. Update the TextView and the later/install button labels in dialog_custom_update.xml to reference existing or new string resources, following the same pattern used in dialog_custom_permission.xml, and make sure any other affected views in the same layout also use resource-based text so the dialog is localizable.android/app/src/main/res/layout/activity_main.xml (2)
140-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent token usage:
paddingTop="32dp"vspaddingBottom="@dimen/os_spacing_xl".Both are the same 32dp value; the top uses a literal while the bottom uses the dimen resource in the same view.
♻️ Proposed fix
- android:paddingTop="32dp" + android:paddingTop="`@dimen/os_spacing_xl`" android:paddingBottom="`@dimen/os_spacing_xl`"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_main.xml` around lines 140 - 141, The padding values in the same view are inconsistent because activity_main.xml uses a literal 32dp for android:paddingTop while android:paddingBottom already references the shared dimension resource. Update the view attributes to use the same dimen resource for both padding values, keeping the existing `@dimen/os_spacing_xl` usage consistent throughout the layout.
45-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded spacing values instead of the new
os_spacing_*scale.Paddings here (
56dp,32dp,12dp,16dp,8dp) are literals even though this PR just introduced a spacing scale (os_spacing_xs/sm/md/lg/xl/xxl) for exactly this purpose. Several values (e.g. the32dpbottom padding) have an exact token match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/layout/activity_main.xml` around lines 45 - 127, The top overlay layout is still using hardcoded padding values instead of the new spacing tokens. Update the padding/margin values in activity_main.xml around the top overlay, liveBadge, streamInfoChip, and zoomLabel to use the existing os_spacing_* dimens wherever there is an exact match, especially replacing literals like 32dp and 16dp/12dp/8dp with the corresponding scale values.android/app/src/main/res/drawable/bg_btn_accent.xml (1)
2-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReference the new design tokens instead of hardcoding duplicate values.
999dpand#FFFFFFFFexactly match@dimen/os_corner_pilland@color/os_accent, both newly defined in this PR'scolors.xml/dimens.xml. Hardcoding here (and identically inbg_minimal_pill_active.xml) defeats the token system this restyle introduces and makes future theme/color tweaks require multi-file edits.♻️ Proposed fix
<shape xmlns:android="http://schemas.android.com/apk/res/android" android:shape="rectangle"> - <corners android:radius="999dp" /> - <solid android:color="`#FFFFFFFF`" /> + <corners android:radius="`@dimen/os_corner_pill`" /> + <solid android:color="`@color/os_accent`" /> </shape>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_btn_accent.xml` around lines 2 - 6, Replace the hardcoded pill radius and white fill in the drawable with the new shared design tokens so the style stays token-driven. Update the shape used by bg_btn_accent to reference `@dimen/os_corner_pill` for the corner radius and `@color/os_accent` for the solid color, and apply the same token swap in bg_minimal_pill_active so both drawables use the centralized values instead of duplicated literals.android/app/src/main/res/drawable/bg_minimal_btn_ghost.xml (1)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate of
bg_btn_ghost.xml— consider consolidating.This new drawable is byte-for-byte identical to
bg_btn_ghost.xml(999dpcorners, transparent fill,1dp#33FFFFFFstroke). Reusing the existing drawable instead of duplicating it would avoid two files drifting apart over time. It also inherits the same missing pressed-state feedback as the other ghost/danger buttons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_minimal_btn_ghost.xml` around lines 1 - 7, The new drawable duplicates the existing ghost button shape, so consolidate it by reusing the shared drawable instead of keeping a second identical XML resource. Update the references for bg_minimal_btn_ghost to point at bg_btn_ghost, or otherwise centralize the shape definition in a single drawable so future changes only happen in one place. Use the drawable resource names bg_minimal_btn_ghost and bg_btn_ghost to locate and update the duplicated asset.android/app/src/main/res/drawable/bg_btn_danger.xml (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded hex color bypasses the theme token system.
#FF3B30is hardcoded instead of referencing a color resource. This PR introduces new color tokens incolors.xmlfor the minimalist theme; using raw hex here (and in the other restyled drawables) undermines centralized theming and makes future palette tweaks error-prone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/res/drawable/bg_btn_danger.xml` at line 5, The drawable background is using a hardcoded hex value instead of the theme token system. Update bg_btn_danger and the other restyled drawable resources to reference the corresponding color resource from colors.xml rather than using raw hex, so the centralized theming introduced for the minimalist palette is used consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt`:
- Around line 533-537: The background selection in MainActivity.kt’s
setBackgroundResource block has a redundant branch because both the
reserved-but-not-connected case and the available case use the same drawable.
Update the conditional so the reserved state maps to a distinct background asset
from the available state, using the existing isReservedForThisPhone and
phoneConnected checks to separate live/active from non-live slots and remove the
dead duplicate branch.
---
Outside diff comments:
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt`:
- Around line 901-923: The latency handling in connectionTargetFromManualFields
is re-clamping to a different minimum than SettingsActivity.saveSettings, so the
applied ConnectionTarget can differ from the persisted preference. Update
connectionTargetFromManualFields to use the same latency bounds as
SettingsActivity’s validation/save path, and keep the clamp logic aligned when
building the ConnectionTarget so saved and applied latency values match.
---
Duplicate comments:
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt`:
- Around line 961-980: changePort() only updates the in-memory port, so stepper
changes can be lost when onResume() reloads the persisted value and re-applies
it. Update MainActivity.changePort() to also persist the clamped port into
SettingsActivity.PREFS_NAME using the same key that onResume() reads before
restarting the phone server and recreating PhoneDiscoveryAdvertiser. Keep the
existing currentPort, portLabel, renderConnectionInfo, stopPhoneServer, and
phoneAdvertiser refresh flow unchanged.
---
Nitpick comments:
In `@android/app/src/main/res/drawable/bg_btn_accent.xml`:
- Around line 2-6: Replace the hardcoded pill radius and white fill in the
drawable with the new shared design tokens so the style stays token-driven.
Update the shape used by bg_btn_accent to reference `@dimen/os_corner_pill` for
the corner radius and `@color/os_accent` for the solid color, and apply the same
token swap in bg_minimal_pill_active so both drawables use the centralized
values instead of duplicated literals.
In `@android/app/src/main/res/drawable/bg_btn_danger.xml`:
- Line 5: The drawable background is using a hardcoded hex value instead of the
theme token system. Update bg_btn_danger and the other restyled drawable
resources to reference the corresponding color resource from colors.xml rather
than using raw hex, so the centralized theming introduced for the minimalist
palette is used consistently.
In `@android/app/src/main/res/drawable/bg_minimal_btn_ghost.xml`:
- Around line 1-7: The new drawable duplicates the existing ghost button shape,
so consolidate it by reusing the shared drawable instead of keeping a second
identical XML resource. Update the references for bg_minimal_btn_ghost to point
at bg_btn_ghost, or otherwise centralize the shape definition in a single
drawable so future changes only happen in one place. Use the drawable resource
names bg_minimal_btn_ghost and bg_btn_ghost to locate and update the duplicated
asset.
In `@android/app/src/main/res/layout/activity_main.xml`:
- Around line 140-141: The padding values in the same view are inconsistent
because activity_main.xml uses a literal 32dp for android:paddingTop while
android:paddingBottom already references the shared dimension resource. Update
the view attributes to use the same dimen resource for both padding values,
keeping the existing `@dimen/os_spacing_xl` usage consistent throughout the
layout.
- Around line 45-127: The top overlay layout is still using hardcoded padding
values instead of the new spacing tokens. Update the padding/margin values in
activity_main.xml around the top overlay, liveBadge, streamInfoChip, and
zoomLabel to use the existing os_spacing_* dimens wherever there is an exact
match, especially replacing literals like 32dp and 16dp/12dp/8dp with the
corresponding scale values.
In `@android/app/src/main/res/layout/activity_settings.xml`:
- Around line 16-163: Externalize the remaining hardcoded UI text in
activity_settings.xml into string resources to keep the screen localizable and
consistent with the existing hint strings. Update the TextView labels and button
texts used by the settings screen, including the title, field labels, and the
btnCheckUpdates, btnSaveSettings, and btnBackSettings texts, to reference
`@string` values. Keep the existing layout structure and resource naming aligned
with the current settings UI identifiers so the referenced views remain easy to
locate.
In `@android/app/src/main/res/layout/dialog_custom_permission.xml`:
- Around line 14-63: The dialog in dialog_custom_permission.xml uses hardcoded
text for the title, message, and button labels instead of string resources,
which prevents localization. Update the TextView fields in this layout to
reference existing or new entries in strings.xml for the permission title, body
copy, Cancel, and Settings labels, and keep the IDs such as
dialogPermissionDismiss and dialogPermissionAction unchanged so the code wiring
still works.
In `@android/app/src/main/res/layout/dialog_custom_update.xml`:
- Around line 14-21: The custom update dialog still hardcodes user-facing text
instead of using string resources. Update the TextView and the later/install
button labels in dialog_custom_update.xml to reference existing or new string
resources, following the same pattern used in dialog_custom_permission.xml, and
make sure any other affected views in the same layout also use resource-based
text so the dialog is localizable.
🪄 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: 7e9f965b-c306-4c17-98b9-8b3d5c401398
📒 Files selected for processing (29)
android/app/build.gradle.ktsandroid/app/src/main/java/dev/openstream/app/MainActivity.ktandroid/app/src/main/java/dev/openstream/app/stream/StreamConfig.ktandroid/app/src/main/java/dev/openstream/app/update/AppUpdater.ktandroid/app/src/main/res/drawable/bg_btn_accent.xmlandroid/app/src/main/res/drawable/bg_btn_danger.xmlandroid/app/src/main/res/drawable/bg_btn_ghost.xmlandroid/app/src/main/res/drawable/bg_card.xmlandroid/app/src/main/res/drawable/bg_chip.xmlandroid/app/src/main/res/drawable/bg_controls_panel.xmlandroid/app/src/main/res/drawable/bg_lens_selector.xmlandroid/app/src/main/res/drawable/bg_minimal_btn.xmlandroid/app/src/main/res/drawable/bg_minimal_btn_danger.xmlandroid/app/src/main/res/drawable/bg_minimal_btn_ghost.xmlandroid/app/src/main/res/drawable/bg_minimal_input.xmlandroid/app/src/main/res/drawable/bg_minimal_panel.xmlandroid/app/src/main/res/drawable/bg_minimal_pill.xmlandroid/app/src/main/res/drawable/bg_minimal_pill_active.xmlandroid/app/src/main/res/drawable/live_dot.xmlandroid/app/src/main/res/drawable/scrim_bottom.xmlandroid/app/src/main/res/drawable/scrim_top.xmlandroid/app/src/main/res/layout/activity_main.xmlandroid/app/src/main/res/layout/activity_settings.xmlandroid/app/src/main/res/layout/dialog_custom_permission.xmlandroid/app/src/main/res/layout/dialog_custom_update.xmlandroid/app/src/main/res/values/colors.xmlandroid/app/src/main/res/values/dimens.xmlandroid/app/src/main/res/values/styles.xmlbuild_output.txt
💤 Files with no reviewable changes (1)
- android/app/src/main/java/dev/openstream/app/stream/StreamConfig.kt
✅ Files skipped from review due to trivial changes (8)
- android/app/src/main/res/drawable/bg_minimal_pill.xml
- android/app/src/main/res/drawable/bg_minimal_btn.xml
- android/app/src/main/res/drawable/bg_minimal_panel.xml
- android/app/src/main/res/drawable/scrim_top.xml
- android/app/src/main/res/drawable/bg_minimal_btn_danger.xml
- build_output.txt
- android/app/build.gradle.kts
- android/app/src/main/res/drawable/bg_chip.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/res/drawable/live_dot.xml
| setBackgroundResource( | ||
| if (isReservedForThisPhone && phoneConnected) R.drawable.bg_minimal_pill_active | ||
| else if (isReservedForThisPhone) R.drawable.bg_minimal_pill | ||
| else R.drawable.bg_minimal_pill | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Redundant branch: "Reserved" and "Available" slots render with the same background.
Both the isReservedForThisPhone (not yet connected) branch and the final else branch resolve to the same R.drawable.bg_minimal_pill, even though a distinct bg_minimal_pill_active-style asset exists for active/live state. The middle condition is dead code and reserved-but-not-live slots are visually indistinguishable from available ones (aside from text color).
🎨 Proposed fix
setBackgroundResource(
if (isReservedForThisPhone && phoneConnected) R.drawable.bg_minimal_pill_active
- else if (isReservedForThisPhone) R.drawable.bg_minimal_pill
+ else if (isReservedForThisPhone) R.drawable.bg_minimal_pill_active
else R.drawable.bg_minimal_pill
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setBackgroundResource( | |
| if (isReservedForThisPhone && phoneConnected) R.drawable.bg_minimal_pill_active | |
| else if (isReservedForThisPhone) R.drawable.bg_minimal_pill | |
| else R.drawable.bg_minimal_pill | |
| ) | |
| setBackgroundResource( | |
| if (isReservedForThisPhone && phoneConnected) R.drawable.bg_minimal_pill_active | |
| else if (isReservedForThisPhone) R.drawable.bg_minimal_pill_active | |
| else R.drawable.bg_minimal_pill | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android/app/src/main/java/dev/openstream/app/MainActivity.kt` around lines
533 - 537, The background selection in MainActivity.kt’s setBackgroundResource
block has a redundant branch because both the reserved-but-not-connected case
and the available case use the same drawable. Update the conditional so the
reserved state maps to a distinct background asset from the available state,
using the existing isReservedForThisPhone and phoneConnected checks to separate
live/active from non-live slots and remove the dead duplicate branch.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex Please review the slot switching fix and the new updater consent prompt. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex Please review the audio encoder restoration and update dialog layout fix. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt (1)
148-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate dialog-construction logic.
showUpdatePromptandshowInstallPermissionPromptboth build a themedDialog, inflate a layout,findViewByIdaction/dismissTextViews, and wire near-identical dismiss listeners. Consider extracting a small helper (e.g.createThemedDialog(layoutRes): Dialog) to reduce duplication as more dialogs are added.♻️ Example extraction
+ private fun createThemedDialog(`@LayoutRes` layoutRes: Int): Dialog { + val dialog = Dialog(activity, R.style.MinimalDialogTheme) + dialog.setContentView(layoutRes) + dialog.setCancelable(true) + return dialog + } + private fun showUpdatePrompt(release: ReleaseUpdate) { - val dialog = Dialog(activity, R.style.MinimalDialogTheme) - dialog.setContentView(R.layout.dialog_custom_update) - dialog.setCancelable(true) + val dialog = createThemedDialog(R.layout.dialog_custom_update)Also applies to: 206-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt` around lines 148 - 170, `showUpdatePrompt` duplicates the same themed dialog setup used in `showInstallPermissionPrompt`, including dialog creation, layout inflation, view lookup, and dismiss wiring. Extract the shared setup into a small helper (for example, a `createThemedDialog(...)` method in `AppUpdater`) that returns the configured `Dialog`, then reuse it in both prompt methods and keep only the prompt-specific text and click actions there.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@android/app/src/main/java/dev/openstream/app/update/AppUpdater.kt`:
- Around line 148-170: `showUpdatePrompt` duplicates the same themed dialog
setup used in `showInstallPermissionPrompt`, including dialog creation, layout
inflation, view lookup, and dismiss wiring. Extract the shared setup into a
small helper (for example, a `createThemedDialog(...)` method in `AppUpdater`)
that returns the configured `Dialog`, then reuse it in both prompt methods and
keep only the prompt-specific text and click actions there.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3afb353-fdf0-450a-97e2-eb2eb7850036
📒 Files selected for processing (2)
android/app/src/main/java/dev/openstream/app/MainActivity.ktandroid/app/src/main/java/dev/openstream/app/update/AppUpdater.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- android/app/src/main/java/dev/openstream/app/MainActivity.kt
|
@codex Please review the audio encoder restoration and update dialog layout fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0e69013da
ℹ️ 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".
| @@ -720,7 +738,6 @@ class MainActivity : Activity() { | |||
| ): Boolean { | |||
| val currentReservation = reservedBy | |||
| if (phoneConnected && currentReservation != sourceInstanceId) return false | |||
There was a problem hiding this comment.
When an OBS slot is reserved but temporarily disconnected during the 45-second reconnect hold, phoneConnected is false. With this guard now only rejecting different sources while connected, any /reserve request from another slot through CameraControlServer.handleReserve can overwrite reservedBy and cancel the existing hold even though the phone is still advertising itself as busy/reserved. Keep blocking non-matching currentReservation here unless the request is an explicit local slot switch.
Useful? React with 👍 / 👎.
| btnSave.setOnClickListener { saveSettings() } | ||
| btnBack.setOnClickListener { finish() } | ||
| btnCheckUpdates.setOnClickListener { | ||
| appUpdater.checkForUpdates(showAlreadyCurrent = true) |
There was a problem hiding this comment.
When a user checks for updates from this Settings screen and accepts the install prompt, the download id is stored only in this SettingsActivity's AppUpdater. If they press Back/Save or the activity is otherwise destroyed before DownloadManager sends completion, onDestroy unregisters the receiver and MainActivity's separate updater does not know the pending download, so the APK is downloaded but the in-app install flow never resumes. Use a shared/longer-lived updater or persist the pending download id.
Useful? React with 👍 / 👎.
| if (manualContainer.visibility == View.VISIBLE) "ADV ON" else "ADV" | ||
| btnSettings.setOnClickListener { | ||
| val intent = Intent(this, SettingsActivity::class.java) | ||
| startActivity(intent) |
There was a problem hiding this comment.
Tapping Config during an active OBS feed launches a full-screen SettingsActivity; when MainActivity is covered, its onStop() calls stopPhoneServer(), which disconnects the SRT stream and clears the reservation. Because this control remains visible while live and replaces the old in-place advanced panel, a normal config or update-check action unexpectedly drops the current feed.
Useful? React with 👍 / 👎.
1 participant
@codex Please review this Neo-Brutalist UI redesign.
Summary by CodeRabbit