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

Migrate redux reducer about planner #63

Merged
merged 10 commits into from
May 13, 2024
Merged

Conversation

useruseruse
Copy link
Contributor

@useruseruse useruseruse commented Apr 26, 2024

변경사항

  1. list.ts

    • search, basic, humanity, taken 은 필수로 있고, 전공과목들도 optional 하게 key 로 추가될 수 있음.
      이에 따라 enum 타입에 전공과목들을 추가하고 이를 key 로 사용함.
      전공과목 탭을 클릭할 경우 state 의 list의 키로 해당 전공과목이 추가되며, 불러온 리스트들이 value 로 닮김.
    • dictionary 나 timetable 에서도 해당 enum 을 사용하고 있는데 동일한 구조임.
      image
  2. planner.ts

    • 원래 initial state 의 planners가 null 이고, 컴포넌트 마운팅할 때 planner를 처음 불러오며 [] 로 초기화 시켜주는데,
      [] 로 초기화 시켜주는 것을 지우고 initial state 를 null 에서 Planner[] 로 변경,
      Planners 가 null 일 필요가 없는 것으로 확인하여 type 추론에 유리하도록 변경하였음.
      image
  3. search.ts

    • initial state 의 start: null, end: null, day: null, 사용하지 않아서 제거하였음.
  4. itemFocus.ts

    • 모든 ItemFocusState는 '@/shapes/state/planner/ItemFocus' 에 정의된 5가지 중 하나의 타입을 가짐.
      type ItemFocus = NoneItem | ListItem | AddingItem | TableItem | CategoryItem;
      새로운 state를 만들 때 이전의 state 를 복제하는 부분을 지우는 것이 의미적으로 맞는 것 같은데, 상태를 변경하기 전에 clear 를 통해 초기화해주므로 문제는 없는 것 같아 그대로 두었음. (ex. category Item을 set 할 때, 이전의 ListItem 의 속성을 가지면 안되므로...)
    image

    기타

    • planner 관련하여 잘 작동하는지 테스트 완료 하였습니다.

@useruseruse useruseruse self-assigned this Apr 26, 2024
@useruseruse useruseruse requested review from yumincho, sboh1214, jooyeongmee and Davidace136 and removed request for yumincho and sboh1214 April 26, 2024 14:00
Copy link

codecov bot commented Apr 26, 2024

Codecov Report

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

Project coverage is 4.51%. Comparing base (316d614) to head (3fd3e2b).

Files Patch % Lines
src/reducers/planner/itemFocus.ts 0.00% 23 Missing and 1 partial ⚠️
src/reducers/planner/list.ts 0.00% 15 Missing and 1 partial ⚠️
src/reducers/planner/search.ts 0.00% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           migration     #63      +/-   ##
============================================
+ Coverage       4.12%   4.51%   +0.38%     
============================================
  Files            211     210       -1     
  Lines           6547    6495      -52     
  Branches        1686    1747      +61     
============================================
+ Hits             270     293      +23     
+ Misses          6134    6026     -108     
- Partials         143     176      +33     

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

@useruseruse
Copy link
Contributor Author

더이상 리뷰 없어 주석 지우고 PR 올리겠습니다

@useruseruse useruseruse marked this pull request as ready for review May 8, 2024 13:11
@yumincho yumincho changed the title migrate reducer Migrate redux reducer about planner May 10, 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.

planner.ts 코드 원복합시다 @yumincho

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.

Dog LGTM Keyboard

@sboh1214 sboh1214 merged commit c729e98 into migration May 13, 2024
5 checks passed
@sboh1214 sboh1214 deleted the migration@reducers/planner branch May 13, 2024 16:49
@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
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