-
Notifications
You must be signed in to change notification settings - Fork 0
#27 refactor: 강의 상세 페이지 리펙토링 #28
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.
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
Conversation
…ourse-overview-update
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
App Structure & Routing src/App.tsx |
Adds UserTypeContext, new AppContent deriving userType from useLocation and providing it to routes; moves routing into AppContent. |
Models / Types src/models/common.ts, src/models/course.ts, src/pages/course-overview/models/response.ts |
New ApiResponse<T> and UserType; comprehensive course models (SemesterCode, SubmissionStatus, Assignment, Unit, BaseCourse, CourseOverview, CourseOverviewResponse); updates response typing. |
| CourseOverview (removed: common → added: pages) Removed: src/components/common/CourseOverview/* Added: src/pages/course-overview/CourseOverviewPage.tsx, src/pages/course-overview/ui/*, src/pages/course-overview/models/* |
Removes legacy shared CourseOverview components and types; introduces page-scoped components: CourseHero, CourseContent, CourseActionsBar, AssignmentList, UnitItem, plus page and types. |
SVG assets & imports src/assets/svg/*, src/assets/svg/index.ts, various callers |
Deletes several SVG component files and the barrel index; updates many imports to direct @/assets/svg/... (with ?react where used). |
Course utilities src/utils/course.ts, removed src/utils/getSemesterLabel.ts |
Adds formatSemester, formatDate, formatPeriod, formatCourseInfo, getTotalAssignmentCount; removes old getSemesterLabel. |
UI tweaks & other updates src/components/common/Badge.tsx, src/components/common/Dashboard/*, src/index.css, src/layout/Layout.tsx, assorted pages |
Badge now imports icons directly and adds text-sm; CourseCard uses formatSemester; adds bg-radial-gradient utility; normalizes SVG import paths and centralizes UserType type import in Layout. |
Removed legacy page src/pages/common/CourseOverviewPage.tsx |
Deletes legacy common CourseOverview page in favor of the new course-overview page implementation. |
Sequence Diagram(s)
sequenceDiagram
actor Browser
participant App as App (BrowserRouter)
participant AppContent as AppContent (useLocation)
participant Routes as Route Resolver
participant CoursePage as CourseOverviewPage
participant Hero as CourseHero
participant Content as CourseContent
Browser->>App: Navigate to /course-overview/...
App->>AppContent: render
AppContent->>AppContent: determine userType from pathname
AppContent->>Routes: render routes with UserTypeContext provider
Routes->>CoursePage: match /course-overview
CoursePage->>Hero: render CourseHero (reads UserTypeContext)
CoursePage->>Content: render CourseContent (reads UserTypeContext)
Hero-->>Browser: present hero UI
Content-->>Browser: present units/assignments UI
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- #16 feat: 강의상세보기 페이지 구현 #19 — Overlapping CourseOverview rework: both PRs modify CourseOverview components, Assignment/Unit UI, and related types.
- #3 feat: Dashboard UI 구현 #8 — Related App.tsx routing/user-type handling changes (both add/modify location-based user-type logic).
- #13 refactor: assets 관리 구조 개선 및 라우팅 최적화 (Prettier 도입) #23 — Related SVG asset/export changes (barrel removal and direct import updates).
Suggested labels
🛠️ refactor, 🎨 ui
Suggested reviewers
- JiiminHa
Poem
🐰 I hopped through routes and types today,
A context planted where pathlights sway,
Old course bits moved to a tidy nest,
Icons found new homes and styles were dressed,
Cheers from a rabbit — the code hops best!
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly summarizes the main change: a refactoring of the course detail page (강의 상세 페이지 리펙토링). It is concise, specific, and directly related to the extensive structural changes in the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/common/Badge.tsx (1)
97-108: Non-null assertion onprops.statusmay cause runtime error.The
props.status!assertion on line 99 assumesstatusis always defined. However,VariantProps<typeof submissionBadgeStyles>likely makesstatusoptional. Ifstatusisundefined, accessingSubmissionMeta[undefined]will returnundefined, causing a destructuring error.Suggested fix with fallback
// 제출 상태 배지 case 'submission': - const {label, icon} = SubmissionMeta[props.status!]; + const status = props.status ?? 'NOT_SUBMITTED'; + const {label, icon} = SubmissionMeta[status]; return ( <span className={submissionBadgeStyles({ - status: props.status, + status, })}> {icon} {label} </span> );src/pages/course-overview/models/response.ts (1)
19-42: Data inconsistency:assignmentCountdoesn't match actual assignments.The first unit declares
assignmentCount: 2but contains 4 assignments in the array. This mismatch could cause bugs in components that rely onassignmentCountfor rendering or calculations (e.g.,getTotalAssignmentCountinsrc/utils/course.ts).Suggested fix
isOpen: true, - assignmentCount: 2, + assignmentCount: 4, assignments: [
🤖 Fix all issues with AI agents
In `@src/components/common/Dashboard/CourseCard.tsx`:
- Line 30: In CourseCard.tsx remove the extra "학기" that is appended after the
formatSemester(semester) call (formatSemester already returns a value with
"학기"), and update the template that builds the label to concatenate year,
formatSemester(semester), and section with the separators so the output is "year
\ semester \ section분반" without duplicating the suffix; reference the
formatSemester function and the variables year, semester, and section in your
change.
In `@src/models/course.ts`:
- Around line 8-12: The Assignment interface requires submittedStatus but a
duplicate/interface variant in components uses submittedStatus? which causes
type mismatch; update the canonical Assignment definition (the exported
Assignment interface) to make submittedStatus optional (submittedStatus?) or
unify both by importing a single shared type (e.g., export/import the Assignment
type) so both src/models/course.ts and
src/components/admin/assignments/dummy/types.ts reference the same definition;
adjust any call sites that assume submittedStatus is always present to handle
undefined or provide defaults.
In `@src/pages/course-overview/ui/AssignmentList.tsx`:
- Around line 32-37: The Link in the AssignmentItem component is using the
display index for routing instead of the assignment's real identifier; update
the AssignmentItemProps and the AssignmentItem signature to accept the
assignment id (e.g., id: string | number) and replace any occurrences of
`/assignments/${index}` with `/assignments/${id}` (also update the other Link
instance mentioned around lines 49–54 to use id as well) so routes use the
actual Assignment.id rather than the 1-based display index.
In `@src/pages/course-overview/ui/CourseActionsBar.tsx`:
- Around line 8-17: The two Link components in CourseActionsBar.tsx currently
have empty to='' props, making the links non-functional; update the Link
destinations to real routes (e.g., use the previous pattern such as
'/admin/student?course={courseId}' for the "학생 목록" Link and a route like
'/admin/course/{courseId}/add-unit' or similar for the "단원 추가" Link) or, if you
truly intend placeholders, add a clear TODO comment and a non-empty stub route
(e.g., '/todo/course-actions') so navigation is explicit; locate the Link
elements around the Button components that reference isActiveCourse to apply the
changes.
🧹 Nitpick comments (11)
src/models/common.ts (1)
5-8: Consider adding an error field for failed responses.The current
ApiResponse<T>only hassuccessandresponse. Whensuccessisfalse, consumers may need error details (message, code, etc.).💡 Optional enhancement
export interface ApiResponse<T> { success: boolean; response: T; + error?: { + message: string; + code?: string; + }; }src/pages/admin/assignments/AssignmentCreatePage.tsx (1)
40-44: Consider using a stable key instead of array index.Using
idxaskeycan cause incorrect component behavior if examples are reordered or deleted in the future. A unique identifier would be more robust.💡 Suggested improvement
-const [examples, setExamples] = useState([{input: '', output: '', 공개: ''}]); +const [examples, setExamples] = useState([{id: crypto.randomUUID(), input: '', output: '', 공개: ''}]); const handleAddExample = () => { - setExamples([...examples, {input: '', output: '', 공개: ''}]); + setExamples([...examples, {id: crypto.randomUUID(), input: '', output: '', 공개: ''}]); };Then use
example.idas the key:-<div key={idx} ...> +<div key={example.id} ...>src/pages/course-overview/models/response.ts (1)
68-115: Remove commented-out code.This large block of commented-out code is dead code that adds noise. If it's needed for reference, consider moving it to version control history instead.
src/utils/course.ts (1)
3-8: Minor: Redundantas constassertion.The
as constassertion on line 8 is unnecessary since the object is already explicitly typed asRecord<SemesterCode, '1' | '2' | '여름' | '겨울'>. The type annotation already constrains the values.Optional cleanup
const SEMESTER_MAP: Record<SemesterCode, '1' | '2' | '여름' | '겨울'> = { FIRST: '1', SECOND: '2', SUMMER: '여름', WINTER: '겨울', -} as const; +};src/pages/course-overview/CourseOverviewPage.tsx (2)
8-10: Consider usingUserTypeContextinstead of duplicating pathname logic.The PR introduces
UserTypeContextinApp.tsxto eliminate props drilling and centralize user type detection. However, this component duplicates the samepathname.startsWith('/admin')logic. Using the context would be more consistent with the PR's stated goals.♻️ Suggested refactor
import {getTotalAssignmentCount} from '@/utils/course'; import CourseContent from './ui/CourseContent'; import CourseHero from './ui/CourseHero'; import {courseResponse} from '@/pages/course-overview/models/response'; -import {useLocation} from 'react-router-dom'; +import {useContext} from 'react'; +import {UserTypeContext} from '@/App'; const CourseOverviewPage = () => { - // 관리자 경로 확인 - const pathname = useLocation().pathname; - const isAdmin = pathname.startsWith('/admin'); + const userType = useContext(UserTypeContext); + const isAdmin = userType === 'admin';
14-14: Simplify boolean expression.The ternary
courseData.unitCount !== 0 ? true : falseis redundant. The comparison already produces a boolean.♻️ Simplified version
- const hasUnits = courseData.unitCount !== 0 ? true : false; + const hasUnits = courseData.unitCount > 0;src/pages/course-overview/ui/CourseActionsBar.tsx (1)
5-7: Consider extracting props interface and using appropriate semantic element.Per the codebase learning, using TypeScript interfaces for component props improves maintainability. Additionally,
<article>is semantically intended for self-contained content;<div>or<nav>would be more appropriate for a button bar.♻️ Suggested improvements
+interface CourseActionsBarProps { + isActiveCourse: boolean; +} + // 강의 관리 버튼 바 (관리자 전용) -const CourseActionsBar = ({isActiveCourse}: {isActiveCourse: boolean}) => { +const CourseActionsBar = ({isActiveCourse}: CourseActionsBarProps) => { return ( - <article className='flex gap-5'> + <div className='flex gap-5'> ... - </article> + </div> ); };Based on learnings, using TypeScript interfaces for React component props provides clear structure and improves maintainability.
src/App.tsx (1)
19-23: Consider extracting userType derivation for testability.The nested ternary works but could be extracted into a utility function for easier unit testing and reuse.
♻️ Optional extraction
// In a utils file or at the top of App.tsx const getUserTypeFromPath = (pathname: string): UserType => { if (pathname.startsWith('/admin')) return 'admin'; if (pathname.startsWith('/student')) return 'student'; return 'guest'; }; // In AppContent const userType = getUserTypeFromPath(pathname);src/pages/course-overview/ui/CourseContent.tsx (1)
13-26: Consider handling empty units array when course is active.If
isActiveCourseistruebutunitsis an empty array, the component will render an empty<section>with no visible content. Consider whether this edge case should also showEmptyCourseor some placeholder.🔧 Suggested handling
const CourseContent = ({units, isActiveCourse}: CourseContentProps) => { // 빈 강의 - if (isActiveCourse === false) { + if (!isActiveCourse || units.length === 0) { return <EmptyCourse />; }src/pages/course-overview/ui/UnitItem.tsx (1)
60-63: Semantic mismatch in<time>element.The
dateTimeattribute contains onlyreleaseDate, but the displayed content ({deadline}) shows a date range (release ~ due). For proper semantics, consider either:
- Removing
dateTimesince the content represents a range, or- Wrapping each date in its own
<time>element🔧 Option: Use two separate time elements
- <div className='shrink-0 ml-2 text-base text-light-black font-normal min-w-0 whitespace-nowrap'> - <time dateTime={releaseDate}>{deadline}</time> - </div> + <div className='shrink-0 ml-2 text-base text-light-black font-normal min-w-0 whitespace-nowrap'> + <time dateTime={releaseDate}>{formatDate(releaseDate)}</time> + <span> ~ </span> + <time dateTime={dueDate}>{formatDate(dueDate)}</time> + </div>src/pages/course-overview/ui/CourseHero.tsx (1)
20-20: Redundant ternary expression.The comparison
=== 'admin'already returns a boolean; the ternary? true : falseis unnecessary.🔧 Simplified version
- const isAdmin = useContext(UserTypeContext) === 'admin' ? true : false; + const isAdmin = useContext(UserTypeContext) === 'admin';
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (22)
public/vite.svgis excluded by!**/*.svgsrc/assets/svg/addIcon.svgis excluded by!**/*.svgsrc/assets/svg/arrowdownIcon.svgis excluded by!**/*.svgsrc/assets/svg/arrowleftIcon.svgis excluded by!**/*.svgsrc/assets/svg/arrowrightIcon.svgis excluded by!**/*.svgsrc/assets/svg/chatIcon.svgis excluded by!**/*.svgsrc/assets/svg/chevrondown.svgis excluded by!**/*.svgsrc/assets/svg/correct.svgis excluded by!**/*.svgsrc/assets/svg/deleteIcon.svgis excluded by!**/*.svgsrc/assets/svg/dragAndDropIcon.svgis excluded by!**/*.svgsrc/assets/svg/editIcon.svgis excluded by!**/*.svgsrc/assets/svg/ellipsisIcon.svgis excluded by!**/*.svgsrc/assets/svg/file.svgis excluded by!**/*.svgsrc/assets/svg/incorrect.svgis excluded by!**/*.svgsrc/assets/svg/lock.svgis excluded by!**/*.svgsrc/assets/svg/notificationIcon.svgis excluded by!**/*.svgsrc/assets/svg/profileImage.svgis excluded by!**/*.svgsrc/assets/svg/search.svgis excluded by!**/*.svgsrc/assets/svg/signoutIcon.svgis excluded by!**/*.svgsrc/assets/svg/singleEllipsisIcon.svgis excluded by!**/*.svgsrc/assets/svg/unsubmitted.svgis excluded by!**/*.svgsrc/assets/svg/userIcon.svgis excluded by!**/*.svg
📒 Files selected for processing (42)
src/App.tsxsrc/assets/svg/Correct.tsxsrc/assets/svg/Incorrect.tsxsrc/assets/svg/Unsubmitted.tsxsrc/assets/svg/index.tssrc/components/admin/assignments/AssignmentCard.tsxsrc/components/admin/assignments/AssignmentPageLayout.tsxsrc/components/admin/assignments/CourseSelector.tsxsrc/components/admin/form/FileUpload.tsxsrc/components/admin/form/LabeledDropdown.tsxsrc/components/common/Badge.tsxsrc/components/common/CourseOverview/AssignmentItem.tsxsrc/components/common/CourseOverview/AssignmentList.tsxsrc/components/common/CourseOverview/CourseActionsBar.tsxsrc/components/common/CourseOverview/CourseContent.tsxsrc/components/common/CourseOverview/CourseHero.tsxsrc/components/common/CourseOverview/CourseStat.tsxsrc/components/common/CourseOverview/EmptyCourse.tsxsrc/components/common/CourseOverview/SubmissionBadge.tsxsrc/components/common/CourseOverview/Unit.tsxsrc/components/common/CourseOverview/UnitHeader.tsxsrc/components/common/CourseOverview/dummy/types.tssrc/components/common/Dashboard/CourseCard.tsxsrc/components/common/Dashboard/CourseList.tsxsrc/index.csssrc/layout/Layout.tsxsrc/models/common.tssrc/models/course.tssrc/pages/admin/assignments/AssignmentCreatePage.tsxsrc/pages/admin/student/studentManagementPage.tsxsrc/pages/common/CourseOverviewPage.tsxsrc/pages/common/LandingPage.tsxsrc/pages/common/UserIdInputPage.tsxsrc/pages/course-overview/CourseOverviewPage.tsxsrc/pages/course-overview/models/response.tssrc/pages/course-overview/ui/AssignmentList.tsxsrc/pages/course-overview/ui/CourseActionsBar.tsxsrc/pages/course-overview/ui/CourseContent.tsxsrc/pages/course-overview/ui/CourseHero.tsxsrc/pages/course-overview/ui/UnitItem.tsxsrc/utils/course.tssrc/utils/getSemesterLabel.ts
💤 Files with no reviewable changes (17)
- src/components/common/CourseOverview/CourseContent.tsx
- src/components/common/CourseOverview/CourseStat.tsx
- src/pages/common/CourseOverviewPage.tsx
- src/components/common/CourseOverview/AssignmentList.tsx
- src/components/common/CourseOverview/EmptyCourse.tsx
- src/components/common/CourseOverview/CourseHero.tsx
- src/components/common/CourseOverview/dummy/types.ts
- src/components/common/CourseOverview/SubmissionBadge.tsx
- src/assets/svg/Incorrect.tsx
- src/components/common/CourseOverview/Unit.tsx
- src/assets/svg/index.ts
- src/assets/svg/Unsubmitted.tsx
- src/components/common/CourseOverview/UnitHeader.tsx
- src/assets/svg/Correct.tsx
- src/components/common/CourseOverview/AssignmentItem.tsx
- src/components/common/CourseOverview/CourseActionsBar.tsx
- src/utils/getSemesterLabel.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: JiiminHa
Repo: 2025-snowCode/snowCode-Client PR: 2
File: src/components/Header/StudentHeader.tsx:10-10
Timestamp: 2025-07-13T00:27:31.464Z
Learning: Using TypeScript interfaces for React component props (like StudentHeaderProps) provides clear structure, improves readability, and makes components more maintainable and extensible for future development.
📚 Learning: 2025-07-16T13:05:15.174Z
Learnt from: suminb99
Repo: 2025-snowCode/snowCode-Client PR: 6
File: src/layout/Layout.tsx:34-34
Timestamp: 2025-07-16T13:05:15.174Z
Learning: The codebase uses a custom utility class `flex-center` defined in index.css that applies `flex items-center justify-center` styles.
Applied to files:
src/index.css
📚 Learning: 2025-07-13T00:27:31.464Z
Learnt from: JiiminHa
Repo: 2025-snowCode/snowCode-Client PR: 2
File: src/components/Header/StudentHeader.tsx:10-10
Timestamp: 2025-07-13T00:27:31.464Z
Learning: Using TypeScript interfaces for React component props (like StudentHeaderProps) provides clear structure, improves readability, and makes components more maintainable and extensible for future development.
Applied to files:
src/pages/course-overview/ui/UnitItem.tsxsrc/components/admin/assignments/CourseSelector.tsxsrc/components/admin/assignments/AssignmentCard.tsxsrc/models/course.tssrc/pages/course-overview/ui/CourseHero.tsxsrc/components/common/Dashboard/CourseCard.tsx
📚 Learning: 2025-07-16T13:14:38.834Z
Learnt from: suminb99
Repo: 2025-snowCode/snowCode-Client PR: 6
File: src/assets/svg/ChatIcon.tsx:14-14
Timestamp: 2025-07-16T13:14:38.834Z
Learning: In this project, the svgr workflow requires placing original SVG files in the `public/svg` folder first, then running `npm run svgr` command to generate converted TSX files in the `src/assets/svg` folder according to the root configuration.
Applied to files:
src/components/admin/assignments/AssignmentCard.tsxsrc/layout/Layout.tsx
🧬 Code graph analysis (16)
src/components/admin/form/LabeledDropdown.tsx (2)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/assets/svg/Correct.tsx (1)
props(3-11)
src/components/common/Dashboard/CourseList.tsx (4)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/components/common/IconButton.tsx (1)
IconButtonProps(7-9)src/assets/svg/Correct.tsx (1)
props(3-11)src/assets/svg/Incorrect.tsx (1)
props(3-16)
src/pages/course-overview/ui/AssignmentList.tsx (4)
src/components/common/CourseOverview/AssignmentList.tsx (3)
AssignmentListProps(9-24)assignment(12-21)AssignmentListProps(4-7)src/components/common/CourseOverview/AssignmentItem.tsx (2)
AssignmentItemProps(9-45)AssignmentItemProps(4-7)src/components/admin/assignments/AssignmentList.tsx (2)
AssignmentListProps(4-8)assignment(30-37)src/components/common/CourseOverview/Unit.tsx (1)
unit(8-24)
src/pages/course-overview/ui/CourseActionsBar.tsx (5)
src/components/common/CourseOverview/CourseActionsBar.tsx (3)
CourseActionsBarProps(10-23)CourseActionsBarProps(4-8)navigate(13-15)src/components/common/CourseOverview/EmptyCourse.tsx (1)
div(4-17)src/components/common/CourseOverview/CourseHero.tsx (1)
CoursHeroProps(19-59)src/pages/admin/courses/CourseCreatePage.tsx (1)
AssignmentFormLayout(6-54)src/components/common/CourseOverview/CourseStat.tsx (1)
CourseStatProps(7-19)
src/pages/course-overview/models/response.ts (2)
src/models/course.ts (1)
CourseOverviewResponse(44-44)src/components/common/CourseOverview/dummy/types.ts (2)
CourseResponse(28-31)CourseInfo(17-26)
src/components/admin/assignments/CourseSelector.tsx (3)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/components/common/IconButton.tsx (1)
IconButtonProps(7-9)src/assets/svg/Incorrect.tsx (1)
props(3-16)
src/pages/course-overview/ui/CourseContent.tsx (2)
src/models/course.ts (1)
CourseOverview(39-42)src/App.tsx (1)
UserTypeContext(15-15)
src/components/admin/assignments/AssignmentCard.tsx (4)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/assets/svg/Incorrect.tsx (1)
props(3-16)src/assets/svg/Correct.tsx (1)
props(3-11)src/components/common/IconButton.tsx (1)
IconButtonProps(7-9)
src/models/course.ts (5)
src/models/common.ts (1)
ApiResponse(5-8)src/components/admin/assignments/dummy/types.ts (3)
Course(10-18)CoursesResponse(20-26)Assignment(4-8)src/components/common/Dashboard/types.ts (1)
Course(19-28)src/components/common/CourseOverview/CourseHero.tsx (1)
CoursHeroProps(8-17)src/components/common/CourseOverview/CourseContent.tsx (1)
CourseContentProps(4-6)
src/components/common/Badge.tsx (3)
src/components/common/CourseOverview/SubmissionBadge.tsx (2)
SubmissionBadgeProps(31-42)SubmissionBadgeProps(6-8)src/components/common/CourseOverview/AssignmentItem.tsx (1)
AssignmentItemProps(9-45)src/components/common/Dashboard/ScheduleCard.tsx (1)
ScheduleCardProps(7-28)
src/layout/Layout.tsx (2)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/assets/svg/Correct.tsx (1)
props(3-11)
src/utils/course.ts (1)
src/models/course.ts (1)
Unit(15-23)
src/pages/admin/assignments/AssignmentCreatePage.tsx (4)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/assets/svg/Correct.tsx (1)
props(3-11)src/assets/svg/Incorrect.tsx (1)
props(3-16)src/components/common/IconButton.tsx (2)
IconButtonProps(7-9)IconButtonProps(3-5)
src/App.tsx (2)
src/models/common.ts (1)
UserType(10-10)src/components/common/CourseOverview/CourseHero.tsx (1)
CoursHeroProps(19-59)
src/pages/admin/student/studentManagementPage.tsx (3)
src/assets/svg/Unsubmitted.tsx (1)
props(3-18)src/assets/svg/Correct.tsx (1)
props(3-11)src/assets/svg/Incorrect.tsx (1)
props(3-16)
src/components/common/Dashboard/CourseCard.tsx (1)
src/utils/course.ts (1)
formatSemester(10-13)
🔇 Additional comments (35)
src/pages/common/UserIdInputPage.tsx (1)
4-4: LGTM!The import path normalization to use the
@/alias is consistent with the PR's goal of standardizing imports across the codebase. The?reactsuffix correctly leverages Vite's SVG-to-React-component transformation.src/components/admin/assignments/AssignmentPageLayout.tsx (1)
5-5: LGTM!Import path updated to use the
@/alias, consistent with the project-wide SVG import normalization.src/components/admin/assignments/AssignmentCard.tsx (1)
2-5: LGTM!All four SVG imports consistently updated to use the
@/assets/svg/alias path with the?reactsuffix for SVGR.src/pages/admin/student/studentManagementPage.tsx (1)
2-2: LGTM!Search SVG import updated to use the
@/alias, aligning with the project-wide asset path normalization.src/pages/common/LandingPage.tsx (1)
4-8: LGTM!Import paths updated consistently to use the
@/alias. The distinction between regular image imports (lines 4-7) and the React component import with?reactsuffix (line 8) is correct based on their respective usages.src/components/common/Dashboard/CourseList.tsx (1)
3-3: LGTM!AddIcon import updated to use the
@/assets/svg/alias path, consistent with the project-wide refactor.src/components/admin/assignments/CourseSelector.tsx (1)
2-2: LGTM!The import path migration from root-relative to the
@/assets/svg/alias is consistent with the broader codebase refactoring in this PR.src/components/admin/form/FileUpload.tsx (1)
3-3: LGTM!The SVG import path update to
@/assets/svg/alias is consistent with the project-wide asset import standardization.src/components/admin/form/LabeledDropdown.tsx (1)
2-2: LGTM!The SVG import path migration to
@/assets/svg/alias maintains consistency with the updated import conventions across the codebase.src/models/common.ts (1)
10-10: LGTM!The
UserTypedefinition provides good type safety for user roles and aligns well with the newUserTypeContextintroduced in this PR. Based on learnings, using TypeScript types for component props and context values improves maintainability.src/pages/admin/assignments/AssignmentCreatePage.tsx (1)
7-7: LGTM!The import path update from barrel export to direct asset path aligns with the project's migration to explicit SVG imports.
src/layout/Layout.tsx (1)
4-9: LGTM! Import consolidation aligns with PR objectives.The SVG imports correctly use the
?reactsuffix for Vite's svgr plugin, and theUserTypeimport from@/models/commonproperly centralizes the type definition. This eliminates the local type alias and supports the broader UserTypeContext pattern introduced in this PR.src/components/common/Badge.tsx (2)
2-4: LGTM! SVG imports updated consistently.The imports correctly use the
?reactsuffix for direct SVG-to-React-component transformation, maintaining consistency with the project's asset import pattern.
23-23: Style enhancement approved.Adding
text-smto the submission badge base class improves text sizing consistency for the badge component.src/models/course.ts (1)
1-44: Good type centralization for course domain.This establishes a well-structured type hierarchy with
BaseCoursefor common fields andCourseOverviewextending it for detailed views. The use ofApiResponse<T>for the response type promotes consistency.src/pages/course-overview/models/response.ts (1)
1-3: Import consolidation approved.The switch to
CourseOverviewResponsefrom@/models/courseproperly aligns this mock data with the centralized type definitions.src/utils/course.ts (2)
1-21: Clean utility functions with good type safety.The helper functions are well-structured, pure, and properly typed. The
SEMESTER_MAPprovides type-safe mapping fromSemesterCodeto localized labels.
19-21: VerifyassignmentCountreflects actual data.
getTotalAssignmentCountsums theassignmentCountfield rather thanassignments.length. If these values can differ (as seen in the mock data whereassignmentCount: 2but 4 assignments exist), this could produce incorrect totals. Ensure this is intentional or consider using the actual array length.src/index.css (1)
65-67: LGTM!The new
bg-radial-gradientutility consolidates the radial gradient styling into a reusable class, following the same pattern as the existingflex-centerutility. This improves consistency and reduces duplication across components likeCourseHero.src/components/common/Dashboard/CourseCard.tsx (1)
2-4: LGTM on import updates.The alias path for
EllipsisIconand the migration toformatSemesterfrom the centralized@/utils/coursemodule align with the PR's refactoring goals.src/pages/course-overview/CourseOverviewPage.tsx (1)
12-13: Static mock data usage noted.The component uses static
courseResponsedata. Ensure this is intentional for the current phase and that real data fetching (e.g., via React Query or useEffect) is planned.src/App.tsx (2)
15-15: LGTM on UserTypeContext setup.Exporting the context with a sensible default value (
'guest') enables proper type inference and fallback behavior. Components can now consume user type viauseContext(UserTypeContext).
17-54: Good architectural pattern for context provision.The
AppContentcomponent correctly wraps the routes withUserTypeContext.ProviderinsideBrowserRouter, ensuringuseLocationis available for derivinguserType. This cleanly separates routing setup from context provision.src/pages/course-overview/ui/CourseContent.tsx (2)
8-11: Props interface follows best practices.Good use of indexed access type
CourseOverview['units']for type safety, maintaining consistency with the model definition. Based on learnings, using TypeScript interfaces for component props improves maintainability and extensibility.
28-42: EmptyCourse component correctly leverages UserTypeContext.Clean implementation that uses context to conditionally render the admin-specific
CourseActionsBar. The UI structure with semantic elements and proper styling is well-organized.src/pages/course-overview/ui/UnitItem.tsx (3)
7-9: Interface extends Unit type correctly.Good use of interface extension to add the
indexprop while inheriting allUnitproperties. This aligns with TypeScript best practices for component props.
11-26: Clean component structure with proper defaults.Good use of nullish coalescing (
??) on Line 12 to defaultisOpentotrue. The 1-based index conversion for display is handled correctly.
29-32: UnitHeader type definition and implementation are well-structured.Good use of
Omitto createUnitHeaderPropsfromUnitProps, ensuring only relevant props are passed. The component cleanly separates concerns between left (index/title/lock) and right (deadline) sections.Also applies to: 34-67
src/pages/course-overview/ui/AssignmentList.tsx (3)
5-8: Interface is well-defined with clear props.Clean interface definition following TypeScript best practices.
10-24: Good use of semantic HTML with<ul>element.Using semantic list elements (
<ul>/<li>) and Tailwind'sdivide-yutility for separators is a clean approach that improves accessibility.
38-41: Conditional styling and badge rendering logic is correct.The
isLockedclass application and conditionalBadgerendering whenisOpenis true follows the expected behavior.Also applies to: 62-70
src/pages/course-overview/ui/CourseHero.tsx (4)
9-13: Props interface is well-structured.Good use of
Omit<CourseOverview, 'units'>to derive the courseData type, ensuring type safety while excluding irrelevant properties.
23-47: Component structure and conditional rendering are well-implemented.Clean composition with
CourseInfoandCourseStatssubcomponents. The admin-specificCourseActionsBaris correctly conditionally rendered based on bothisAdminandisActiveCoursestates.
50-66: CourseInfo component is clean and semantic.Good use of
Pickfor type-safe props, and proper semantic HTML structure with<article>and<h1>.
68-90: CourseStats logic handles admin-specific display correctly.The conditional
studentInfostring construction cleanly handles the admin-only student count display with proper undefined check.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export interface Assignment { | ||
| id: number; | ||
| title: string; | ||
| submittedStatus: SubmissionStatus; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all Assignment interface definitions
ast-grep --pattern 'interface Assignment {
$$$
}'Repository: 2025-snowCode/snowCode-Client
Length of output: 996
🏁 Script executed:
#!/bin/bash
# Also search more broadly for Assignment interface
rg 'interface Assignment' -A 5Repository: 2025-snowCode/snowCode-Client
Length of output: 3318
🏁 Script executed:
#!/bin/bash
# Check if the specific file mentioned exists and read it
if [ -f "src/components/admin/assignments/dummy/types.ts" ]; then
echo "=== File: src/components/admin/assignments/dummy/types.ts ==="
cat -n "src/components/admin/assignments/dummy/types.ts"
else
echo "File src/components/admin/assignments/dummy/types.ts does not exist"
fiRepository: 2025-snowCode/snowCode-Client
Length of output: 965
🏁 Script executed:
#!/bin/bash
# Check the current state of src/models/course.ts
if [ -f "src/models/course.ts" ]; then
echo "=== File: src/models/course.ts ==="
cat -n "src/models/course.ts"
else
echo "File src/models/course.ts does not exist"
fiRepository: 2025-snowCode/snowCode-Client
Length of output: 1370
Align submittedStatus optionality across Assignment interfaces.
The Assignment interface in src/models/course.ts (line 11) requires submittedStatus, but src/components/admin/assignments/dummy/types.ts (line 7) defines the same interface with an optional submittedStatus? field. This inconsistency can cause type mismatches when data from different sources is consumed by the same code.
🤖 Prompt for AI Agents
In `@src/models/course.ts` around lines 8 - 12, The Assignment interface requires
submittedStatus but a duplicate/interface variant in components uses
submittedStatus? which causes type mismatch; update the canonical Assignment
definition (the exported Assignment interface) to make submittedStatus optional
(submittedStatus?) or unify both by importing a single shared type (e.g.,
export/import the Assignment type) so both src/models/course.ts and
src/components/admin/assignments/dummy/types.ts reference the same definition;
adjust any call sites that assume submittedStatus is always present to handle
undefined or provide defaults.
| const AssignmentItem = ({ | ||
| title, | ||
| index, | ||
| submittedStatus, | ||
| isOpen, | ||
| }: AssignmentItemProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link uses display index instead of assignment id for routing.
The Link navigates to /assignments/${index} where index is the 1-based display position. This will produce incorrect routes (e.g., /assignments/1, /assignments/2) instead of using the actual assignment identifier. The id field from Assignment should be used for routing.
🐛 Proposed fix
const AssignmentItem = ({
+ id,
title,
index,
submittedStatus,
isOpen,
}: AssignmentItemProps) => {
const isLocked = isOpen === false ? 'opacity-60 pointer-events-none' : '';
return (
<li className={`w-full p-4 ${isLocked}`}>
{/* 좌측: 인덱스, 문제명 */}
<div className='max-w-185 mx-auto flex items-center justify-between'>
<div className='flex items-center gap-7.5 flex-1'>
<span className='w-7.5 h-7.5 shrink-0 flex-center rounded-full border border-purple-stroke text-base text-light-black font-medium'>
{index}
</span>
{isOpen ? (
- <Link to={`/assignments/${index}`} className='min-w-0'>
+ <Link to={`/assignments/${id}`} className='min-w-0'>
<p className='truncate text-secondary-black text-base font-normal hover:text-primary hover:underline hover:underline-offset-4 cursor-pointer'>
{title}
</p>
</Link>Also applies to: 49-54
🤖 Prompt for AI Agents
In `@src/pages/course-overview/ui/AssignmentList.tsx` around lines 32 - 37, The
Link in the AssignmentItem component is using the display index for routing
instead of the assignment's real identifier; update the AssignmentItemProps and
the AssignmentItem signature to accept the assignment id (e.g., id: string |
number) and replace any occurrences of `/assignments/${index}` with
`/assignments/${id}` (also update the other Link instance mentioned around lines
49–54 to use id as well) so routes use the actual Assignment.id rather than the
1-based display index.
| <Link to=''> | ||
| <Button color={isActiveCourse ? 'outlineWhite' : 'outlinePurple'}> | ||
| 학생 목록 | ||
| </Button> | ||
| </Link> | ||
| <Link to=''> | ||
| <Button color={isActiveCourse ? 'outlinePurple' : 'primary'}> | ||
| 단원 추가 | ||
| </Button> | ||
| </Link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty to attributes in Link components.
Both Link components have empty to='' destinations. This creates non-functional links that navigate to the current page. The previous implementation in src/components/common/CourseOverview/CourseActionsBar.tsx had actual navigation logic (e.g., to /admin/student?course=...).
If these are placeholders, consider adding a TODO comment or implementing the actual routes.
💡 Example with placeholder routes
- <Link to=''>
+ <Link to='students'> {/* TODO: Implement student list route */}
<Button color={isActiveCourse ? 'outlineWhite' : 'outlinePurple'}>
학생 목록
</Button>
</Link>
- <Link to=''>
+ <Link to='units/create'> {/* TODO: Implement unit creation route */}
<Button color={isActiveCourse ? 'outlinePurple' : 'primary'}>
단원 추가
</Button>
</Link>📝 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.
| <Link to=''> | |
| <Button color={isActiveCourse ? 'outlineWhite' : 'outlinePurple'}> | |
| 학생 목록 | |
| </Button> | |
| </Link> | |
| <Link to=''> | |
| <Button color={isActiveCourse ? 'outlinePurple' : 'primary'}> | |
| 단원 추가 | |
| </Button> | |
| </Link> | |
| <Link to='students'> {/* TODO: Implement student list route */} | |
| <Button color={isActiveCourse ? 'outlineWhite' : 'outlinePurple'}> | |
| 학생 목록 | |
| </Button> | |
| </Link> | |
| <Link to='units/create'> {/* TODO: Implement unit creation route */} | |
| <Button color={isActiveCourse ? 'outlinePurple' : 'primary'}> | |
| 단원 추가 | |
| </Button> | |
| </Link> |
🤖 Prompt for AI Agents
In `@src/pages/course-overview/ui/CourseActionsBar.tsx` around lines 8 - 17, The
two Link components in CourseActionsBar.tsx currently have empty to='' props,
making the links non-functional; update the Link destinations to real routes
(e.g., use the previous pattern such as '/admin/student?course={courseId}' for
the "학생 목록" Link and a route like '/admin/course/{courseId}/add-unit' or similar
for the "단원 추가" Link) or, if you truly intend placeholders, add a clear TODO
comment and a non-empty stub route (e.g., '/todo/course-actions') so navigation
is explicit; locate the Link elements around the Button components that
reference isActiveCourse to apply the changes.
JiiminHa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다🤍
⚙️ Related ISSUE Number
Related #27
📄 Work Description
사용자 역할 전역 상태 관리 (Context)
강의 상세(course-overview) 컴포넌트 리팩터링
스타일 및 UI 수정
bg-radial-gradient유틸리티 클래스 추가유틸 함수 및 포맷터 추가
타입 · 모델 구조 정리
📷 Screenshot
💬 To Reviewers
App컴포넌트에서UserTypeContext를 정의하고, 라우팅 영역 하위 컴포넌트에Provider를 적용했습니다.useContext(UserTypeContext)를 통해 역할 값을 호출하여 사용하시면 됩니다!src/models/폴더에서 관리하고 있습니다.🔗 Reference
Summary by CodeRabbit
New Features
Refactor
Style
Chore
✏️ Tip: You can customize this high-level summary in your review settings.