Skip to content

fix: 피드백 수정#14

Open
kimbosung521 wants to merge 2 commits into
developfrom
feature-web-19
Open

fix: 피드백 수정#14
kimbosung521 wants to merge 2 commits into
developfrom
feature-web-19

Conversation

@kimbosung521

Copy link
Copy Markdown

캘린더 점 위치 수정

📌 작업 내용

🔗 관련 이슈

📸 스크린샷 / 화면 녹화

🧪 테스트 방법

✅ 체크리스트

  • npm run lint 통과를 확인했습니다
  • npm run format 을 적용했습니다
  • 디버깅용 console.log 를 제거했습니다
  • 사용하지 않는 import / 변수를 정리했습니다
  • 컨벤션 (README.md) 을 준수했습니다
  • base 브랜치가 develop 인지 확인했습니다

💬 리뷰어에게 한 마디

캘린더 점 위치 수정

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the calendar date rendering in CalendarScheduleView.jsx by replacing pseudo-element schedule indicators with an explicit span element. The reviewer provided valuable feedback pointing out Tailwind CSS class conflicts and duplicate classes on the date buttons, along with a suggestion to adjust the schedule dot's positioning to a bottom-based offset for better responsiveness and alignment.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +72 to +81
className={[
'relative grid h-[31px] w-[34px] place-items-center text-[18px] font-normal leading-none sm:w-10 xl:h-[40px] xl:w-[40px]',
date.key === todayKey
? 'h-10 w-10 rounded-[11px] bg-brand font-bold text-white shadow-schedule-day xl:h-10 xl:w-10 shadow-schedule-day'
: '',
date.key !== todayKey && !date.isCurrentMonth ? 'text-schedule-inactive' : '',
date.key !== todayKey && date.isCurrentMonth ? 'text-ink' : '',
]
.filter(Boolean)
.join(' ')}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

⚠️ Tailwind 클래스 충돌 및 중복 클래스 발견

  1. 클래스 충돌: date.key === todayKey일 때, 기본 클래스의 크기(h-[31px] w-[34px] xl:h-[40px] xl:w-[40px])와 오늘 날짜 클래스의 크기(h-10 w-10 xl:h-10 xl:w-10)가 동시에 적용되어 Tailwind 클래스 충돌이 발생합니다. CSS 로드 순서에 따라 의도치 않은 크기로 렌더링될 수 있습니다.
  2. 중복 클래스: shadow-schedule-day 클래스가 중복으로 선언되어 있습니다.

해결 방안:
오늘 날짜 여부(date.key === todayKey)에 따라 크기 관련 클래스를 분기 처리하여 충돌을 방지하고 중복 클래스를 제거합니다.

Suggested change
className={[
'relative grid h-[31px] w-[34px] place-items-center text-[18px] font-normal leading-none sm:w-10 xl:h-[40px] xl:w-[40px]',
date.key === todayKey
? 'h-10 w-10 rounded-[11px] bg-brand font-bold text-white shadow-schedule-day xl:h-10 xl:w-10 shadow-schedule-day'
: '',
date.key !== todayKey && !date.isCurrentMonth ? 'text-schedule-inactive' : '',
date.key !== todayKey && date.isCurrentMonth ? 'text-ink' : '',
]
.filter(Boolean)
.join(' ')}
className={[
'relative grid place-items-center text-[18px] font-normal leading-none',
date.key === todayKey
? 'h-10 w-10 rounded-[11px] bg-brand font-bold text-white shadow-schedule-day xl:h-10 xl:w-10'
: 'h-[31px] w-[34px] sm:w-10 xl:h-[40px] xl:w-[40px]',
date.key !== todayKey && !date.isCurrentMonth ? 'text-schedule-inactive' : '',
date.key !== todayKey && date.isCurrentMonth ? 'text-ink' : '',
]
.filter(Boolean)
.join(' ')}

Comment on lines +83 to +88
{hasSchedule && (
<span
aria-hidden="true"
className="absolute left-1/2 top-[-10px] h-2.5 w-2.5 -translate-x-1/2 rounded-full bg-brand"
/>
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

📌 일정 표시 점(Dot)의 위치 설정 개선

top-[-10px] 클래스는 요소를 버튼의 상단 바깥쪽(위쪽)으로 위치시킵니다. 일반적으로 달력의 일정 표시 점은 날짜 아래에 위치하므로, 의도한 위치가 아래쪽이라면 bottom-[-6px] 또는 bottom-[-8px]과 같이 bottom 기준의 음수 오프셋을 사용하는 것이 적절합니다.

또한, bottom을 기준으로 설정하면 버튼의 높이가 화면 크기에 따라 변하더라도 (h-[31px] vs xl:h-[40px]) 항상 버튼 하단을 기준으로 일정 간격을 유지하므로 반응형 대응이 훨씬 수월해집니다.

추천:
의도한 디자인이 날짜 아래에 점이 위치하는 것이라면, top-[-10px] 대신 bottom-[-6px] 등을 사용하는 것을 권장합니다.

Suggested change
{hasSchedule && (
<span
aria-hidden="true"
className="absolute left-1/2 top-[-10px] h-2.5 w-2.5 -translate-x-1/2 rounded-full bg-brand"
/>
)}
{hasSchedule && (
<span
aria-hidden="true"
className="absolute left-1/2 bottom-[-6px] h-2.5 w-2.5 -translate-x-1/2 rounded-full bg-brand"
/>
)}

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.

1 participant