Skip to content

Conversation

@DTTerastar
Copy link
Collaborator

@DTTerastar DTTerastar commented Dec 16, 2025

Summary by CodeRabbit

  • UI Improvements

    • Anchored devices now display with a distinctive marker shape and anchor indicator for easier visual identification during calibration management.
  • Code Quality

    • Code formatting and template rendering refinements improve maintainability and readability without affecting existing functionality.

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

@DTTerastar DTTerastar temporarily deployed to CI - release environment December 16, 2025 17:58 — with GitHub Actions Inactive
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 16, 2025

📝 Walkthrough

Walkthrough

Two Svelte UI components receive updates: DeviceCalibrationManager undergoes formatting improvements including null-safety enhancements for the isAnchored check, while DeviceMarker introduces conditional rendering to display different marker shapes based on the anchored state.

Changes

Cohort / File(s) Change summary
DeviceCalibrationManager refactoring
src/ui/src/lib/DeviceCalibrationManager.svelte
Minor formatting adjustments to filtering logic and template rendering. filteredDevices expression reformatted, arrow function parameter styles updated, and isAnchored check now handles undefined via nullish coalescing. DataTable props restructured to inline style; "No active devices found" message placed on single line. Semantics unchanged.
DeviceMarker conditional rendering
src/ui/src/lib/DeviceMarker.svelte
Replaced single circle rendering with conditional logic: when d.isAnchored is truthy, renders anchored marker path plus small white circle indicator; otherwise renders original circle. Path preserves fill, stroke, stroke-width, and event handlers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify nullish coalescing operator for undefined isAnchored in displayRoomFloor
  • Confirm both rendering branches in DeviceMarker handle edge cases appropriately (e.g., when isAnchored transitions or is initially undefined)
  • Ensure reformatted DataTable props are valid and equivalent to previous structure

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: improving the robustness of the isAnchored check in room floor display, which aligns with the nullish coalescing operator addition in DeviceCalibrationManager.svelte and conditional rendering in DeviceMarker.svelte.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex-anchor

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

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

@DTTerastar DTTerastar temporarily deployed to CI - release environment December 16, 2025 18:00 — with GitHub Actions Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/ui/src/lib/DeviceMarker.svelte (1)

47-48: Consider extracting hardcoded offsets as named constants.

The marker positioning uses magic numbers (12, 24, 15) that would be clearer as named constants at the top of the component. This improves readability and makes adjustments easier.

For example, at the top of the script section:

+	const MARKER_OFFSET_X = 12;
+	const MARKER_OFFSET_Y = 24;
+	const INDICATOR_OFFSET_Y = 15;
+
 	const r = spring(5, { stiffness: 0.15, damping: 0.3 });

Then update the rendering:

-			<path role="none" d="M12 2C8.13 2 5 5.13 5 9c0 5.25 7 13 7 13s7-7.75 7-13c0-3.87-3.13-7-7-7z" transform="translate({$xScale($x) - 12}, {$yScale($y) - 24})" fill={$c} stroke={d.id == hovered ? 'black' : 'white'} stroke-width={$s} onmouseover={() => hover(d)} onfocus={() => select(d)} onmouseout={() => hover(null)} onblur={() => unselect()} />
-			<circle cx={$xScale($x)} cy={$yScale($y) - 15} r={3} fill="white" pointer-events="none" />
+			<path role="none" d="M12 2C8.13 2 5 5.13 5 9c0 5.25 7 13 7 13s7-7.75 7-13c0-3.87-3.13-7-7-7z" transform="translate({$xScale($x) - MARKER_OFFSET_X}, {$yScale($y) - MARKER_OFFSET_Y})" fill={$c} stroke={d.id == hovered ? 'black' : 'white'} stroke-width={$s} onmouseover={() => hover(d)} onfocus={() => select(d)} onmouseout={() => hover(null)} onblur={() => unselect()} />
+			<circle cx={$xScale($x)} cy={$yScale($y) - INDICATOR_OFFSET_Y} r={3} fill="white" pointer-events="none" />
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1185b3 and 73ae1c2.

📒 Files selected for processing (2)
  • src/ui/src/lib/DeviceCalibrationManager.svelte (3 hunks)
  • src/ui/src/lib/DeviceMarker.svelte (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Place backend C# ASP.NET Core code under src/ (controllers, services, models, utils)

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
src/ui/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Place the SvelteKit frontend under src/ui/ (routes, lib, tests)

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
src/ui/src/**

📄 CodeRabbit inference engine (AGENTS.md)

Place frontend unit tests under src/ui/src

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
src/ui/**/*.{svelte,ts,js}

📄 CodeRabbit inference engine (AGENTS.md)

src/ui/**/*.{svelte,ts,js}: UI (Svelte/TS/JS): Use tabs with an indent size of 2
Keep UI files linted and formatted with ESLint and Prettier before PRs

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
src/ui/src/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Svelte components should use PascalCase filenames (e.g., Map.svelte)

src/ui/src/**/*.svelte: In Svelte 5 components, use camelCase DOM event props (e.g., onclick, onchange, onmouseover)
For custom interactions, use function props instead of custom events (e.g., onselected, onhovered, onclose)
Prefer passing functions as props for component communication rather than dispatching custom events

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
src/ui/src/lib/**

📄 CodeRabbit inference engine (AGENTS.md)

Place frontend stores and utilities under src/ui/src/lib

Files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
  • src/ui/src/lib/DeviceMarker.svelte
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: ESPresense/ESPresense-companion PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T00:51:01.127Z
Learning: Applies to src/ui/src/routes/calibration/**/*.svelte : Calibration UI should determine "Calibrated" status only from configured values (Device.ConfiguredRefRssi), not measured values
📚 Learning: 2025-09-16T00:51:01.127Z
Learnt from: CR
Repo: ESPresense/ESPresense-companion PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T00:51:01.127Z
Learning: Applies to src/ui/src/routes/calibration/**/*.svelte : Calibration UI should determine "Calibrated" status only from configured values (Device.ConfiguredRefRssi), not measured values

Applied to files:

  • src/ui/src/lib/DeviceCalibrationManager.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Deploy to Docker add-on with tag
  • GitHub Check: build
  • GitHub Check: Analyze (csharp)
🔇 Additional comments (3)
src/ui/src/lib/DeviceMarker.svelte (1)

46-46: LGTM! Null-safety check is robust.

The nullish coalescing operator ?? false properly handles undefined and null values for isAnchored, ensuring the conditional rendering never fails. This aligns well with the PR objective.

src/ui/src/lib/DeviceCalibrationManager.svelte (2)

42-42: LGTM! Consistent null-safety improvement.

The nullish coalescing operator ?? false matches the pattern used in DeviceMarker.svelte, ensuring consistent and robust handling of undefined or null values for isAnchored across components.


11-17: LGTM! Formatting improvements enhance readability.

The reformatted filtering logic, arrow function parentheses, and inline DataTable props improve code consistency without changing behavior. These changes align with the coding guidelines.

Also applies to: 21-22, 140-140, 142-142

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants