Skip to content

Conversation

@dioo1461
Copy link
Contributor

@dioo1461 dioo1461 commented Mar 16, 2025

#️⃣ 연관된 이슈>

📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • 확정되지 않은 일정 섹션의 상태를 jotai로 이전하여, 홈 화면에서 다른 페이지에 갔다 와도 원래 보고 있던 화면이 유지되도록 구현했습니다.
2025-03-17.4.20.31.mov

🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

Summary by CodeRabbit

  • Refactor

    • Updated shared schedule features to use a new type name for attending status, improving type clarity across schedule-related screens.
    • Migrated segment selection and pagination state in ongoing schedules to a shared global state, enabling consistent experience and state persistence across components.
    • Adjusted segment control components to use a new prop for initial value selection.
  • Style

    • Minor formatting and code style improvements for better readability.

@dioo1461 dioo1461 requested a review from hamo-o as a code owner March 16, 2025 19:26
@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2025

Walkthrough

This change refactors type names and parameter annotations from AttendType to OngoingScheduleAttendType across the shared schedule modules, updates the state management for ongoing schedule UI components to use Jotai atoms for shared/global state, and adjusts prop names from defaultValue to initialValue in segment controls. No logic or control flow is otherwise altered.

Changes

File(s) Change Summary
.../discussion/ui/DiscussionRank/index.tsx Changed SegmentControl prop from defaultValue to initialValue.
.../shared-schedule/api/index.ts
.../api/keys.ts
.../api/prefetch.ts
.../api/queries.ts
.../api/queryOptions.ts
.../model/ongoingSchedules.ts
Renamed type AttendType to OngoingScheduleAttendType and updated all relevant parameter and import types.
.../shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx Replaced local state for page and selection with Jotai atom-based global state management.
.../shared-schedule/ui/OngoingSchedules/index.tsx Switched segment control state from local to Jotai atom, updated types and formatting, and used initialValue prop.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant OngoingSchedules
    participant JotaiAtom
    participant OngoingScheduleList
    participant useOngoingQuery

    User->>OngoingSchedules: Selects segment in SegmentControl
    OngoingSchedules->>JotaiAtom: Updates segment value atom
    OngoingSchedules->>OngoingScheduleList: Renders with atom state
    OngoingScheduleList->>JotaiAtom: Reads (and updates) page/index state
    OngoingScheduleList->>useOngoingQuery: Fetches data using atom state
    useOngoingQuery-->>OngoingScheduleList: Returns schedule data
    OngoingScheduleList-->>User: Displays updated schedule list
Loading

Possibly related PRs

  • #192: Refactors SegmentControl to support content/context-based state and introduces SegmentControl.Content, related to this PR's prop and usage changes.
  • #213: Refactors SegmentControl to use segmentOptions objects and updates prop from defaultValue to initialValue, closely matching this PR's changes.
  • #280: Modifies OngoingScheduleList component's state to reset selected index on page changes using local state, related to this PR's global state refactor using Jotai atoms.

Poem

A bunny hopped through fields of code,
Renaming types along its road.
Atoms now share state with glee,
Segment controls set properly.
With every hop, the UI’s bright—
Schedules sorted left and right!
🐰✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f6d8889 and 18be1ad.

📒 Files selected for processing (1)
  • frontend/apps/client/src/features/shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/apps/client/src/features/shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx
✨ 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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dioo1461 dioo1461 self-assigned this Mar 16, 2025
@dioo1461 dioo1461 added the 🖥️ FE Frontend label Mar 16, 2025
@dioo1461 dioo1461 changed the title Feature/fe/persist ongoing schedule state [Feat-FE] 확정되지 않은 일정 섹션의 상태를 jotai로 이전 Mar 17, 2025
@dioo1461 dioo1461 changed the title [Feat-FE] 확정되지 않은 일정 섹션의 상태를 jotai로 이전 [FE-Feat] 확정되지 않은 일정 섹션의 상태를 jotai로 이전 Mar 17, 2025
Copy link
Contributor

@hamo-o hamo-o 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 58 to 65
Copy link
Contributor

Choose a reason for hiding this comment

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

p1; 페이지 넘긴 뒤 세그먼트 바꾸면 데이터가 안나옵니다!!

default.mov

page 역시 아톰으로 관리하면서 세그먼트 변환 시 유지되어 2페이지(없는 페이지)가 요청되는 것 같아요. hover했을때(prefetch)와 다른 페이지 요청이 가네요

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

헉 고쳐보겠습니다

Copy link
Contributor Author

@dioo1461 dioo1461 Mar 19, 2025

Choose a reason for hiding this comment

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

01b26eb
qa를 제대로 안했네요 죄송.... 버그는 수정했습니다!

추가로 segment control의 value를 외부에서 주입하도록 구현 변경하는 커밋까지 작업한 후에 다시 pr open하겠습니다~

@dioo1461 dioo1461 marked this pull request as draft March 19, 2025 06:47
@dioo1461 dioo1461 force-pushed the feature/fe/persist-ongoing-schedule-state branch from 01b26eb to f6d8889 Compare June 27, 2025 11:57
@dioo1461 dioo1461 marked this pull request as ready for review June 27, 2025 11:58
@dioo1461
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Jun 27, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@dioo1461
Copy link
Contributor Author

@coderabbitai full-review

@coderabbitai
Copy link

coderabbitai bot commented Jun 29, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@dioo1461 전체 리뷰를 시작하겠습니다. 변경된 파일들을 자세히 살펴보겠습니다.


🏁 Script executed:

#!/bin/bash
# PR에서 변경된 주요 파일들을 확인
echo "=== OngoingScheduleList.tsx 확인 ==="
cat "frontend/apps/client/src/features/shared-schedule/ui/OngoingSchedules/OngoingScheduleList.tsx"

echo -e "\n=== OngoingSchedules/index.tsx 확인 ==="
cat "frontend/apps/client/src/features/shared-schedule/ui/OngoingSchedules/index.tsx"

echo -e "\n=== Jotai atom 정의 확인 ==="
fd -e ts -e tsx . | xargs grep -l "ongoingSegmentAtomFamily\|ongoingSegmentValueAtom" | head -5

Length of output: 6440


🏁 Script executed:

#!/bin/bash
# Jotai atom 정의 확인
echo "=== Atom 정의 확인 ==="
cat "frontend/apps/client/src/store/home/index.ts"

echo -e "\n=== 타입 정의 변경 확인 ==="
cat "frontend/apps/client/src/features/shared-schedule/model/ongoingSchedules.ts"

echo -e "\n=== 기타 변경된 API 파일들 확인 ==="
echo "--- API index.ts ---"
cat "frontend/apps/client/src/features/shared-schedule/api/index.ts"

Length of output: 3878


🔍 전체 리뷰 결과

✨ 주요 변경사항 요약

이 PR은 확정되지 않은 일정 섹션의 상태를 Jotai로 이전하는 목표를 성공적으로 달성했습니다. 주요 변경사항은 다음과 같습니다:

  1. 타입 이름 개선: AttendTypeOngoingScheduleAttendType
  2. 전역 상태 관리 도입: Jotai atom을 사용한 상태 관리
  3. 컴포넌트 prop 업데이트: defaultValueinitialValue

🎯 PR 목표 달성도: ✅ 완료

사용자가 홈 화면에서 다른 페이지로 이동했다가 돌아와도 일정 섹션의 상태가 유지되도록 하는 목표가 잘 구현되었습니다.

💡 코드 품질 평가

우수한 점들

  1. 명확한 타입 네이밍

    // 이전: AttendType (너무 일반적)
    // 현재: OngoingScheduleAttendType (구체적이고 명확)
  2. 효율적인 상태 관리 구조

    // 세그먼트별 독립적인 상태 관리
    export const ongoingSegmentAtomFamily = atomFamily((_: OngoingScheduleAttendType) => 
      atom({
        listIndex: 0,
        page: 1,
      } as OngoingSegmentStateAtom),
    );
  3. 일관성 있는 타입 적용

    • 모든 관련 파일에서 타입 변경이 누락 없이 적용됨
  4. 깔끔한 상태 업데이트 로직

    const setPage = (page: number) => setSegmentAtom((prev) => ({ ...prev, page }));
    const setListIndex = (index: number) => setSegmentAtom((prev) => ({ ...prev, listIndex: index }));

🔧 개선 제안사항

  1. 하드코딩된 쿼리 매개변수

    // OngoingSchedules/index.tsx의 Content 컴포넌트에서
    const { data, isPending } = useOngoingQuery(1, 6, 'ALL');

    이 부분이 여전히 하드코딩되어 있어 atom 상태와 일치하지 않을 수 있습니다.

  2. 사용하지 않는 atom 정리

    // store/home/index.ts에서
    export const ongoingListIndexAtom = atom(0); // 사용되지 않는 것 같음

🚀 성능 및 사용자 경험

  • ✅ 성능: atom family 사용으로 불필요한 리렌더링 방지
  • ✅ UX: 페이지 이동 후 돌아와도 상태 유지로 사용자 경험 개선
  • ✅ 코드 유지보수성: 전역 상태로 관리하여 상태 추적이 용이

📋 테스트 제안

다음 시나리오들을 테스트해보시기를 권장합니다:

  1. 세그먼트 변경 → 다른 페이지 이동 → 홈으로 복귀 시 상태 유지 확인
  2. 페이지네이션 → 다른 페이지 이동 → 홈으로 복귀 시 페이지 상태 유지 확인
  3. 리스트 아이템 선택 → 다른 페이지 이동 → 홈으로 복귀 시 선택 상태 유지 확인

🎉 전체 평가

이 PR은 목표를 성공적으로 달성했으며, 코드 품질도 우수합니다. Jotai를 사용한 상태 관리 패턴이 잘 구현되어 있고, 타입 안전성도 개선되었습니다. 몇 가지 작은 개선사항들이 있지만, 전반적으로 훌륭한 구현입니다.

추천: 머지 승인 ✅


🧠 Learnings used
Learnt from: dioo1461
PR: softeer5th/Team4-enDolphin#53
File: frontend/src/components/Input/MultiInput.tsx:50-59
Timestamp: 2025-02-04T03:10:35.809Z
Learning: User dioo1461 prefers to receive code reviews and responses in Korean language.

Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#101
File: frontend/src/routes/my-schedule/@components/SchedulePopover/PopoverForm.tsx:10-35
Timestamp: 2025-02-06T05:14:12.328Z
Learning: Form state management and validation for the SchedulePopover component in frontend/src/routes/my-schedule/@components/SchedulePopover/PopoverForm.tsx will be implemented in a future PR.

Copy link
Contributor

@hamo-o hamo-o left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!!

PAGE_SIZE,
value,
)}
onValueChange={(value) => setSegmentValue(value)}
Copy link
Contributor

Choose a reason for hiding this comment

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

p3;
onValueChange={setSegmentValue} 이렇게만 넘겨도 될 것 같습니당

Comment on lines +14 to 24
export interface SegmentControlProps<
SegmentValue extends string = string,
> extends PropsWithChildren {
segmentOptions: SegmentOption<SegmentValue>[];
style?: 'weak' | 'filled';
shadow?: boolean;
defaultValue?: string;
onValueChange?: (value: string) => void;
initialValue?: SegmentOption<SegmentValue>['value'];
onValueChange?: (value: SegmentOption<SegmentValue>['value']) => void;
className?: string;
onButtonHover?: (value: string) => void;
onButtonHover?: (value: SegmentOption<SegmentValue>['value']) => void;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

p3;
아래와 같이 간략화하면 어떨까요? (SegmentOption<SegmentValue>['value']가 결국에는 SegmentValue와 같아서요!)

export interface SegmentControlProps<T extends string = string> extends PropsWithChildren {
  segmentOptions: SegmentOption<T>[];
  style?: 'weak' | 'filled';
  shadow?: boolean;
  initialValue?: T;
  onValueChange?: (value: T) => void;
  className?: string;
  onButtonHover?: (value: T) => void;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아 맞네요 수정하겠습니다!!

Comment on lines +14 to +17
atom({
listIndex: 0,
page: 1,
} as OngoingSegmentStateAtom),
Copy link
Contributor

Choose a reason for hiding this comment

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

p3;
단언문보다는 제네릭 활용이 더 안정성 있을 것 같습니다!

Suggested change
atom({
listIndex: 0,
page: 1,
} as OngoingSegmentStateAtom),
atom<OngoingSegmentStateAtom>({
listIndex: 0,
page: 1,
}),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 좋습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🖥️ FE Frontend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants