Refactor(apps): 현장 상황 뷰 혼잡도 이미지 교체#342
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughLiveButton에 혼잡도(congestion) 표시가 추가되었습니다. Chip 컴포넌트에 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 Tip You can customize the high-level summary generated by CodeRabbit.Configure the |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compositions/src/live-button-container/live-button-container.tsx (1)
12-12: 🧹 Nitpick | 🔵 TrivialSuggest:
congestionLevel타입을 더 엄격하게 정의하면 좋겠어요.현재
congestionLevel: string으로 정의되어 있지만, 실제로 허용되는 값은'SMOOTH' | 'NORMAL' | 'CROWDED' | 'NONE'이에요.StatusSheetValue타입을 재사용하면 타입 안전성이 높아지고, Line 36의as StatusSheetValue캐스팅도 불필요해져요.♻️ 타입 개선 제안
interface LiveButtonContainerProps { items: { stageId: number; title: string; location?: string | null; - congestionLevel: string; + congestionLevel: StatusSheetValue; }[]; showIcon?: boolean; isDisabled?: boolean; onClick?: (id: number) => void; }Line 36도 캐스팅 없이 사용할 수 있어요:
- const statusImageUrl = - STATUS_IMAGES[item.congestionLevel as StatusSheetValue]; + const statusImageUrl = STATUS_IMAGES[item.congestionLevel];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/compositions/src/live-button-container/live-button-container.tsx` at line 12, Change the congestionLevel prop from a plain string to the stricter union type by using the existing StatusSheetValue type (replace "congestionLevel: string" with "congestionLevel: StatusSheetValue"), then remove the unnecessary "as StatusSheetValue" cast where congestionLevel is used (the cast near the LiveButtonContainer usage). This reuses StatusSheetValue for type safety and eliminates the manual casting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ads-ui/src/components/button/live-button/live-button.tsx`:
- Around line 8-9: Extract the shared union type into a common exported type and
reuse it instead of duplicating: create a new exported type (e.g., export type
CongestionLevel = 'SMOOTH' | 'NORMAL' | 'CROWDED' | 'NONE' or export type
StatusSheetValue = ...) in a shared types/constants module, update
live-button.tsx to import and use that exported type for CongestionLevel and
derive CongestionChipStatus from it (or export both from the shared module), and
update the StatusSheetValue consumer to import the same shared type so all
components (including functions/components referencing CongestionLevel,
CongestionChipStatus, and StatusSheetValue) use the single source of truth.
- Around line 11-19: Replace the LiveButtonProps type alias with an interface:
convert "type LiveButtonProps = { ... }" to "interface LiveButtonProps { ... }"
preserving all property names, optional markers (subText?, showIcon?,
congestionLevel?), types (string, boolean, () => void) and the onClick
signature; update any local references if necessary (the identifier
LiveButtonProps remains the same so no runtime changes) to comply with the
coding guideline preferring interface for object shapes used by the LiveButton
component.
---
Outside diff comments:
In `@packages/compositions/src/live-button-container/live-button-container.tsx`:
- Line 12: Change the congestionLevel prop from a plain string to the stricter
union type by using the existing StatusSheetValue type (replace
"congestionLevel: string" with "congestionLevel: StatusSheetValue"), then remove
the unnecessary "as StatusSheetValue" cast where congestionLevel is used (the
cast near the LiveButtonContainer usage). This reuses StatusSheetValue for type
safety and eliminates the manual casting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e605ca67-eaf3-4cca-af59-333bbf44170e
⛔ Files ignored due to path filters (7)
packages/ads-ui/src/assets/congestion-crowded-btn.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/ads-ui/src/assets/congestion-normal-btn.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/ads-ui/src/assets/congestion-smooth-btn.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/compositions/src/assets/crowded.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/compositions/src/assets/none.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/compositions/src/assets/normal.webpis excluded by!**/*.webpand included bypackages/**/src/**packages/compositions/src/assets/smooth.webpis excluded by!**/*.webpand included bypackages/**/src/**
📒 Files selected for processing (6)
packages/ads-ui/src/components/button/live-button/live-button.css.tspackages/ads-ui/src/components/button/live-button/live-button.tsxpackages/ads-ui/src/components/chip/chip.css.tspackages/ads-ui/src/components/chip/chip.tsxpackages/ads-ui/src/styles/tokens/color.tspackages/compositions/src/live-button-container/live-button-container.tsx
| type CongestionLevel = 'SMOOTH' | 'NORMAL' | 'CROWDED' | 'NONE'; | ||
| type CongestionChipStatus = 'smooth' | 'normal' | 'crowded' | 'none'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest: CongestionLevel 타입을 공용으로 추출하면 좋겠어요.
CongestionLevel과 CongestionChipStatus 타입이 live-button.tsx 내부에만 정의되어 있는데, StatusSheetValue(packages/ads-ui/src/components/bottom-sheet/status-sheet/status-sheet.constants.ts)와 동일한 값을 가지고 있어요. 타입 일관성을 위해 공용 타입을 재사용하거나, 이 타입들을 별도 파일로 추출하는 것을 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ads-ui/src/components/button/live-button/live-button.tsx` around
lines 8 - 9, Extract the shared union type into a common exported type and reuse
it instead of duplicating: create a new exported type (e.g., export type
CongestionLevel = 'SMOOTH' | 'NORMAL' | 'CROWDED' | 'NONE' or export type
StatusSheetValue = ...) in a shared types/constants module, update
live-button.tsx to import and use that exported type for CongestionLevel and
derive CongestionChipStatus from it (or export both from the shared module), and
update the StatusSheetValue consumer to import the same shared type so all
components (including functions/components referencing CongestionLevel,
CongestionChipStatus, and StatusSheetValue) use the single source of truth.
packages/ads-ui/src/components/button/live-button/live-button.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ads-ui/src/components/button/live-button/live-button.tsx`:
- Around line 35-46: The function toCongestionLevel currently returns
CongestionLevel | null but should be a TypeScript type guard for clearer
inference — change its signature to a type guard (e.g., function
toCongestionLevel(value?: string): value is CongestionLevel) and keep the same
string checks ('SMOOTH' | 'NORMAL' | 'CROWDED' | 'NONE') returning true for
matches and false otherwise; update all call sites that rely on its return to
use the guard (no null checks needed). If you prefer to keep the current
runtime-returning behaviour instead of a guard, simply rename the function to
parseCongestionLevel to match verb+noun naming (toCongestionLevel ->
parseCongestionLevel) and leave logic as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c822a7bf-8f0e-404b-8250-2e13f059c85f
📒 Files selected for processing (1)
packages/ads-ui/src/components/button/live-button/live-button.tsx
packages/ads-ui/src/components/button/live-button/live-button.tsx
Outdated
Show resolved
Hide resolved
jin-evergreen
left a comment
There was a problem hiding this comment.
다 잘 보이는 것 같아요! 고생 많으셨습니다~!!👍👍
📌 Summary
📚 Tasks
🔍 Describe
혼잡도 칩 분리
혼잡도 이미지 변경
👀 To Reviewer
📸 Screenshot