fix(actions): store camera group selection per-action instead of global (#208)#209
Merged
Conversation
…al (#208) Camera group subset was stored in global settings, causing all Camera Controls instances to share the same selection. Move to per-action settings so each button maintains its own camera group list. Falls back to legacy global setting for existing users who already configured their groups. Fixes #208
📝 WalkthroughWalkthroughMigrates camera group subset storage from global settings to per-action settings, threads per-action Changes
Sequence Diagram(s)sequenceDiagram
participant PI as PI (camera-focus.ejs)
participant Settings as Per-Action Settings
participant Legacy as Global Settings
participant Action as CameraControls (action)
participant Engine as Execution / Icon Update
PI->>Legacy: read legacy `cameraGroupSubset`
PI->>Settings: useSettings() hook (per-action)
Settings-->>PI: currentGroups (seeded from Legacy if absent)
PI->>Settings: save updated `cameraGroupSubset`
Action->>Settings: read per-action `cameraGroupSubset`
Action->>Legacy: read legacy `cameraGroupSubset` (fallback)
Action->>Action: parseGroupSubset(raw) / getEnabledGroupNames(raw)
Action->>Engine: executeCycle(..., enabledGroups)
Action->>Engine: updateCycleIcon(enabledGroups)
(Note: colored rectangles not needed for this simple sequence.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream-deck-plugin/src/pi/camera-focus.ejs (1)
273-289:⚠️ Potential issue | 🟠 MajorKeep the legacy global subset visible until an action-specific value exists.
When
useSettings()returns no per-action value, this branch resetscurrentGroupsto{}and the PI rendersDEFAULT_ENABLED, butpackages/actions/src/actions/camera-controls.tsstill falls back to the legacy globalcameraGroupSubset. Existing users will see the wrong selection here, and their first checkbox change will seed/save from defaults instead of their migrated subset. Please read the global value here until a per-action subset has been saved, or migrate it into action settings on first load.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45e4ddbd-043a-424c-bfc6-ad31a7fe4a61
📒 Files selected for processing (3)
packages/actions/src/actions/camera-controls.test.tspackages/actions/src/actions/camera-controls.tspackages/stream-deck-plugin/src/pi/camera-focus.ejs
When no per-action camera group subset exists, read the legacy global setting to display the correct checkboxes. Prevents display mismatch and data loss on first edit for upgrading users.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/stream-deck-plugin/src/pi/camera-focus.ejs (2)
124-128: 🛠️ Refactor suggestion | 🟠 MajorReorder the shared PI partials.
common-settingsis still included beforecolor-overrides. This template family expectscolor-overridesbetween the action-specific controls and the global section, withcommon-settingsimmediately after it.As per coding guidelines: Property Inspector templates must include color-overrides partial with slots and defaults between action-specific settings and global sections; Property Inspector templates must include common-settings partial after color-overrides for flags overlay and future common settings.♻️ Suggested fix
- <%- include('common-settings') %> - - <%- include('color-overrides', { slots: ['backgroundColor', 'textColor', 'graphic1Color'], defaults: require('./data/color-defaults.json')['camera-focus'] }) %> + <%- include('color-overrides', { slots: ['backgroundColor', 'textColor', 'graphic1Color'], defaults: require('./data/color-defaults.json')['camera-focus'] }) %> + + <%- include('common-settings') %>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream-deck-plugin/src/pi/camera-focus.ejs` around lines 124 - 128, Reorder the PI partial includes so that the color-overrides partial is placed between the action-specific controls and the global section, and the common-settings partial is included immediately after color-overrides; specifically, move the include for "common-settings" to follow the include for "color-overrides" (which uses slots ['backgroundColor','textColor','graphic1Color'] and defaults from './data/color-defaults.json'['camera-focus']), keeping the existing "global-settings" include after them.
271-307:⚠️ Potential issue | 🟠 MajorMake the legacy fallback derive from both subscriptions.
Lines 275-306 only consult
legacyGroupsfrom inside the per-action callback. IfuseSettings()resolves beforeuseGlobalSettings(), an upgraded instance renders defaults and the first save can overwrite the legacy subset instead of preserving it. Assigningparsed.groupsdirectly also makes empty/sparse stored subsets render as default or partial checkboxes, even thoughpackages/actions/src/actions/camera-controls.tstreats them as explicit subsets.💡 Suggested fix
- // Set up per-action settings for camera groups, with legacy global fallback - var legacyGroups = null; + // Set up per-action settings for camera groups, with legacy global fallback + var actionGroups; + var legacyGroups; + + function parseStoredGroups(value) { + if (!value) return undefined; + try { + var parsed = JSON.parse(value); + if (!parsed || !parsed.groups || typeof parsed.groups !== "object") { + return undefined; + } + + var normalized = {}; + for (var i = 0; i < ALL_GROUPS.length; i++) { + var name = ALL_GROUPS[i]; + normalized[name] = parsed.groups[name] === true; + } + return normalized; + } catch (e) { + return undefined; + } + } + + function syncCurrentGroups() { + if (actionGroups !== undefined) { + currentGroups = Object.assign({}, actionGroups); + } else if (legacyGroups !== undefined) { + currentGroups = Object.assign({}, legacyGroups); + } else { + currentGroups = {}; + } + renderCheckboxes(); + } if (window.SDPIComponents) { // Read legacy global setting (for users upgrading from shared selection) window.SDPIComponents.useGlobalSettings(SETTING_KEY, function (value) { - if (value) { - try { - var parsed = JSON.parse(value); - if (parsed && parsed.groups) { - legacyGroups = parsed.groups; - } - } catch (e) { - // ignore - } - } + legacyGroups = parseStoredGroups(value); + syncCurrentGroups(); }); var subsetHook = window.SDPIComponents.useSettings(SETTING_KEY, function (value) { - if (value) { - try { - var parsed = JSON.parse(value); - if (parsed && parsed.groups) { - currentGroups = parsed.groups; - } else { - currentGroups = {}; - } - } catch (e) { - currentGroups = {}; - } - } else if (legacyGroups) { - // No per-action value yet — seed from legacy global setting - currentGroups = legacyGroups; - } else { - currentGroups = {}; - } - renderCheckboxes(); + actionGroups = parseStoredGroups(value); + syncCurrentGroups(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream-deck-plugin/src/pi/camera-focus.ejs` around lines 271 - 307, The per-action settings subscription (window.SDPIComponents.useSettings with SETTING_KEY) currently only falls back to legacyGroups when its callback runs, which can lose the legacy subset if useSettings fires before useGlobalSettings; also assigning parsed.groups directly treats empty/sparse subsets as undefined. Fix by making both subscriptions populate a shared canonical value: in the useGlobalSettings callback set legacyGroups to a defensive clone of parsed.groups (or {} if parsed has an explicit empty subset), and in the useSettings callback merge/choose between the explicit per-action parsed.groups (including empty maps as valid) and legacyGroups if per-action is absent; update currentGroups accordingly and then call renderCheckboxes(); reference the legacyGroups variable, window.SDPIComponents.useGlobalSettings, window.SDPIComponents.useSettings, currentGroups, and renderCheckboxes to locate the code to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/stream-deck-plugin/src/pi/camera-focus.ejs`:
- Around line 124-128: Reorder the PI partial includes so that the
color-overrides partial is placed between the action-specific controls and the
global section, and the common-settings partial is included immediately after
color-overrides; specifically, move the include for "common-settings" to follow
the include for "color-overrides" (which uses slots
['backgroundColor','textColor','graphic1Color'] and defaults from
'./data/color-defaults.json'['camera-focus']), keeping the existing
"global-settings" include after them.
- Around line 271-307: The per-action settings subscription
(window.SDPIComponents.useSettings with SETTING_KEY) currently only falls back
to legacyGroups when its callback runs, which can lose the legacy subset if
useSettings fires before useGlobalSettings; also assigning parsed.groups
directly treats empty/sparse subsets as undefined. Fix by making both
subscriptions populate a shared canonical value: in the useGlobalSettings
callback set legacyGroups to a defensive clone of parsed.groups (or {} if parsed
has an explicit empty subset), and in the useSettings callback merge/choose
between the explicit per-action parsed.groups (including empty maps as valid)
and legacyGroups if per-action is absent; update currentGroups accordingly and
then call renderCheckboxes(); reference the legacyGroups variable,
window.SDPIComponents.useGlobalSettings, window.SDPIComponents.useSettings,
currentGroups, and renderCheckboxes to locate the code to change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9630741a-7795-445e-b3d9-4b5e7a81571f
📒 Files selected for processing (1)
packages/stream-deck-plugin/src/pi/camera-focus.ejs
This was referenced Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Fixes #208
What changed?
Camera group subset selection was stored in global settings (
cameraGroupSubset), causing all Camera Controls "Cycle Camera" instances to share the same camera group checkboxes. This PR moves the setting to per-action storage so each button maintains its own selection.cameraGroupSubsetto the per-action settings Zod schemaparseGroupSubset()for reusable parsing logicgetEnabledGroupNames()to read from per-action settings first, falling back to the legacy global setting for existing usersuseGlobalSettingstouseSettingsGLOBAL_KEY→SETTING_KEYin PI andCAMERA_GROUPS_GLOBAL_KEY→CAMERA_GROUPS_SETTING_KEYin action codeHow to test
cameraGroupSubsetshould see their selection preserved until they make a per-button changeChecklist
Summary by CodeRabbit