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

Fix PR #59, fix Detail interface in Timetable #69

Merged
merged 1 commit into from
May 16, 2024

Conversation

useruseruse
Copy link
Contributor

SET_MULTIPLE_FOCUS 액션에서 사용되는 Detail interface 를 변경하였습니다.

SET_MULTIPLE_FOCUS 는 아래 5가지 경우에 dispatch 가 되고, 각 경우에 대해서 테스팅완료하였습니다!

  1. MapSubsection 의 특정 건물 위에 hover
  2. SummarySubsection 의 (a)과목 구분 (b)학점 (c)성적 위에 각각 호버하였을 때
  3. ExamSubsection 의 특정 건물 위에 hover
export function setMultipleFocus(multipleTitle: string, multipleDetails: Detail[]) {
  return {
    type: SET_MULTIPLE_FOCUS,
    multipleTitle: multipleTitle,
    multipleDetails: multipleDetails,
  };
}

아래처럼 lecture list(Lecture[])를 구한뒤, 이를 매핑하여 Detail 로 사용하기 때문에 name 이 있는데, lecture 이 undefined인 경우는 없어, detail interface 에서 lecture의 optional 을 제거하였습니다.
image

@useruseruse useruseruse requested a review from yumincho May 15, 2024 07:38
@useruseruse useruseruse self-assigned this May 15, 2024
Copy link

codecov bot commented May 15, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 4.47%. Comparing base (9318319) to head (8bc0a59).

Files Patch % Lines
src/actions/timetable/lectureFocus.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           migration     #69   +/-   ##
=========================================
  Coverage       4.47%   4.47%           
=========================================
  Files            211     211           
  Lines           6547    6547           
  Branches        1640    1640           
=========================================
  Hits             293     293           
  Misses          6158    6158           
  Partials          96      96           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@yumincho yumincho left a comment

Choose a reason for hiding this comment

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

슬랙에서 논의한 그대로 잘 반영된 것 같습니다 👀

@yumincho yumincho changed the title fix detail interface Fix #59, fix Detail interface in Timetable May 15, 2024
@yumincho yumincho added this to the TypeScript Migration milestone May 16, 2024
@yumincho yumincho added the migrate migrate from JS CC to TS FC label May 16, 2024
@sboh1214 sboh1214 force-pushed the fix@multipleFocusDispatch branch from 982ab46 to 8bc0a59 Compare May 16, 2024 13:38
@sboh1214 sboh1214 changed the title Fix #59, fix Detail interface in Timetable Fix PR #59, fix Detail interface in Timetable May 16, 2024
Copy link
Contributor

@sboh1214 sboh1214 left a comment

Choose a reason for hiding this comment

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

Seth Meyers LGTM

@sboh1214 sboh1214 merged commit ccd274c into migration May 16, 2024
4 of 5 checks passed
@sboh1214 sboh1214 deleted the fix@multipleFocusDispatch branch May 16, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migrate migrate from JS CC to TS FC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants