[Fe] 317 버그출석관리 세션 삭제 안 열림 이슈#318
Hidden character warning
Conversation
Walkthrough회원 상태 변경 옵션을 ACTIVE/INACTIVE로 제한하고, 사이드바에 피드백 작성 모달과 접근성/포커스 복원 로직을 추가했으며, 세션 관리 카드에서 권한 오류 감지·처리와 라운드 테이블 열(시작시간/종료시간) 및 QR 버튼·드롭다운 가시성 관련 스타일을 조정했습니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 docstrings
🧪 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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
Pull request overview
This PR updates the frontend to address attendance session management UX/bug issues, adds a feedback submission modal in the sidebar, and applies several UI styling adjustments.
Changes:
- Added a “피드백 작성하기” modal in the sidebar and custom scrollbar styling.
- Improved attendance session management UI (table columns, QR button styling) and added special-case handling for session-owner permission errors.
- Adjusted Admin member status change flow to restrict selectable statuses.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/components/Sidebar.module.css | Adds scrollbar styling and styles for the new feedback modal/trigger area. |
| frontend/src/components/Sidebar.jsx | Adds feedback modal state/submit logic and renders modal + trigger button in sidebar layout. |
| frontend/src/components/attendancemanage/SessionManagementCard.module.css | Updates QR button styling and changes overflow/z-index to keep dropdown visible. |
| frontend/src/components/attendancemanage/SessionManagementCard.jsx | Adds permission-error detection for delete, updates table columns to show start/end times. |
| frontend/src/components/AdminMemberManage/AdminMemberManage.jsx | Restricts status-change options and adjusts dialog initialization/validation. |
Comments suppressed due to low confidence (1)
frontend/src/components/AdminMemberManage/AdminMemberManage.jsx:201
- 상태 변경 로직이
STATUS_CHANGE_OPTIONS(['ACTIVE','INACTIVE'])로 제한되어GRADUATED로의 변경이 불가능해졌습니다. 의도된 정책 변경이라면 UI에서 '졸업' 상태를 어떻게 설정/해제하는지(예: 별도 버튼/플로우)도 함께 제공되어야 하고, 아니라면GRADUATED를 옵션에 포함하거나 별도 처리를 추가해야 합니다.
}
await changeAdminMemberRole({ userId: member.id, role: value });
} else if (type === 'status') {
if (!STATUS_CHANGE_OPTIONS.includes(value)) {
window.alert('유효하지 않은 상태입니다.');
return;
}
await changeAdminMemberStatus({ userId: member.id, status: value });
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
frontend/src/components/attendancemanage/SessionManagementCard.module.css (1)
600-623:.qrButton스타일 중복 정의를 한 곳으로 통합해 주세요Line 600-623의
.qrButton/:hover는 상단(Line 441-464)과 실질적으로 중복입니다. 동일 규칙이 2군데에 있으면 이후 수정 시 불일치가 발생하기 쉽습니다.정리 예시
-.qrButton { - background: linear-gradient(99.86deg, `#b3ebff` -15.44%, `#91c3ff` 39.86%, `#d3edff` 94.81%); - width: 89px; - height: 40px; - border-radius: 8px; - padding: 12px 26px; - gap: 10px; - display: flex; - justify-content: center; - align-items: center; - border: none; - cursor: pointer; - box-sizing: border-box; - font-family: Inter, Pretendard, sans-serif; - font-weight: 600; - font-size: 14px; - color: `#111827`; - transition: opacity 0.2s, transform 0.2s; -} - -.qrButton:hover { - opacity: 0.9; - transform: translateY(-1px); -} +/* 중복 정의 제거: 상단의 .qrButton 규칙만 유지 */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css` around lines 600 - 623, The .qrButton and .qrButton:hover rules are duplicated; consolidate them into a single definition by keeping one block for .qrButton and one for .qrButton:hover and removing the other duplicate, merging any differing properties so no styles are lost (ensure font, sizing, gradient, box-model, transition and hover transform/opacity are preserved); update only the CSS so all references to .qrButton use the single consolidated ruleset.frontend/src/components/attendancemanage/SessionManagementCard.jsx (1)
231-236: 방어 처리는 필수 아니지만 코드 견고성 개선 시 권장됩니다백엔드
AttendanceRoundService에서closeAt이 항상 설정되도록 보장하므로 (요청값이 없으면startAt으로부터 자동 계산) 현재 코드는 안전합니다. 다만 추가적인 데이터 검증 계층이나 API 변경 시에 대비하는 차원에서 방어 로직을 추가하는 것은 코드 견고성을 높이는 좋은 관행입니다.필요에 따라 아래처럼 개선할 수 있습니다:
선택적 방어 로직 예시
+const formatTime = (value) => { + if (!value) return '-'; + const d = new Date(value); + if (Number.isNaN(d.getTime())) return '-'; + return d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); +}; <td> - {closeTime.toLocaleTimeString([], { - hour: '2-digit', - minute: '2-digit', - })} + {formatTime(round.closeAt)} </td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/attendancemanage/SessionManagementCard.jsx` around lines 231 - 236, Add a defensive null/undefined check around rendering closeTime in SessionManagementCard.jsx: ensure the variable closeTime (derived from the attendance round's closeAt) is validated before calling toLocaleTimeString, and provide a safe fallback (e.g., compute from startAt, render an empty string, or a placeholder) so that if AttendanceRoundService or the API ever returns a missing closeAt the component won't throw; update the rendering logic where closeTime.toLocaleTimeString is used to guard against closeTime being falsy and use the chosen fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css`:
- Around line 706-708: The global .headerControls rule setting overflow: visible
breaks the intended horizontal scroll on narrow screens; change it to apply only
at larger viewports via a media query (e.g., move overflow: visible into a
min-width media query) so the scroll container keeps overflow-x:auto/scroll on
small screens, and ensure any dropdown/select/button within .headerControls uses
position:absolute or a higher z-index so it can overflow without disabling the
horizontal scrolling behaviour.
In `@frontend/src/components/Sidebar.jsx`:
- Around line 385-389: 현재 overlay div with class styles.feedbackModalOverlay
sets aria-hidden="true", which hides the nested role="dialog" from assistive
tech; remove aria-hidden from that overlay (or set it only on background inert
elements) and ensure the actual modal element (the element with role="dialog")
is exposed with appropriate attributes (e.g., aria-modal and focus management).
Locate the overlay using styles.feedbackModalOverlay / closeFeedbackModal and
the modal using role="dialog" and update so the dialog is not hidden while
non-interactive background content is marked inert or aria-hidden instead.
- Around line 372-379: The feedback button currently calls openFeedbackModal
unconditionally, letting anonymous users open the modal but causing backend
errors; update the Sidebar component to check the authenticated user state
(e.g., currentUser, isAuthenticated, or auth context/hook used in this
component) before invoking openFeedbackModal and instead prompt login or show a
toast/error for unauthenticated users; modify both places where the button is
rendered (the block using className={styles.feedbackTriggerWrap} and the other
instance around lines 132-134) so clicks call a guarded handler (e.g.,
handleFeedbackClick) that either calls openFeedbackModal for authenticated users
or triggers the app's login flow / UI message for unauthenticated users.
In `@frontend/src/components/Sidebar.module.css`:
- Around line 347-356: The .feedbackModalOverlay currently fixes the modal but
doesn't allow vertical scrolling; update the overlay and the modal content
wrapper(s) in this stylesheet so the modal can scroll on short viewports by
adding a max-height (e.g., max-height: 100vh or calc(100vh - 40px)) and
overflow-y: auto to the modal container and/or overlay, keep horizontal
centering (display:flex; justify-content:center) but ensure vertical behavior
supports scrolling (you can keep align-items:center but ensure the modal itself
is constrained), and add sensible padding/gap so the bottom buttons remain
accessible on small screens; apply the same changes to the other modal-related
rules in this file that control modal wrapper/content.
---
Nitpick comments:
In `@frontend/src/components/attendancemanage/SessionManagementCard.jsx`:
- Around line 231-236: Add a defensive null/undefined check around rendering
closeTime in SessionManagementCard.jsx: ensure the variable closeTime (derived
from the attendance round's closeAt) is validated before calling
toLocaleTimeString, and provide a safe fallback (e.g., compute from startAt,
render an empty string, or a placeholder) so that if AttendanceRoundService or
the API ever returns a missing closeAt the component won't throw; update the
rendering logic where closeTime.toLocaleTimeString is used to guard against
closeTime being falsy and use the chosen fallback.
In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css`:
- Around line 600-623: The .qrButton and .qrButton:hover rules are duplicated;
consolidate them into a single definition by keeping one block for .qrButton and
one for .qrButton:hover and removing the other duplicate, merging any differing
properties so no styles are lost (ensure font, sizing, gradient, box-model,
transition and hover transform/opacity are preserved); update only the CSS so
all references to .qrButton use the single consolidated ruleset.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aa4a242b-854a-4cb2-971c-ef6491b6651e
📒 Files selected for processing (5)
frontend/src/components/AdminMemberManage/AdminMemberManage.jsxfrontend/src/components/Sidebar.jsxfrontend/src/components/Sidebar.module.cssfrontend/src/components/attendancemanage/SessionManagementCard.jsxfrontend/src/components/attendancemanage/SessionManagementCard.module.css
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/src/components/Sidebar.jsx (1)
134-136:⚠️ Potential issue | 🟠 Major비로그인 사용자의 피드백 진입을 여기서 막아주세요.
openFeedbackModal()이 인증 여부를 확인하지 않아 누구나 모달을 열 수 있습니다. 백엔드backend/src/main/java/org/sejongisc/backend/feedback/controller/FeedbackController.javaLine 19-40은 로그인 사용자를 전제로 처리하므로, 지금 흐름은 작성 후 401로 실패하는 UX가 됩니다. 이 함수에서 로그인 가드와 안내/리다이렉트를 넣는 쪽이 근본 원인 수정입니다.수정 예시
const openFeedbackModal = () => { + if (!isLoggedIn) { + toast.error('로그인 후 피드백을 작성할 수 있습니다.'); + nav('/login'); + handleNavLinkClick(); + return; + } setIsFeedbackModalOpen(true); };Also applies to: 408-413
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/Sidebar.jsx` around lines 134 - 136, openFeedbackModal currently opens the feedback modal unconditionally; update it to check the user's authentication status (e.g., from auth context, Redux selector, or a getCurrentUser hook) before calling setIsFeedbackModalOpen(true). If the user is unauthenticated, show a toast/alert and redirect or open the login flow instead of opening the modal so the subsequent FeedbackController POST won't 401; apply the same guard to the other occurrence referenced (lines ~408-413) and use the same auth check and user-facing message/redirect logic.
🧹 Nitpick comments (1)
frontend/src/components/attendancemanage/SessionManagementCard.module.css (1)
600-618: 중복된.qrButton스타일 정의Lines 441-468에 동일한
.qrButton스타일이 이미 정의되어 있습니다. CSS 캐스케이드 특성상 나중에 정의된 규칙이 적용되므로, 앞선 정의는 데드 코드가 됩니다.하나의 정의만 유지하고 중복을 제거하는 것이 좋습니다.
♻️ 제안: 중복 제거
Lines 441-468의
.qrButton정의를 삭제하거나, Lines 600-622의 정의를 삭제하여 하나만 유지하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css` around lines 600 - 618, There are two identical .qrButton rules; remove the duplicate so only one definition remains to avoid dead CSS—locate the .qrButton block (the rules setting background, width, height, border-radius, padding, gap, display, justify-content, align-items, border, cursor, box-sizing, font-family, font-weight, font-size, color, transition) and delete either the earlier occurrence (lines ~441-468) or the later one (the one shown in the diff) so that a single .qrButton rule defines the component styles.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css`:
- Around line 700-713: The global rule setting overflow: visible on
.sessionManagementCardContainer should be scoped to large screens only; update
the CSS so that remove or revert the top-level overflow: visible and instead add
overflow: visible inside the `@media` (min-width: 1280px) block (mirroring
.headerControls behavior) so that mobile/tablet widths keep overflow: hidden
while desktops allow the dropdown to escape the card.
In `@frontend/src/components/Sidebar.jsx`:
- Around line 143-171: The feedback modal currently only sets initial focus and
restores it on close but does not trap focus, and `inert` is only applied to
`sidebarContent`, allowing keyboard users to tab out of the modal; update the
`useEffect` that watches `isFeedbackModalOpen` (and the other similar blocks
around the feedback modal logic) to implement a true focus trap or apply `inert`
to the app root while the modal is open: when `isFeedbackModalOpen` becomes
true, set focus to `feedbackTextareaRef.current`, add key handlers for Escape
and Tab to keep focus cycling inside the modal, and set
`document.getElementById(appRootId)` or the top-level container to `inert=true`
(or toggle a class that sets inert on all non-modal siblings) and on cleanup
remove inert, remove handlers, and restore focus to `previousActiveElement` or
`feedbackTriggerButtonRef.current`; ensure the focus-trap logic references
`feedbackTextareaRef`, `feedbackTriggerButtonRef`, and `sidebarContent` so it
applies consistently wherever the modal is opened.
---
Duplicate comments:
In `@frontend/src/components/Sidebar.jsx`:
- Around line 134-136: openFeedbackModal currently opens the feedback modal
unconditionally; update it to check the user's authentication status (e.g., from
auth context, Redux selector, or a getCurrentUser hook) before calling
setIsFeedbackModalOpen(true). If the user is unauthenticated, show a toast/alert
and redirect or open the login flow instead of opening the modal so the
subsequent FeedbackController POST won't 401; apply the same guard to the other
occurrence referenced (lines ~408-413) and use the same auth check and
user-facing message/redirect logic.
---
Nitpick comments:
In `@frontend/src/components/attendancemanage/SessionManagementCard.module.css`:
- Around line 600-618: There are two identical .qrButton rules; remove the
duplicate so only one definition remains to avoid dead CSS—locate the .qrButton
block (the rules setting background, width, height, border-radius, padding, gap,
display, justify-content, align-items, border, cursor, box-sizing, font-family,
font-weight, font-size, color, transition) and delete either the earlier
occurrence (lines ~441-468) or the later one (the one shown in the diff) so that
a single .qrButton rule defines the component styles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de3e4666-d7ab-4d4c-9684-1eed74e3cfe2
📒 Files selected for processing (3)
frontend/src/components/Sidebar.jsxfrontend/src/components/Sidebar.module.cssfrontend/src/components/attendancemanage/SessionManagementCard.module.css
✅ Files skipped from review due to trivial changes (1)
- frontend/src/components/Sidebar.module.css
세션 수정 및 삭제 버그 수정
피드백 모달 추가
기타 UI/UX 수정
Summary by CodeRabbit
릴리스 노트
New Features
Bug Fixes
Style