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

타임서비스 이름 변경 및 타임테이블 모달 내부 요소 추가 #199

Merged
merged 8 commits into from
Aug 15, 2024

Conversation

SWARVY
Copy link
Contributor

@SWARVY SWARVY commented Aug 14, 2024

Summary

#97 TimeTable Page 개발

  • 타임테이블 모달 창 내부의 요소를 추가했습니다.
  • 타임 서비스의 이름을 변경했습니다. (time -> kgu-plus)

Tasks

  • 모달 내부에서 수원/서울 캠퍼스, 강의 구분(전공, 교양 등..)을 선택할 수 있도록 필터를 추가했습니다.
  • 검색 섹션도 추가하였으나 해당 기능은 아직 내부 기능까지 모두 구현되지는 않았습니다.
  • 필터 요소들을 모두 다중선택할 수 있도록 변경하였습니다. 또한, 선택된 필터를 한번 더 눌렀을 때 비활성화하도록 하였습니다.
  • 테이블 색션도 추가하였으나, 백엔드간의 연동이 아직 되어있지 않아 해당 부분은 백엔드 스테이징 서버 작업 이후 연동 예정입니다.

ETC

  • 모달 컴포넌트의 경우 수정을 거친 이후 Clab Design-system에 추가할 예정입니다.

Screenshot

image

@SWARVY SWARVY added ✨ Feature 새로운 기능 명세 및 개발 ⏰ Time time 프로젝트 관련 labels Aug 14, 2024
@SWARVY SWARVY requested a review from gwansikk August 14, 2024 14:24
@SWARVY SWARVY self-assigned this Aug 14, 2024
@SWARVY SWARVY requested a review from Jeong-Ag as a code owner August 14, 2024 14:24
Copy link

changeset-bot bot commented Aug 14, 2024

⚠️ No Changeset found

Latest commit: a214184

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@gwansikk
Copy link
Member

@SWARVY conflicts 확인 부탁드려요

@gwansikk gwansikk marked this pull request as ready for review August 14, 2024 15:14
Copy link
Member

@gwansikk gwansikk left a comment

Choose a reason for hiding this comment

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

LGTM 👍👍

Copy link
Contributor

@Jeong-Ag Jeong-Ag 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 +273 to +284
const [selectedRegion, setSelectedRegion] = useState<Region[]>([
REGION.campus1,
REGION.campus2,
]);
const [selectedGrade, setSelectedGrade] = useState<Grade[]>([]);
const [selectedDay, setSelectedDay] = useState<DayKor[]>([day]);
const [selectedPeriod, setSelectedPeriod] = useState<
DayPeriod[] | NightPeriod[]
>([period]);
const [selectedLectureType, setSelectedLectureType] = useState<LectureKey[]>(
[],
);
Copy link
Contributor

Choose a reason for hiding this comment

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

useState 하나로 묶어서 관리하기엔 어려움이 있을까요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그냥 객체로 수정하면 안될 것 같고, useReducer를 사용하여 state를 관리하는 방법에 대해서 고민해보도록 하겠습니다 :)

Copy link
Member

Choose a reason for hiding this comment

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

해당 State는 복잡성이 높기 때문에useReducer로 구현할 경우 코드량이 더욱 많아지고 이해하기 힘들 수도 있을 거 같아요, 현재 상태로 유지하다가 추후에 더 복잡성을 가지게 될 때 리팩토링을 하는 방향도 좋을 거 같습니다

@gwansikk
Copy link
Member

@Jeong-Ag @SWARVY 큰 문제는 없어 해당 PR은 머지하겠습니다~

@gwansikk gwansikk merged commit 3dc3761 into main Aug 15, 2024
4 checks passed
@gwansikk gwansikk deleted the feature/#97 branch August 15, 2024 07:01
@SWARVY SWARVY mentioned this pull request Aug 15, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 새로운 기능 명세 및 개발 ⏰ Time time 프로젝트 관련
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants