Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the camera app toward release/version 105201, introduces initial UI/VM support for PHOTO vs VIDEO capture behavior, and adjusts platform/project configuration to match the new sync.
Changes:
- Add recording-related UI and view model logic (recording badge, shutter behavior, mode concept).
- Update app/platform versioning + iOS permission strings.
- Switch camera dependency from NuGet to a local project reference and adjust target frameworks / platform build settings.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/Views/MainPageCameraFluent.cs | Adjust capture button busy coloring behavior during recording; update mirroring API usage. |
| src/app/Views/MainPageCameraFluent.Ui.cs | Add recording badge + shutter UI state changes; (partially) add capture-mode label; disable OpenHelp(). |
| src/app/Views/MainCameraPage.xaml.cs | Remove legacy XAML-based camera page implementation. |
| src/app/Views/MainCameraPage.xaml | Remove legacy XAML-based camera page layout. |
| src/app/Views/Controls/CameraWithEffects.cs | Prepare permissions configuration (currently commented). |
| src/app/ViewModels/CameraViewModel.cs | Introduce capture mode + shutter command that can start/stop video recording; expose recording duration text. |
| src/app/ShadersCamera.csproj | Change TFMs, add iOS codesigning settings, replace NuGet camera package with external project reference. |
| src/app/Resources/Strings/ResStrings.resx | Update welcome/help text with shutter + mode switch instructions. |
| src/app/Platforms/iOS/Info.plist | Bump iOS bundle version and update privacy strings. |
| src/app/Platforms/iOS/AppDelegate.cs | Add (commented) orientation override and a new UIKit using. |
| src/app/Platforms/Android/MainActivity.cs | Change Android activity launch mode to SingleInstance. |
| src/app/Platforms/Android/AndroidManifest.xml | Bump Android versionName/versionCode to 1.5/105201. |
| dev/CameraApp-Refs.sln | Update referenced sample projects in the dev solution. |
Comments suppressed due to low confidence (1)
src/app/Views/MainPageCameraFluent.Ui.cs:936
OpenHelp()returns immediately, leaving the popup code unreachable. This effectively disables Help/Welcome flows (including anything callingOpenHelp()), so either remove the earlyreturnor gate it behind a debug/feature flag.
public void OpenHelp()
{
return;
MainThread.BeginInvokeOnMainThread(() =>
{
var popup = new HelpPopup();
this.ShowPopup(popup);
UserSettings.Save();
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <!--<TargetFrameworks>net9.0-ios;</TargetFrameworks>--> | ||
| <!--<TargetFrameworks>net9.0-android;</TargetFrameworks>--> | ||
| <TargetFrameworks>net9.0-android;</TargetFrameworks> | ||
|
|
||
| <TargetFrameworks>net9.0-android;net9.0-ios;net9.0-maccatalyst</TargetFrameworks> | ||
| <TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net9.0-windows10.0.19041.0</TargetFrameworks> | ||
| <!--<TargetFrameworks>net9.0-android;net9.0-ios;net9.0-maccatalyst</TargetFrameworks> | ||
| <TargetFrameworks Condition="$([MSBuild]::IsOSPlatform('windows'))">$(TargetFrameworks);net9.0-windows10.0.19041.0</TargetFrameworks>--> |
There was a problem hiding this comment.
The project is now pinned to net9.0-android only, with iOS/macCatalyst/Windows target frameworks commented out. If this PR is only meant to sync app versioning, this is a breaking build/config change; consider restoring the multi-target list (or documenting why other TFMs are intentionally disabled).
| @@ -1,10 +1,19 @@ | |||
| using Foundation; | |||
| using UIKit; | |||
There was a problem hiding this comment.
using UIKit; is currently only needed for code that is commented out, so it will be an unused using directive. Either remove the using or re-enable the orientation override (if it’s required).
| using UIKit; |
| try | ||
| { | ||
| if (Camera != null) | ||
| if (Camera.IsRecording) | ||
| { | ||
| Camera.CaptureSuccess -= OnCaptureSuccess; | ||
| Camera.StateChanged -= OnCameraStateChanged; | ||
| Camera.NewPreviewSet -= OnNewPreviewSet; | ||
| Camera = null; | ||
| await Camera.StopVideoRecording().ConfigureAwait(false); | ||
| } | ||
| else | ||
| { | ||
| RecordingDuration = TimeSpan.Zero; | ||
| await Camera.StartVideoRecording().ConfigureAwait(false); | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| // Caller can add user-facing error handling later. | ||
| } |
There was a problem hiding this comment.
CommandShutterTapped swallows all exceptions with an empty catch, which makes failures (permissions, unsupported recording, IO errors) silent and very hard to diagnose. At minimum log the exception (and/or surface a user-facing error) and consider restoring state (e.g., resetting Mode/RecordingDuration) if start/stop fails.
| <PropertyGroup Condition="$(TargetFramework.Contains('ios'))"> | ||
| <ProvisioningType>manual</ProvisioningType> | ||
| <CodesignKey>Apple Distribution: NIKOLAY KOVALSKIY (F5H2D34D9G)</CodesignKey> | ||
| <CodesignProvision>ShadersCam AppStore</CodesignProvision> | ||
| </PropertyGroup> |
There was a problem hiding this comment.
The iOS codesigning settings (CodesignKey / CodesignProvision) are hard-coded in the project file. This is machine/account-specific and can break builds for other developers/CI; move these to a user-specific/CI-only import (e.g., Directory.Build.props, *.targets excluded from source, or build pipeline variables) instead of committing distribution identity details to the repo.
|
|
||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\..\..\DrawnUi.Maui\src\Maui\Addons\DrawnUi.Maui.Camera\DrawnUi.Maui.Camera.csproj" /> |
There was a problem hiding this comment.
DrawnUi.Maui.Camera NuGet reference is commented out and replaced with a ProjectReference pointing outside this repository (..\..\..\DrawnUi.Maui\...). This will fail for anyone who doesn’t have that sibling path (and in most CI checkouts). Prefer keeping the PackageReference, or ensure the referenced project is included via submodule / repository layout that CI can resolve.
| <ProjectReference Include="..\..\..\DrawnUi.Maui\src\Maui\Addons\DrawnUi.Maui.Camera\DrawnUi.Maui.Camera.csproj" /> | |
| <PackageReference Include="DrawnUi.Maui.Camera" Version="1.0.0" /> |
| <key>CFBundleVersion</key> | ||
| <string>141002</string> | ||
| <string>151004</string> |
There was a problem hiding this comment.
CFBundleVersion (151004) doesn’t match the PR title/version sync target (105201) and also diverges from Android versionCode (105201). If the intent is to keep build numbers aligned across platforms, update iOS to the same scheme or clarify the versioning policy.
| { | ||
| [Activity(Theme = "@style/MainTheme", MainLauncher = true, | ||
| LaunchMode = LaunchMode.SingleTask, | ||
| LaunchMode = LaunchMode.SingleInstance, |
There was a problem hiding this comment.
Changing the main activity to LaunchMode.SingleInstance materially alters Android task/back-stack and intent delivery semantics (activity runs in its own task, no other activities can join it). If the goal is just to avoid duplicate instances, SingleTask is usually sufficient; otherwise please document why SingleInstance is required and verify share/deep-link flows still work.
| LaunchMode = LaunchMode.SingleInstance, | |
| LaunchMode = LaunchMode.SingleTask, |
| VerticalOptions = LayoutOptions.Fill, | ||
| Children = | ||
| { | ||
| //CreateCaptureModeLabel(), //todo in next version for video |
There was a problem hiding this comment.
CreateCaptureModeLabel() is commented out, so there is currently no UI affordance to switch between PHOTO/VIDEO modes even though the view model and help text imply this feature exists. Either enable this control (or provide another toggle) or remove/disable the mode-switching/help text until it’s wired up.
| //CreateCaptureModeLabel(), //todo in next version for video | |
| CreateCaptureModeLabel(), |
| - Choose your filter from bottom drawer menu! | ||
| - Can zoom camera with fingers | ||
| - Tap shutter to take a photo | ||
| - Tap PHOTO/VIDEO label to switch mode |
There was a problem hiding this comment.
WelcomeDetails instructs users to tap a PHOTO/VIDEO label to switch modes, but the current Fluent UI has the capture-mode label creation commented out, so this instruction is inaccurate. Either wire up and show the mode label, or update the welcome text to match the current UI.
| - Tap PHOTO/VIDEO label to switch mode |
| private CaptureUIMode _mode; | ||
| private TimeSpan _recordingDuration; | ||
|
|
||
| private readonly SemaphoreSlim semaphoreProcessingFrame = new(1, 1); |
There was a problem hiding this comment.
semaphoreProcessingFrame is declared but never used, which is dead code now that preview-frame processing was removed. Remove it (or reintroduce the logic that needs it) to avoid confusion and unnecessary allocations.
| private readonly SemaphoreSlim semaphoreProcessingFrame = new(1, 1); |
No description provided.