184 feat 달력 UI 수정 및 아이콘 그림자 추가#185
Merged
Conversation
Walkthrough세 개의 컴포넌트에서 UI 개선 작업이 진행되었습니다. NotificationBell에는 아이콘에 드롭섀도 스타일이 추가되었고, ScheduleDetail에서는 일정의 월/일 헤더가 제거되었습니다. Schedule/index에서는 바텀시트 구조를 절대 위치 기반에서 Portal 기반 고정 위치로 변경하고, CSS 변수를 활용한 높이 계산이 도입되었습니다. Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/pages/Schedule/index.tsx (2)
90-90: 페이지 루트는Layout로 감싸는 편이 맞습니다.지금처럼 높이를 페이지에서 직접 계산하면 헤더/배경/하단 영역 제어가 이 파일에 계속 남습니다. 이 페이지도
Layout+showBottomNav/contentClassName조합으로 맞춰주세요.As per coding guidelines: "
src/pages/**/*.tsx: PassshowBottomNav(bottom tab display) andcontentClassName(background color, etc.) props to Layout component"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Schedule/index.tsx` at line 90, Wrap the page root JSX in the Layout component instead of manually computing heights: replace the top-level div with Layout and pass the props showBottomNav and contentClassName to control bottom tab visibility and background styling; remove the manual height calc on the div (the element with className "relative flex h-[calc(var(--viewport-height)-44px)] flex-col overflow-hidden bg-white") and move any remaining layout-specific classNames into Layout's contentClassName so header/footer/background are managed by Layout.
132-135: 조건부 Tailwind 클래스는cn()으로 합쳐주세요.Line 133의 템플릿 문자열은 이 저장소의 클래스 병합 규칙과 어긋납니다.
cn()으로 바꾸면 조건 분기가 더 안전하고 읽기 쉽습니다.As per coding guidelines: "Use `cn()` utility from `src/utils/ts/cn.ts` to merge Tailwind CSS classes"예시
+ import { cn } from '@/utils/ts/cn'; ... - className={`fixed inset-0 z-[31] bg-black/40 transition-opacity duration-300 ${isSheetExpanded ? 'pointer-events-auto opacity-100' : 'pointer-events-none opacity-0'}`} + className={cn( + 'fixed inset-0 z-[31] bg-black/40 transition-opacity duration-300', + isSheetExpanded ? 'pointer-events-auto opacity-100' : 'pointer-events-none opacity-0' + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Schedule/index.tsx` around lines 132 - 135, Replace the template-string Tailwind usage with the project's classnames helper: import and use the cn() utility to merge static and conditional classes for the backdrop div (where isSheetExpanded controls 'pointer-events-auto opacity-100' vs 'pointer-events-none opacity-0'); update the JSX that currently builds the className with a template literal to call cn('fixed inset-0 z-[31] bg-black/40 transition-opacity duration-300', isSheetExpanded ? 'pointer-events-auto opacity-100' : 'pointer-events-none opacity-0') and keep the onClick handler (setIsSheetExpanded(false)) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Schedule/index.tsx`:
- Around line 137-155: The bottom sheet lacks modal accessibility: add
role="dialog" and aria-modal="true" to the section rendered by Schedule (the
sheet using isSheetExpanded), implement focus management by saving
document.activeElement before opening, moving focus into a tabbable element
inside the sheet (use a ref on the sheet container or first focusable child)
when isSheetExpanded becomes true, restore focus to the saved element on close
(setIsSheetExpanded false), and add an Escape key handler (tied to the same
logic that handleSheetTouchEnd/setIsSheetExpanded) to close the sheet; also
ensure background content is hidden from assistive tech (e.g., set aria-hidden
on the main page container while the sheet is open) and consider using a small
focus trap implemented in the Schedule component to keep tab focus cycling
inside the ScheduleDetail content.
---
Nitpick comments:
In `@src/pages/Schedule/index.tsx`:
- Line 90: Wrap the page root JSX in the Layout component instead of manually
computing heights: replace the top-level div with Layout and pass the props
showBottomNav and contentClassName to control bottom tab visibility and
background styling; remove the manual height calc on the div (the element with
className "relative flex h-[calc(var(--viewport-height)-44px)] flex-col
overflow-hidden bg-white") and move any remaining layout-specific classNames
into Layout's contentClassName so header/footer/background are managed by
Layout.
- Around line 132-135: Replace the template-string Tailwind usage with the
project's classnames helper: import and use the cn() utility to merge static and
conditional classes for the backdrop div (where isSheetExpanded controls
'pointer-events-auto opacity-100' vs 'pointer-events-none opacity-0'); update
the JSX that currently builds the className with a template literal to call
cn('fixed inset-0 z-[31] bg-black/40 transition-opacity duration-300',
isSheetExpanded ? 'pointer-events-auto opacity-100' : 'pointer-events-none
opacity-0') and keep the onClick handler (setIsSheetExpanded(false)) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cddd71bf-9f1c-4832-892a-a64ef4e58f11
📒 Files selected for processing (3)
src/components/layout/Header/components/NotificationBell.tsxsrc/pages/Schedule/components/ScheduleDetail.tsxsrc/pages/Schedule/index.tsx
💤 Files with no reviewable changes (1)
- src/pages/Schedule/components/ScheduleDetail.tsx
Comment on lines
+137
to
+155
| <section | ||
| className="fixed inset-x-0 bottom-0 z-[32] flex flex-col rounded-t-3xl bg-white shadow-[0_-4px_12px_rgba(0,0,0,0.06)] transition-transform duration-300 ease-out" | ||
| style={{ | ||
| height: `calc(var(--viewport-height) - ${SHEET_TOP_OFFSET}px)`, | ||
| transform: isSheetExpanded ? 'translateY(0)' : `translateY(calc(100% - ${PEEK_HEIGHT}px))`, | ||
| }} | ||
| > | ||
| <div | ||
| className="flex shrink-0 justify-center pt-3 pb-2" | ||
| onTouchStart={handleSheetTouchStart} | ||
| onTouchEnd={handleSheetTouchEnd} | ||
| > | ||
| <div className="h-1 w-8 rounded-full bg-[#D1D5DB]" /> | ||
| </div> | ||
|
|
||
| <div className="flex flex-1 flex-col overflow-hidden"> | ||
| <ScheduleDetail year={year} month={month} day={day} onItemClick={() => setIsSheetExpanded(true)} /> | ||
| </div> | ||
| </section> |
There was a problem hiding this comment.
포털 바텀시트에 모달 접근성 처리가 빠져 있습니다.
이제 UI가 사실상 모달인데 role="dialog"/aria-modal이 없고, 포커스 진입·복귀나 Escape 닫기도 없습니다. 현재 상태면 키보드/스크린리더에서 배경으로 포커스가 빠질 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Schedule/index.tsx` around lines 137 - 155, The bottom sheet lacks
modal accessibility: add role="dialog" and aria-modal="true" to the section
rendered by Schedule (the sheet using isSheetExpanded), implement focus
management by saving document.activeElement before opening, moving focus into a
tabbable element inside the sheet (use a ref on the sheet container or first
focusable child) when isSheetExpanded becomes true, restore focus to the saved
element on close (setIsSheetExpanded false), and add an Escape key handler (tied
to the same logic that handleSheetTouchEnd/setIsSheetExpanded) to close the
sheet; also ensure background content is hidden from assistive tech (e.g., set
aria-hidden on the main page container while the sheet is open) and consider
using a small focus trap implemented in the Schedule component to keep tab focus
cycling inside the ScheduleDetail content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
변경 사항
스타일
개선