fix: only allow alpha channel toggle when toggle is supported #16
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the UI and settings logic so the “Capture Alpha Channel” control is only interactive when the selected video codec actually supports toggling alpha.
Changes:
- Add codec capability flags (
alwaysHasAlpha,canToggleAlpha) and updatevideoCodecsetter to enforce alpha behavior. - Make alpha toggle always visible in both Settings and menu bar UI, but disabled when toggling isn’t supported.
- Improve contextual help text and disabled styling for the toggle.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| BetterCapture/View/SettingsView.swift | Adds codec-specific help text and disables alpha toggle when it can’t be changed. |
| BetterCapture/View/MenuBarSettingsView.swift | Makes menu bar alpha toggle always visible, adds disabled styling/behavior. |
| BetterCapture/Model/SettingsStore.swift | Introduces capability helpers and enforces alpha defaults on codec changes. |
Comments suppressed due to low confidence (1)
BetterCapture/Model/SettingsStore.swift:1
- This enforces the alpha invariant only when
videoCodecis set. If persisted settings (or any other code path) can producevideoCodec == .proRes4444whilecaptureAlphaChannel == false, the UI is disabled (cannot fix it) and state may remain inconsistent. Consider enforcing the invariant in initialization/loading as well (e.g., normalize after reading persisted values), or make the effective alpha behavior derived fromvideoCodecforalwaysHasAlphacodecs so it can’t drift.
//
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct MenuBarToggle: View { | ||
| let name: String | ||
| @Binding var isOn: Bool | ||
| var isDisabled: Bool = false |
There was a problem hiding this comment.
isDisabled is view input state and doesn’t appear to be mutated; prefer let isDisabled: Bool = false to better communicate immutability and avoid accidental mutation in a value-type View.
| var isDisabled: Bool = false | |
| let isDisabled: Bool = false |
| Text(name) | ||
| .font(.system(size: 13, weight: .medium)) | ||
| .foregroundStyle(.primary) | ||
| .foregroundStyle(isDisabled ? .secondary : .primary) | ||
| Spacer() | ||
| Toggle("", isOn: $isOn) | ||
| .toggleStyle(.switch) | ||
| .tint(.blue) | ||
| .scaleEffect(0.8) | ||
| .disabled(isDisabled) |
There was a problem hiding this comment.
The Toggle has an empty label (\"\"), which can lead to an unlabeled control for VoiceOver. Consider giving the toggle an accessibility label (e.g., .accessibilityLabel(Text(name))) or restructuring to use Toggle(isOn:) { Text(name) } and then style/layout accordingly.
| return "ProRes 4444 always includes alpha channel support" | ||
| case .hevc: | ||
| return "Enable transparency support for HEVC" | ||
| case .h264, .proRes422: | ||
| return "Alpha channel not supported by this codec" |
There was a problem hiding this comment.
Minor grammar tweaks would make these help strings read more naturally (e.g., add an article and/or verb): "ProRes 4444 always includes an alpha channel" and "Alpha channel is not supported by this codec."
| return "ProRes 4444 always includes alpha channel support" | |
| case .hevc: | |
| return "Enable transparency support for HEVC" | |
| case .h264, .proRes422: | |
| return "Alpha channel not supported by this codec" | |
| return "ProRes 4444 always includes an alpha channel." | |
| case .hevc: | |
| return "Enable transparency support for HEVC." | |
| case .h264, .proRes422: | |
| return "The alpha channel is not supported by this codec." |
1 participant
No description provided.