Handles legacy camera size (strings)

[p-delorme]

Fix the issue when upgrading from v0.3.38 to v0.4.x where the camera size has been modified from a string to integer numbers.

Summary by CodeRabbit

• Bug Fixes
  • Camera preview now correctly interprets legacy size strings (e.g., "lg") and normalizes them to numeric sizes, applies clamping to min/max values, and preserves previously saved camera size settings across all preview modes for seamless backward compatibility.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds normalization for legacy string-based camera size values in two preview components, converting strings like "lg" to numeric presets and updating state before sizing/clamping logic.

Changes

Cohort / File(s)	Summary
Legacy size normalization for camera preview components apps/desktop/src/routes/camera.tsx	Added normalization logic in NativeCameraPreviewPage and LegacyCameraPreviewPage to convert string size values (e.g., "lg") to numeric equivalents (CAMERA_PRESET_LARGE or CAMERA_DEFAULT_SIZE) via createEffect, and use the normalized number for state, clamping, and comparisons.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Review createEffect logic in both components for correct mapping and state updates.
• Verify CAMERA_PRESET_LARGE, CAMERA_DEFAULT_SIZE, and clamping bounds (CAMERA_MIN_SIZE, CAMERA_MAX_SIZE) are the intended values for legacy inputs.
• Check for potential re-render loops or stale state dependencies in the effects.

Poem

🐰
Strings once lost in frame and light,
I hop and turn them true and bright.
"lg" to large, numbers play,
Old sizes find a modern way.
Click—compatibility in flight 📸

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Handles legacy camera size (strings)' directly and clearly summarizes the main change: adding support for legacy string-based camera size values.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 21f197e and 0e16f7c.

📒 Files selected for processing (1)

• apps/desktop/src/routes/camera.tsx (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• apps/desktop/src/routes/camera.tsx

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/desktop/src/routes/camera.tsx (1)

297-306: Migration logic is correct; consider adding clamping for consistency.

The string-to-number normalization works correctly. However, unlike NativeCameraPreviewPage (lines 108-114), this effect doesn't clamp out-of-bounds numeric values in the state. While clamping happens downstream (line 522 in windowSize resource, line 492 in handleResizeMove), proactively clamping the state would improve consistency between the two preview implementations.

Apply this diff to add clamping:

 createEffect(() => {
   // Support for legacy size strings.
   const currentSize = state.size as number | string;
   if (typeof currentSize !== "number" || Number.isNaN(currentSize)) {
     setState(
       "size",
       currentSize === "lg" ? CAMERA_PRESET_LARGE : CAMERA_DEFAULT_SIZE,
     );
+    return;
   }
+
+  const clampedSize = Math.max(
+    CAMERA_MIN_SIZE,
+    Math.min(CAMERA_MAX_SIZE, currentSize),
+  );
+  if (clampedSize !== currentSize) {
+    setState("size", clampedSize);
+  }
 });

Optional: Both normalization blocks are nearly identical. Consider extracting a shared helper function to reduce duplication:

function normalizeCameraSize(size: number | string): number {
  if (typeof size !== "number" || Number.isNaN(size)) {
    return size === "lg" ? CAMERA_PRESET_LARGE : CAMERA_DEFAULT_SIZE;
  }
  return Math.max(CAMERA_MIN_SIZE, Math.min(CAMERA_MAX_SIZE, size));
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a292974 and 21f197e.

📒 Files selected for processing (1)

• apps/desktop/src/routes/camera.tsx (2 hunks)

🔇 Additional comments (1)

apps/desktop/src/routes/camera.tsx (1)

98-116: I'm unable to access the repository directly to verify the claims in the review comment. However, I can analyze what's provided and rewrite it into a proper, complete review format.

Based on the code snippet and context provided, the review comment itself is incomplete and has structural issues that need to be addressed:

The review comment as provided is a malformed work-in-progress that mixes scratchpad notes with incomplete verification.

The original review contains:

1. An embedded scratchpad section (which should be hidden)
2. Incomplete analysis with unresolved [pending] items
3. An unfinished verification task requesting shell script execution
4. No clear conclusion or actionable feedback for the developer

Here is the rewritten review comment:

---

Migration logic correctly handles legacy string-to-number conversion with appropriate flow control.

The implementation handles the string-to-number migration properly:

• The early return on line 105 (after state normalization) prevents the clamping logic from running with the string value
• The effect re-runs after setState, allowing clamping to apply on the second pass with the normalized number
• The type casting for runtime migration is appropriate

However, verify that "lg" is the only legacy size string that needs special handling. If v0.3.38 had other size strings (e.g., "sm", "md", "xl"), they would silently default to CAMERA_DEFAULT_SIZE (230) rather than being explicitly handled.

---