Show trains in border area in grey#135
Conversation
📝 WalkthroughWalkthroughTrainMarker component now conditionally applies an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/map/components/Markers/TrainMarker.tsx (1)
44-49: Optional: simplify with a ternary template string.The array + spread +
join(" ")works, but a direct ternary is a little more idiomatic here and avoids the intermediate array allocation on every render.♻️ Suggested simplification
- const borderAreaClass = train.TrainData.InBorderStationArea ? ["in-border-area"] : []; const icon = L.icon({ iconUrl: train.TrainData.ControlledBySteamID && avatar ? avatar : botIcon, iconSize: [24, 24], popupAnchor: [0, -12], - className: ["steam-avatar", ...borderAreaClass].join(" "), + className: `steam-avatar${train.TrainData.InBorderStationArea ? " in-border-area" : ""}`, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/map/components/Markers/TrainMarker.tsx` around lines 44 - 49, The current construction builds borderAreaClass array and then spreads/join to form the className for the L.icon, which is unnecessary; replace the intermediate borderAreaClass variable and the ["steam-avatar", ...borderAreaClass].join(" ") usage with a single ternary/template expression so className becomes something like `"steam-avatar" + (train.TrainData.InBorderStationArea ? " in-border-area" : "")`; update the L.icon call (symbol: icon) to use that inline ternary and remove the borderAreaClass variable to simplify rendering while keeping the same conditional inclusion, and retain the existing selection of iconUrl (train.TrainData.ControlledBySteamID && avatar ? avatar : botIcon).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/map/components/Markers/TrainMarker.tsx`:
- Around line 44-49: The current construction builds borderAreaClass array and
then spreads/join to form the className for the L.icon, which is unnecessary;
replace the intermediate borderAreaClass variable and the ["steam-avatar",
...borderAreaClass].join(" ") usage with a single ternary/template expression so
className becomes something like `"steam-avatar" +
(train.TrainData.InBorderStationArea ? " in-border-area" : "")`; update the
L.icon call (symbol: icon) to use that inline ternary and remove the
borderAreaClass variable to simplify rendering while keeping the same
conditional inclusion, and retain the existing selection of iconUrl
(train.TrainData.ControlledBySteamID && avatar ? avatar : botIcon).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b93da94-eebb-4cfb-8a22-474dd15f624a
📒 Files selected for processing (2)
packages/map/components/Markers/TrainMarker.tsxpackages/map/styles/globals.css
In order to make it easier to distinguish which trains can be driven and which are in the border area, show the avatar of trains in border area in grey colour.
Summary by CodeRabbit