Skip to content

♻️Refactor : 행사 날짜 - 당일로 포맷팅 #143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
KongMezu merged 2 commits into from
Nov 15, 2025
Merged

♻️Refactor : 행사 날짜 - 당일로 포맷팅 #143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a1d762
feat : 날짜 포맷팅(오늘 날짜로)
KongMezu Nov 15, 2025
ea26b17
feat : 팝업 오류 해결
KongMezu Nov 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/shared/components/datePicker/DatePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,19 @@ export function DatePicker({
...calendarProps
}: DatePickerProps) {
const [open, setOpen] = useState(false);
const [innerDate, setInnerDate] = useState<Date | undefined>(defaultValue);

const [displayMonth, setDisplayMonth] = useState<Date>(
const today = new Date();
const startOfToday = new Date(
today.getFullYear(),
today.getMonth(),
today.getDate(),
);
Comment on lines +27 to +32

@coderabbitai coderabbitai Bot Nov 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

미사용 변수: startOfToday가 선언되었으나 사용되지 않습니다.

startOfToday 변수가 27-32라인에서 선언되었지만, 92-93라인의 관련 코드가 주석 처리되어 실제로는 사용되지 않습니다. 이는 리팩토링이 완료되지 않았음을 시사합니다.

다음 중 하나를 선택하세요:

  1. startOfToday를 사용하는 기능을 구현하고 92-93라인의 주석을 해제
  2. startOfToday 변수 선언을 제거

주석 처리된 코드는 향후 구현 예정인 기능인가요? 그렇다면 TODO 주석을 추가하거나 이슈를 생성하는 것을 권장합니다.

이 기능 구현을 도와드릴까요? 또는 추적을 위한 이슈를 생성해드릴까요?

Also applies to: 92-93

🤖 Prompt for AI Agents 
In src/shared/components/datePicker/DatePicker.tsx around lines 27-32,
startOfToday is declared but unused (the related logic at lines 92-93 is
commented out); either remove the startOfToday declaration to eliminate the dead
variable, or re-enable and wire the commented logic at 92-93 so startOfToday is
actually used (ensure the comparison/usage matches the intended behavior and
tests); if the commented code is intended for future work, restore the
declaration removal but add a TODO comment and/or create a tracking issue
referencing the planned feature instead of leaving unused code.

⚠️ Potential issue | 🟠 Major

성능 이슈: 매 렌더링마다 날짜 객체가 재생성됩니다.

today와 startOfToday가 컴포넌트 본문 내에서 선언되어 매 렌더링마다 새로운 Date 객체가 생성됩니다. 이는 불필요한 메모리 할당과 성능 저하를 야기합니다.

다음과 같이 수정하여 최적화하세요:

  ...calendarProps
}: DatePickerProps) {
  const [open, setOpen] = useState(false);

+  const [today] = useState(() => new Date());
+  const [startOfToday] = useState(() => {
+    const t = new Date();
+    return new Date(t.getFullYear(), t.getMonth(), t.getDate());
+  });
-  const today = new Date();
-  const startOfToday = new Date(
-    today.getFullYear(),
-    today.getMonth(),
-    today.getDate(),
-  );

  const [innerDate, setInnerDate] = useState<Date | undefined>(

또는 컴포넌트 외부에서 한 번만 계산하도록 이동하세요.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const today = new Date();
const startOfToday = new Date(
today.getFullYear(),
today.getMonth(),
today.getDate(),
);
const [today] = useState(() => new Date());
const [startOfToday] = useState(() => {
const t = new Date();
return new Date(t.getFullYear(), t.getMonth(), t.getDate());
});
🤖 Prompt for AI Agents 
In src/shared/components/datePicker/DatePicker.tsx around lines 27 to 32, the
component recreates new Date objects on every render (today and startOfToday),
causing unnecessary allocations; fix by computing these values once and reusing
them — either move the calculation outside the component at module scope so they
are created only once, or wrap the creation in a useMemo/useRef with an empty
dependency array inside the component to memoize the Date instances; update any
usages to reference the memoized/module-scope variables and remove per-render
creation.


const [innerDate, setInnerDate] = useState<Date | undefined>(
defaultValue ?? today
);

const [displayMonth, setDisplayMonth] = useState<Date>(
toFirstOfMonth(value ?? innerDate ?? new Date()),
);

Expand Down Expand Up @@ -56,12 +66,6 @@ export function DatePicker({
setOpen(false);
};

const today = new Date();
const startOfToday = new Date(
today.getFullYear(),
today.getMonth(),
today.getDate(),
);

return (
<div className={cn('flex flex-col gap-3', className)}>
Expand Down
4 changes: 2 additions & 2 deletions src/shared/components/set/PopupSet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ export default function PopupSet({ text, onClose }: PopupSetProps) {

return (
<>
<Overlay />
<Overlay onClick={handleClose} />
{/* 중앙 고정 */}
<div
className={cn(
'fixed inset-0 z-[100]',
'fixed inset-0 z-[10000]',

@coderabbitai coderabbitai Bot Nov 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

z-index 값이 과도하게 높습니다.

z-index를 100에서 10000으로 변경한 것은 Overlay 컴포넌트의 9999와 일관성을 유지하기 위한 것으로 보이지만, 두 값 모두 불필요하게 높습니다.

Overlay(9999)와 PopupSet(10000)의 1 차이는 의미가 있지만, 전체적인 z-index 스케일을 낮추는 것을 권장합니다 (예: Overlay 100, PopupSet 101).

🤖 Prompt for AI Agents 
In src/shared/components/set/PopupSet.tsx around line 27, the z-index class
'z-[10000]' is excessively large; lower the stacking scale so PopupSet uses a
modest value (e.g., z-101 or z-[101]) instead of 10000, and ensure consistency
by updating the Overlay component to use a base like z-100 so the relative
ordering (Overlay 100, PopupSet 101) is preserved across the codebase; replace
the long-form 10000 class with the smaller value and run a quick UI check to
confirm no stacking regressions.

'flex items-center justify-center',
)}
>
Expand Down
2 changes: 1 addition & 1 deletion src/shared/overlay/Overlay.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const Overlay = ({ onClick, opacity = 30, className }: OverlayProps) => {
<div
onClick={onClick}
className={`
fixed inset-0 z-[99]
fixed inset-0 z-[9999]

@coderabbitai coderabbitai Bot Nov 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

z-index 값이 과도하게 높습니다.

z-index를 99에서 9999로 변경하는 것은 100배 증가이며, 이는 z-index 관리 전략에 문제가 있을 수 있음을 시사합니다. 이러한 높은 값은 다음과 같은 문제를 야기할 수 있습니다:

  • 향후 더 높은 레이어가 필요할 때 값을 계속 증가시키는 악순환
  • 서드파티 라이브러리나 다른 컴포넌트와의 z-index 충돌 가능성
  • 코드베이스 전반의 z-index 체계 관리 어려움

100~200 범위의 값으로도 충분할 가능성이 높으니, 실제로 이렇게 높은 값이 필요한지 검토해주세요.

다음 스크립트로 프로젝트 전체의 z-index 사용 현황을 확인할 수 있습니다:

🏁 Script executed:

#!/bin/bash
# z-index 사용 패턴 분석

# Tailwind z-index 클래스 검색
rg -nP 'z-\[?\d+\]?' --type ts --type tsx -C2

# CSS 파일에서 z-index 검색
rg -nP 'z-index:\s*\d+' --type css -C2

Length of output: 27

z-index 사용 현황을 정확히 파악하기 위해 수정된 스크립트를 실행하겠습니다.

🏁 Script executed:

#!/bin/bash
# z-index 사용 패턴 분석 (수정)

# Tailwind z-index 클래스 검색 (ts, tsx, jsx 파일)
echo "=== Tailwind z-index 클래스 (z-[숫자]) ==="
rg -nP 'z-\[\d+\]' -g '*.ts' -g '*.tsx' -g '*.jsx' -C2

# 대체 방식: z- 패턴 검색
echo "=== Tailwind z- 패턴 ==="
rg -n 'z-\d+' -g '*.ts' -g '*.tsx' -g '*.jsx' -C2

# CSS 파일에서 z-index 검색
echo "=== CSS z-index ==="
rg -nP 'z-index' -g '*.css' -g '*.scss' -C2

# Overlay.tsx 파일 내용 확인
echo "=== Overlay.tsx 파일 확인 ==="
fd -t f 'Overlay.tsx' -x cat {}

Length of output: 8186

z-index 관리 체계 개선이 필수입니다. 더 심각한 우선순위 문제도 발견되었습니다.

코드베이스 검토 결과, 리뷰 우려가 타당합니다:

  • z-index 값들이 일관성 없이 혼재함 (0, 10, 50, 100, 2000, 9999, 10000)
  • 악순환이 이미 시작됨: PopupSet.tsx (라인 27)에서 z-[10000] 사용 중이므로, Overlay의 z-[9999]는 의도와 달리 뒤로 가게 됨
  • 제3자 라이브러리와의 충돌 위험 높음

수정 필요 사항:

  1. 긴급: src/shared/components/set/PopupSet.tsx (라인 27)에서 z-[10000] 검토 – Overlay보다 우선순위가 높은지 확인
  2. 필수: 전사적 z-index 관리 체계 정립 (e.g., 100~200 범위로 계층화)
  3. 필수: Overlay의 z-[9999], ResultMap의 z-[9999]를 체계적 값으로 변경
  4. 권고: 향후 새로운 레이어 추가 시 사전에 체계 검토
🤖 Prompt for AI Agents 
src/shared/overlay/Overlay.tsx around line 12: Overlay currently uses z-[9999],
which conflicts with other high z-indexes and is lower than PopupSet's
z-[10000]; change Overlay to use a project-standard z-index (e.g., move to a
defined layer like z-[200] or a CSS variable/class such as var(--z-overlay) or
.z-overlay) and update any other hardcoded 9999 usages (e.g., ResultMap) to the
same standard; also inspect src/shared/components/set/PopupSet.tsx line 27
(z-[10000]) and either lower it to the correct layer or document/justify it in
the new z-index plan so the overlay stacking order is consistent across the app.

transition-opacity duration-300
${className ?? ''}
`}
Expand Down
Loading