Skip to content
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

[졸업학점계산기] 강의 테이블 및 교양 영역 컴포넌트 UI 생성 #651

Merged

Conversation

Yejin0070
Copy link
Contributor

What is this PR? 🔍

  • 기능
  1. 강의 테이블 및 교양 영역 컴포넌트 UI 생성
  2. 강의정보 불러오기
  3. 강의 추가/삭제 버튼 활성화 (추가하기 버튼 클릭 시 정규 과목 추가 페이지로 이동)

ScreenShot 📷

image image

✔️ Please check if the PR fulfills these requirements

  • It's submitted to the correct branch, not the develop branch unconditionally?
  • If on a hotfix branch, ensure it targets main?
  • There are no warning message when you run yarn lint

@Yejin0070 Yejin0070 self-assigned this Feb 5, 2025
@Yejin0070 Yejin0070 added the ✨ Feature 기능 개발 label Feb 5, 2025
Copy link
Contributor

@Gwak-Seungju Gwak-Seungju left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~! 테이블 태그를 잘 사용하신 것 같네요!

Comment on lines 65 to 69
<td className="class_title" align="center">{lecture.class_title}</td>
<td align="center">{lecture.professor}</td>
<td align="center">{lecture.grades}</td>
<td className="course_type" align="center">{}</td>
<td align="center">
Copy link
Contributor

Choose a reason for hiding this comment

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

className에서 _으로 단어를 구분 짓기보다는 -로 단어를 구분 지어요.
또한 -로 단어를 구분 짓기 때문에 BEM에서 modifier는 --로 구분하는 게 좋을 것 같아요! 다른 부분에서 modifier를 -로 구분 짓고 있더라고요..!

Copy link
Contributor Author

@Yejin0070 Yejin0070 Feb 7, 2025

Choose a reason for hiding this comment

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

엇 임의로 값을 표시해두기 위해 적어둔 것인데 삭제하는 것을 깜빡한 것 같습니다..! 수정하였습니다..!

31b4bf5

className={styles.table__trigger}
onClick={onClickAddLecture}
>
강의 추가하기
Copy link
Contributor

Choose a reason for hiding this comment

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

시간표 수정하기로 명칭이 바뀌었습니다! 피그마 확인해주시면 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

31b4bf5

수정하였습니다 :)

<tbody className={styles.table__body}>
{filteredMyLectures.map((lecture: MyLectureInfo) => (
<tr key={lecture.id} className={styles['table__body-row']}>
<td className="class_title" align="center">{lecture.class_title}</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

피그마 '제작 중'에 나온 명세인데, 과목명 12글자 이상일 경우 말줄임표를 표시하기로 했습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

과목명 최대 10글자, 교수명 최대 4글자로 그 범위 넘어갈 경우 말줄임표를 표시하도록 하였습니다!

797323f

<SemesterList />
</div>
<div className={styles.table}>
<table className={styles.table__content}>
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 table 코드는 다른 컴포넌트에서도 사용할 것 같아서 table 부분만 컴포넌트화하는 건 어떨까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

추가로 스크롤바가 나오면 요소들이 왼쪽으로 밀립니다! 디자이너분과 상의를 해서 스크롤바를 없앨지 위치를 옮길지 정해야될 것 같네요.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스크롤바를 제거하는 방향으로 반영하였습니다!

31b4bf5

@Yejin0070 Yejin0070 merged commit ce2ad30 into feat/graduation-calculator Feb 7, 2025
@github-actions github-actions bot deleted the feat/#633/lecture-list-component branch February 7, 2025 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 기능 개발
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants