Skip to content

Groups 목록, 그룹 멤버 찌르기 구현#34

Merged
devgony merged 117 commits intoprography:mainfrom
devgony:groups
Jul 24, 2025
Merged

Groups 목록, 그룹 멤버 찌르기 구현#34
devgony merged 117 commits intoprography:mainfrom
devgony:groups

Conversation

@devgony
Copy link
Collaborator

@devgony devgony commented Jul 8, 2025

resolves #39

Todo

  • Groups 목록 페이지
  • 그룹 멤버 찌르기
  • 찌르기 받은 목록 AppBar 에서 알림 페이지로 진입하여 확인
  • Login 시 Loding spinner lottie component 로 변경

Summary by CodeRabbit

  • New Features

    • Introduced group management pages, including group list, group detail, and group member pages with join, leave, and poke functionalities.
    • Added notification page with infinite scroll and mark-all-as-read support.
    • Implemented new shared Loading spinner component with animation.
    • Added new icons and enhanced notification and group-related UI components.
    • Expanded Storybook coverage for group, notification, and loading components.
  • Improvements

    • Refactored group and notification components for clearer type usage and improved navigation.
    • Centralized notification state management and updated AppBar notification handling.
    • Enhanced error handling and user feedback for group and notification actions.
  • Bug Fixes

    • Fixed notification indicator logic and group join/leave flows for better reliability.
  • Chores

    • Updated environment configuration and build scripts.
    • Added and reorganized exports for shared and group components.

Hys-Lee and others added 9 commits July 4, 2025 23:16
* feat: TodoBottomSheet 완성

비동기 동작의 책임을 외부에 넘겼습니다.

* feat: 데이터 페칭 이후 타입 변환의 책임을 비동기 훅으로 변경

비동기 훅은 도메인 별로 커스텀 가능한 것이기 때문에 변경했습니다.

* feat: goal 관련 fetching함수 제작

* feat: TodoBottomSheet 동작 수정

초기값 바뀌는 동작 외부에서 key로 제어하지 않고, 내부에서 useEffect로 제어하도록 변경함.
key로 제어하면 remount때문에 애니메이션 이상해서.

* feat: todo, goal, subGoal에 대한 fethcing과 쿼리 훅 생성

데이터 처리를 쿼리 훅 내부에서 하도록 수정.
mutate는 대부분 낙관적 업데이트로 사용하지 않기 때문에 변환 로직으로 덮어쓰지 않았음.

* feat: TodoBottomSheet활성화, 선택된 goal관련 zustand store생성

이들 데이터는 멀리 떨어진 컴포넌트 사이에서 사용되기 때문에 전역 상태 사용함.

* feat: msw모킹 위한 api 및 db 생성

* feat: 메인 페이지 goal관련 데이터 갖는 GoalCard 컴포넌트 생성 (진행중)

* feat: 메인 페이지에 필요한 컴포넌트 생성 및 수정  MainHeader, GoalMenuContainer (임시)

문구 api연결은 아직 안했고, 컴포넌트 구분만 해뒀습니다.
GoalMenuContainer는 선택에 대해 zustand로 전역 상태 처리하도록 수정했습니다.

* feat: 메인 페이지 연결을 위한 자잘한 수정

fetchTemplate는 위치 변경으로 인한 수정.
GoalMenu는 api연결에 필요한 타입 export
TodoItem은 UI가 반응형으로 동작하도록 수정

* feat: TodoList 메인페이지 api연결을 위한 수정

쿼리 훅과 zustand 사용해 동작하도록 수정함. edit버튼 클릭에 대한 동작 추가함. UI반응형으로 동작하도록 w-full로 수정함.

* fix: 바텀 시트 todo추가 동작 수정, 바텀시트 선택된 subgoal 버튼에 보이도록 수정

todo추가 동작 수정에 대해서, 기존 분기처리 잘못해 update를 실행하던 것을 create실행하도록 수정

* fix: TodoBottomSheet 상태 초기화 수정

initTodoInfo를 매번 object 새로 생성해 넘겨줘서 useEffect가 동작했음. 이에 컴폰너트 내부에서 분해해서 사용하도록 수정함.

* feat: GoalCard 모달이 존재하면 바텀시트 닫도록 수정.

이렇지 않으면 dialog가 rule로 지정된 바텀시트에 focus-trap이 걸림.

* feat: useModal 수정

바텀시트 사용할 때 모달의 사용을 알 수 있도록 isOpen을 추가함.

* feat: TodoList에서 ModalAddingSubGoal 동작 관련 연결

subgoal을 추가할 수 있도록 api제작 및 연결

* fix: 자잘한 UI 에러 수정

* feat: 페칭 및 훅 자잘한 수정

use client추가 및 타입 수정

* feat: useTodoList undefined도 가능하도록 수정.

사용하는 입장에서 undefined반환 값으로 처리 분기할 수 있도록.

* feat: cheer, point 관련 fetching 제작

goalFetching에서 불필요 부분 삭제

* feat: 모킹 구조 변경 및 points, cheer 핸들러 추가

관리하기 쉽도록 분리했습니다.

* feat: 불필요 부분 삭제

콘솔 제거 및 element정리

* feat: fetch 동작에서 헤더 설정 수정

get요청에서도 authorization등 적용되도록 수정

* feat: 목표 추가 버튼 제작

모달 클릭 후 페이지 이동은 아직 구현하지 않았습니다.
해당 페이지 구현 마친 후 라우팅 추가하겠습니다.

* feat: GoalCard와 useGoalWithSubGoalTodo대해 타입 및 props명 변경에 따른 수정

* feat: BottomTabBar Link추가로 페이지 이동할 수 있도록 함.

* feat: main페이지에 BottomTabBar추가에 따른 UI수정

(이전 커밋에서 GoalCard에서 pb추가)
TodoBottomSheet는 바텀탭바 존재에 따라 위치 변경 가능하도록 수정.
추가로,
여기서, 기본 subGoal 지정.

* feat: main page에서 bottom tab bar관련 불필요 패딩 제거.

* feat: 로컬 개발 시 msw사용하도록 수정

발표할 때 필요할 것 같아서.
발표할 때 필요할 것 같아서.

* feat: 시연 전 ui 수정사항 반영
* feat: 임시 배포에서도 msw동작하도록 수정
- Replace isClient workarounds with Next.js dynamic imports
- Apply { ssr: false } to BottomTabBar and MyPage components
- Remove unnecessary useClientOnly and useAuthState hooks
- Clean up conditional rendering logic in components
- Prevent 'Hydration failed' errors in console
- Maintain clean code without workarounds
- Create GroupEmptyState component for no groups state
- Add PendingGroupCard component for individual group cards
- Build PendingGroupList component for groups collection
- Implement GroupPage main component with AppBar integration
- Add comprehensive Storybook stories for all components
- Create app/group route with BottomTabBar integration
- Download and integrate group empty state image from Figma
- Follow component composition pattern for reusability
- Include proper TypeScript interfaces and props
- Support responsive design with Tailwind CSS
- Apply SUIT font family with 700 weight to section titles
- Update font size from 24px (text-2xl) to 20px (text-xl) per Figma spec
- Add line-height 1.2em and letter-spacing -1% for precise typography
- Improve layout with proper padding (px-4, py-4) around sections
- Update GroupPage stories to use GroupData type consistently
- Add WithJoinedGroups story for testing joined groups state
- Ensure text alignment matches design: left-aligned titles

Figma reference: node-id=54330-13373
@coderabbitai
Copy link

coderabbitai bot commented Jul 8, 2025

Walkthrough

This update introduces a comprehensive group management feature, including group list, group detail, group joining, and group member pages, along with notification handling and improved loading UI. It adds several new React components, utility functions, API hooks, and Storybook stories for group and notification-related UIs. Type definitions and prop interfaces are refactored for clarity and stronger typing. Notification and loading logic are centralized and improved, and new vector-based loading animations are included.

Changes

File(s) / Path(s) Change Summary
.env.localhost, package.json Added local development environment config; updated scripts and dependencies.
.storybook/main.ts, .storybook/nextjs-mock.ts, .storybook/preview.tsx Storybook config: added app stories, Next.js config path, navigation mocks, formatting.
api/hooks.ts, api/service.ts, api/useApiQuery.ts Added/updated API hooks for groups, notifications, health; improved error handling; updated exports.
app/group/[id]/page.tsx, app/group/[id]/member/page.tsx, app/group/join-random/page.tsx, app/group/page.tsx, app/notification/page.tsx New group detail, member, join, list, and notification pages with data fetching and navigation.
app/group/join-random/JoinRandomGroup.stories.tsx, components/group/GroupEmptyState.stories.tsx, components/group/GroupList.stories.tsx, components/group/GroupPage.stories.tsx, components/group/PendingGroupCard.stories.tsx, components/group/PendingGroupList.stories.tsx, components/group/SystemMessage.stories.tsx, components/group/GroupMemberList.stories.tsx, components/icons/UsersGroupIcon.stories.tsx, components/shared/Loading/Loading.stories.tsx, components/notification/NotificationContent.stories.tsx, components/shared/AppBar/AppBar.stories.tsx, components/mypage/EditProfile.stories.tsx, components/group/GroupChat.stories.tsx New and updated Storybook stories for group, notification, and loading components.
components/group/GroupEmptyState.tsx, components/group/GroupPage.tsx, components/group/GroupList.tsx, components/group/GroupItem.tsx, components/group/PendingGroupCard.tsx, components/group/PendingGroupList.tsx, components/group/SystemMessage.tsx, components/group/GroupMemberList.tsx, components/group/GroupChat.tsx, components/group/GroupChatItem.tsx New and refactored group-related components with improved typing, UI, and logic.
components/icons/UsersGroupIcon.tsx, components/icons/NotificationIcon.tsx, components/icons/BellIcon.tsx, components/icons/index.ts Added new icons, refactored exports, updated notification icon logic.
components/shared/AppBar/AppBar.tsx, components/shared/Button/Button.tsx, components/shared/Loading/Loading.tsx, components/shared/index.ts AppBar: internalized notification logic; Button: centered content; Loading: new animated loading component and export.
components/shared/Loading/loading.json, public/animations/loading.json Added new Lottie JSON loading animations.
components/notification/NotificationContent.tsx New notification list component with infinite scroll, mark all read, and empty state.
lib/date.ts Added date formatting utility function.
components/mypage/MyPage.tsx, app/onboarding/_components/LoginScreen.tsx, app/onboarding/page.tsx Refactored notification and loading logic, removed alert popups.
components/group/index.ts Reorganized and expanded group component exports.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant GroupPage
    participant API
    participant Router

    User->>GroupPage: Visit /group
    GroupPage->>API: Fetch joinedGroups, pendingGroups
    API-->>GroupPage: Return group data
    GroupPage->>User: Show group lists
    User->>GroupPage: Click on joined group
    GroupPage->>Router: Navigate to /group/[id]
    Router->>GroupDetailPage: Render group detail
    GroupDetailPage->>API: Fetch group detail, chat messages
    API-->>GroupDetailPage: Return group/chat data
    GroupDetailPage->>User: Show group detail and chat
    User->>GroupDetailPage: Click members
    GroupDetailPage->>Router: Navigate to /group/[id]/member
    Router->>GroupMemberPage: Render member list
    GroupMemberPage->>API: Fetch members
    API-->>GroupMemberPage: Return member data
    GroupMemberPage->>User: Show member list
    User->>GroupMemberPage: Click leave group
    GroupMemberPage->>API: Call leave group API
    API-->>GroupMemberPage: Return result
    GroupMemberPage->>User: Show toast and redirect
Loading
sequenceDiagram
    participant User
    participant NotificationPage
    participant API

    User->>NotificationPage: Visit /notification
    NotificationPage->>API: Fetch notifications (page 1)
    API-->>NotificationPage: Return notifications
    NotificationPage->>User: Show notifications
    User->>NotificationPage: Scroll to bottom
    NotificationPage->>API: Fetch notifications (next page)
    API-->>NotificationPage: Return more notifications
    NotificationPage->>User: Append notifications
    User->>NotificationPage: Click "Mark all read"
    NotificationPage->>API: Mark all as read (TODO)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

In the warren of code, new groups now abound,
With chats, lists, and members all neatly found.
Notifications ping, and loading spinners spin,
Rabbits hop through pages, eager to begin.
With stories to tell, and icons anew,
This garden of features was grown just for you!
🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

npm error Exit handler never called!
npm error This is an error with npm itself. Please report this error at:
npm error https://github.com/npm/cli/issues
npm error A complete log of this run can be found in: /.npm/_logs/2025-07-24T01_26_00_939Z-debug-0.log

Note

⚡️ Unit Test Generation - Beta

CodeRabbit's unit test generation is now available in Beta! Automatically generate comprehensive unit tests for your code changes, ensuring better test coverage and catching edge cases you might miss. Our AI analyzes your code structure and creates tests that follow best practices and your project's testing patterns. Learn more here, or just try it under ✨ Finishing Touches.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 generate unit tests to generate unit tests for 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.

devgony added 20 commits July 9, 2025 16:04
- Create service.ts as centralized API client wrapper
- Integrate with useAuthStore for automatic token management
- Export individual API groups for convenient usage
- Automatically add Bearer token to all API requests via securityWorker
- Replace manual hook creation with dynamic useApiQuery function
- Add comprehensive type definitions for API client mapping
- Implement automatic parameter and return type inference
- Add useQuery convenience object for common patterns
- Maintain backward compatibility with legacy hook exports
- Enable future-proof API integration without manual mapping
- Support conditional fetching and custom SWR keys
- Optimize SWR fetcher functions to remove unnecessary async/await
- Add debugging capabilities for API response parsing issues
- Test useMyProfile hook implementation in MyPage component
- Investigate and address API response data handling problems
- Update service.ts with better SWR integration patterns
- Remove hooks/main/queries folder with custom SWR hooks
- Replace useGoalList with useGoals from service.ts
- Replace useGoalWithSubGoalTodo with useGoalWithSubGoals from service.ts
- Replace useTodoList with useSubGoalTodos from service.ts
- Migrate data conversion logic inline to components
- Maintain backward compatibility with existing component interfaces
- Consolidate all API calls through centralized service layer
- Remove duplicate SWR logic and improve maintainability
- Move service.ts to api/service.ts for better organization
- Separate SWR hooks into api/hooks.ts for modular architecture
- Split core API client from SWR functionality
- Update all import paths from @/service to @/api/hooks
- Maintain clean separation of concerns:
  - api/service.ts: Core API client and types
  - api/hooks.ts: SWR-based React hooks
- All components updated with new import paths
- Build successful with no compilation errors
- Create dedicated api/useApiQuery.ts for dynamic API query hook
- Remove useApiQuery implementation from api/hooks.ts
- Import and re-export useApiQuery from the new module
- Maintain full backward compatibility with existing imports
- Improve code organization with single responsibility principle
- Clean separation between core hook logic and convenience wrappers
- Build successful with no compilation errors
- Update api/service.ts to use SWAGGER_URL from .env file
- Extract base server URL from SWAGGER_URL automatically
- Add getBaseUrl() helper function with URL parsing logic
- Maintain fallback to empty string for proxy scenarios
- Fix import in MyPage.tsx to use separate useApiQuery module
- Environment-driven configuration for better deployment flexibility
- Build successful with no compilation errors
- Add NEXT_PUBLIC_API_URL to .env file with proper API base URL
- Update api/service.ts to use NEXT_PUBLIC_API_URL instead of API_URL
- Remove debugging console.log statement
- Fix Next.js client-side environment variable access
- Ensure proper API URL resolution in production builds
- Build successful with correct environment variable usage
- Add dotenv-cli dependency for explicit env file control
- Update package.json scripts:
  - 'npm run dev': uses .env file
  - 'npm run local': uses .env.local file (new)
- Create .env.local with local development overrides
- Add comments to distinguish .env and .env.local usage
- Add .env.local to .gitignore for local-only settings
- Support different API endpoints for dev vs local environments
- Build successful with proper environment variable loading
- Add USE_LOCAL_API environment variable to control API destination
- Update next.config.ts to dynamically choose between local and remote API
- Configure .env for remote API (motimo.kro.kr:8080) in dev mode
- Configure .env.local for local API (localhost:8080) in local mode
- Set NEXT_PUBLIC_API_URL to empty string in local mode to use Next.js proxy
- Enable seamless switching between local and remote API environments
- Build successful with conditional proxy configuration
- Rename .env to .env.dev for team development settings
- Update package.json dev script to use .env.dev explicitly
- Add .env.local to .gitignore for local-only settings
- Complete isolation: no cross-contamination between environments
- Dev mode: only .env.dev loaded (remote API)
- Local mode: only .env.local loaded (localhost API)
- Build successful with proper environment separation
- Teams can share .env.dev while keeping .env.local private
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (1)
app/notification/page.tsx (1)

28-31: Implement mark all read functionality.

The mark all read feature is currently a placeholder. This functionality would improve user experience significantly.

Would you like me to help implement the mark all read API integration? I can generate the API call and state management logic needed for this feature.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9781d9f and c6d7ddd.

📒 Files selected for processing (8)
  • app/group/[id]/member/page.tsx (1 hunks)
  • app/group/[id]/page.tsx (1 hunks)
  • app/notification/page.tsx (1 hunks)
  • components/group/GroupItem.tsx (5 hunks)
  • components/group/GroupList.tsx (2 hunks)
  • components/icons/BellIcon.tsx (2 hunks)
  • components/shared/AppBar/AppBar.tsx (2 hunks)
  • lib/date.ts (1 hunks)
📓 Path-based instructions (3)
components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For every styling, use tailwind css.

Files:

  • components/shared/AppBar/AppBar.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For tokens, use tokens.css (mdc:app/tokens.css).

Files:

  • components/shared/AppBar/AppBar.tsx
components/**/*

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

Component should be created in components/

Files:

  • components/shared/AppBar/AppBar.tsx
🧬 Code Graph Analysis (1)
components/shared/AppBar/AppBar.tsx (2)
api/hooks.ts (1)
  • useNotifications (170-170)
components/icons/BellIcon.tsx (1)
  • BellIcon (12-53)
🪛 Biome (1.9.4)
components/shared/AppBar/AppBar.tsx

[error] 27-27: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

🚧 Files skipped from review as they are similar to previous changes (4)
  • app/group/[id]/page.tsx
  • components/icons/BellIcon.tsx
  • components/group/GroupItem.tsx
  • components/group/GroupList.tsx
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For every styling, use tailwind css.

Files:

  • components/shared/AppBar/AppBar.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For tokens, use tokens.css (mdc:app/tokens.css).

Files:

  • components/shared/AppBar/AppBar.tsx
components/**/*

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

Component should be created in components/

Files:

  • components/shared/AppBar/AppBar.tsx
🧬 Code Graph Analysis (1)
components/shared/AppBar/AppBar.tsx (2)
api/hooks.ts (1)
  • useNotifications (170-170)
components/icons/BellIcon.tsx (1)
  • BellIcon (12-53)
🪛 Biome (1.9.4)
components/shared/AppBar/AppBar.tsx

[error] 27-27: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: chromatic-test
🔇 Additional comments (7)
components/shared/AppBar/AppBar.tsx (3)

5-6: LGTM! Imports support the refactoring.

The new imports are necessary for internal notification state management and navigation.


91-91: LGTM! BellIcon prop updated correctly.

The prop change from hasNotification to hasNewMessages aligns with the component's internal state management and the BellIcon interface.


25-31: Simplify hasNewMessages and verify notification count semantics

The ternary on line 27 can be replaced with a simple boolean coercion, and we need to confirm what totalCount actually represents.

• Replace this:

- const hasNewMessages = totalCount && totalCount > 0 ? true : false;
+ const hasNewMessages = Boolean(totalCount);

• Please verify whether totalCount from useNotifications(0, 1) is the number of unread notifications.
– If it’s the total number of notifications (read + unread), adjust the hook or query to return only unread counts so the indicator only shows for new items.

app/group/[id]/member/page.tsx (2)

17-26: LGTM! Well-structured component with appropriate hook usage.

The component properly uses hooks for data fetching and state management. The destructuring with default values prevents potential undefined access issues.


107-143: LGTM! Well-implemented member list rendering.

The member list properly identifies the current user, formats dates consistently, and provides appropriate interactions. The conditional poke button and accessibility attributes are well implemented.

app/notification/page.tsx (2)

7-13: LGTM! Well-organized notification type constants.

The constant mapping with as const assertion provides good type safety and maintainability for notification messages.


155-194: LGTM! Well-structured notification list rendering.

The notification list properly handles read/unread states with appropriate visual indicators and maintains consistent styling. The separation of icon and content areas provides good visual hierarchy.

Comment on lines +62 to +74
const handlePokeUser = async (userId: string, nickname: string) => {
try {
await groupApi.sendPokeNotification(id, userId);
setToastMessage(`${nickname}님에게 찌르기를 보냈어요!`);
setToastVisible(true);

setTimeout(() => {
setToastVisible(false);
}, 3000);
} catch (error) {
console.error("찌르기 전송 실패:", error);
}
};

This comment was marked as resolved.

Comment on lines +33 to +89
const renderNotificationContent = (notification: any) => {
const type = notification.type;
const content = notification.content || "";

// Extract nickname from content if possible
const nicknameMatch = content.match(/^([^님]+)님/);
const nickname = nicknameMatch ? nicknameMatch[1] : "닉네임";

switch (type) {
case "REACTION":
return (
<>
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{nickname}
</span>
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
님이 리액션을 남겼습니다.
</span>
</>
);
case "POKE":
return (
<>
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{nickname}
</span>
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
님이 찔렀어요!
</span>
</>
);
case "TODO_DUE_DAY":
return (
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{content || "\"투두 내용이 여기에\" 투두가 1일 남았어요!"}
</span>
);
case "GROUP_TODO_COMPLETED":
return (
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{content || `${nickname}님이 투두를 완료했어요!`}
</span>
);
case "GROUP_TODO_RESULT_COMPLETED":
return (
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{content || `${nickname}님이 기록을 남겼어요!`}
</span>
);
default:
return (
<span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
{content}
</span>
);
}
};

This comment was marked as resolved.

devgony added 7 commits July 23, 2025 08:33
- Always render notification count header instead of duplicating
- Use conditional rendering for "모두 읽음" button based on notification count
- Simplify empty state by removing redundant count display
- Remove unnecessary fragment wrapper in notification list
Replace eye icon with correct bell notification icon matching Figma specifications
Add automatic loading of next page when user scrolls near bottom with loading indicator
Add mock notification generator with 156 items and debug toggle to test infinite scroll functionality
… Storybook

- Extract reusable NotificationContent component from notification page
- Clean up notification page to use only real API data
- Add comprehensive Storybook stories for testing infinite scroll with mock data
- Improve component separation and testability
- Remove invalid hasNotification and onNotificationClick props from AppBar usage
- Import and use NotificationItemRsTypeEnum for proper type safety in stories
- Change null to undefined for CustomPageNotificationItemRs compatibility
- Update AppBar stories to remove non-existent prop controls
- Replace ineffective Suspense with proper Loading component display
- Show Loading component during goal data fetch and group joining
- Remove unused Suspense wrapper and import
- Replace button loading text with full-screen Loading component
@devgony devgony changed the title Groups 목록 페이지 구현 Groups 목록 페이지, 그룹 멤버 찌르기 구현 Jul 23, 2025
@devgony devgony changed the title Groups 목록 페이지, 그룹 멤버 찌르기 구현 Groups 목록, 그룹 멤버 찌르기 구현 Jul 23, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (3)
app/notification/page.tsx (1)

11-11: Improve type safety by using proper notification interface.

The allNotifications state uses any[] type, which reduces type safety and could lead to runtime errors.

Based on the NotificationContent component (lines 15-21 in components/notification/NotificationContent.tsx), use the proper Notification interface:

+interface Notification {
+  id: string;
+  type: string;
+  content: string;
+  isRead: boolean;
+  createdAt: string;
+}
+
-  const [allNotifications, setAllNotifications] = useState<any[]>([]);
+  const [allNotifications, setAllNotifications] = useState<Notification[]>([]);
components/notification/NotificationContent.stories.tsx (1)

59-59: Use proper notification types for consistency.

Similar to the page component, the wrapper uses any[] for notifications which reduces type safety.

Use the Notification interface for consistency with the component being tested:

+interface Notification {
+  id: string;
+  type: string;
+  content: string;
+  isRead: boolean;
+  createdAt: string;
+}
+
-  const [allNotifications, setAllNotifications] = useState<any[]>([]);
+  const [allNotifications, setAllNotifications] = useState<Notification[]>([]);
components/notification/NotificationContent.tsx (1)

87-143: Improve notification content rendering and use defined constants.

The notification content rendering has similar issues to those flagged in previous reviews: fragile regex parsing and not utilizing the defined constants.

Improve the implementation by using the NOTIFICATION_TYPE_MESSAGES constant and more robust parsing:

 const renderNotificationContent = (notification: Notification) => {
   const type = notification.type;
   const content = notification.content || "";

   // Extract nickname from content if possible
-  const nicknameMatch = content.match(/^([^님]+)님/);
-  const nickname = nicknameMatch ? nicknameMatch[1] : "닉네임";
+  const nicknameMatch = content.match(/^([^님]+)님/);
+  const nickname = nicknameMatch?.[1] || "닉네임";

   switch (type) {
     case "REACTION":
+    case "POKE":
+    case "GROUP_TODO_COMPLETED":
+    case "GROUP_TODO_RESULT_COMPLETED":
       return (
         <>
           <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
             {nickname}
           </span>
           <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
-            님이 리액션을 남겼습니다.
+            {NOTIFICATION_TYPE_MESSAGES[type as keyof typeof NOTIFICATION_TYPE_MESSAGES]}
           </span>
         </>
       );
-    case "POKE":
-      return (
-        <>
-          <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
-            {nickname}
-          </span>
-          <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
-            님이 찔렀어요!
-          </span>
-        </>
-      );
     case "TODO_DUE_DAY":
       return (
         <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
           {content || '"투두 내용이 여기에" 투두가 1일 남았어요!'}
         </span>
       );
-    case "GROUP_TODO_COMPLETED":
-      return (
-        <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
-          {content || `${nickname}님이 투두를 완료했어요!`}
-        </span>
-      );
-    case "GROUP_TODO_RESULT_COMPLETED":
-      return (
-        <span className="font-SUIT_Variable font-normal text-base leading-[1.4] tracking-[-0.01em] text-[#1E2124]">
-          {content || `${nickname}님이 기록을 남겼어요!`}
-        </span>
-      );
🧹 Nitpick comments (2)
app/notification/page.tsx (2)

19-36: Consider optimizing the effect dependency array.

The pagination logic is well-implemented with proper duplicate prevention. However, including page in the dependency array may cause unnecessary re-runs when only the page changes without new data.

Consider removing page from the dependency array since the effect logic already handles page changes appropriately:

-  }, [notificationData, page]);
+  }, [notificationData]);

The page === 0 check inside the effect already handles the distinction between first and subsequent pages correctly.


42-45: Complete the mark all as read functionality.

The handleMarkAllRead function currently only logs a message and has a TODO comment for the actual implementation.

Do you want me to help implement the mark all as read API call? I can generate the implementation that integrates with your notification API service.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between c6d7ddd and 3703b06.

📒 Files selected for processing (8)
  • app/group/join-random/page.tsx (1 hunks)
  • app/notification/page.tsx (1 hunks)
  • components/group/GroupPage.tsx (1 hunks)
  • components/icons/NotificationIcon.tsx (1 hunks)
  • components/mypage/MyPage.tsx (0 hunks)
  • components/notification/NotificationContent.stories.tsx (1 hunks)
  • components/notification/NotificationContent.tsx (1 hunks)
  • components/shared/AppBar/AppBar.stories.tsx (0 hunks)
📓 Path-based instructions (4)
components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For every styling, use tailwind css.

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For tokens, use tokens.css (mdc:app/tokens.css).

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
components/**/*

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

Component should be created in components/

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
components/**/*.stories.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

components/**/*.stories.{js,jsx,ts,tsx}: If Component is created, add storybook files as well.
The Default example of storybook should be named Primary

Files:

  • components/notification/NotificationContent.stories.tsx
🧠 Learnings (1)
components/notification/NotificationContent.stories.tsx (2)

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : If Component is created, add storybook files as well.

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : The Default example of storybook should be named Primary

🧬 Code Graph Analysis (3)
app/notification/page.tsx (3)
hooks/useSafeRouter.ts (1)
  • useSafeRouter (4-18)
api/hooks.ts (1)
  • useNotifications (170-170)
components/notification/NotificationContent.tsx (1)
  • NotificationContent (33-240)
components/notification/NotificationContent.stories.tsx (1)
components/notification/NotificationContent.tsx (1)
  • NotificationContent (33-240)
components/notification/NotificationContent.tsx (3)
api/generated/motimo/Api.ts (1)
  • CustomPageNotificationItemRs (345-355)
plopfile.js (1)
  • path (3-3)
components/icons/NotificationIcon.tsx (1)
  • NotificationIcon (9-30)
💤 Files with no reviewable changes (2)
  • components/mypage/MyPage.tsx
  • components/shared/AppBar/AppBar.stories.tsx
✅ Files skipped from review due to trivial changes (1)
  • components/icons/NotificationIcon.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/group/join-random/page.tsx
  • components/group/GroupPage.tsx
🧰 Additional context used
📓 Path-based instructions (4)
components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For every styling, use tailwind css.

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For tokens, use tokens.css (mdc:app/tokens.css).

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
components/**/*

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

Component should be created in components/

Files:

  • components/notification/NotificationContent.stories.tsx
  • components/notification/NotificationContent.tsx
components/**/*.stories.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

components/**/*.stories.{js,jsx,ts,tsx}: If Component is created, add storybook files as well.
The Default example of storybook should be named Primary

Files:

  • components/notification/NotificationContent.stories.tsx
🧠 Learnings (1)
components/notification/NotificationContent.stories.tsx (2)

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : If Component is created, add storybook files as well.

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : The Default example of storybook should be named Primary

🧬 Code Graph Analysis (3)
app/notification/page.tsx (3)
hooks/useSafeRouter.ts (1)
  • useSafeRouter (4-18)
api/hooks.ts (1)
  • useNotifications (170-170)
components/notification/NotificationContent.tsx (1)
  • NotificationContent (33-240)
components/notification/NotificationContent.stories.tsx (1)
components/notification/NotificationContent.tsx (1)
  • NotificationContent (33-240)
components/notification/NotificationContent.tsx (3)
api/generated/motimo/Api.ts (1)
  • CustomPageNotificationItemRs (345-355)
plopfile.js (1)
  • path (3-3)
components/icons/NotificationIcon.tsx (1)
  • NotificationIcon (9-30)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: chromatic-test
🔇 Additional comments (11)
app/notification/page.tsx (2)

15-16: Good use of pagination and safe data handling.

The notification data fetching correctly implements pagination and safely handles potentially undefined totalCount using nullish coalescing.


52-63: Clean component structure with proper prop passing.

The component render correctly passes all necessary props to NotificationContent including event handlers, state, and data. The separation of concerns between page-level state management and UI rendering is well implemented.

components/notification/NotificationContent.stories.tsx (3)

6-54: Excellent mock data generation for comprehensive testing.

The mock data generator creates realistic and varied notifications using proper API enum types. The content generation logic accurately reflects different notification types and provides good test coverage for the component.


105-122: Well-configured Storybook meta for mobile UI testing.

The meta configuration appropriately uses fullscreen layout and mobile viewport for testing the notification component. The totalCount control provides effective testing capabilities for different notification scenarios.


139-189: Comprehensive story scenarios for thorough testing.

The individual stories effectively cover key testing scenarios including empty state, pagination boundaries, infinite scroll behavior, and notification type variety. Each story includes helpful documentation for clarity.

components/notification/NotificationContent.tsx (6)

7-31: Excellent type safety and constant definitions.

The type definitions significantly improve upon the type safety issues seen in other parts of the notification system. The NOTIFICATION_TYPE_MESSAGES constant provides centralized message templates, and the Notification interface is properly typed with explicit fields.


42-85: Robust infinite scroll and pagination implementation.

The infinite scroll implementation correctly uses useCallback for performance optimization, properly calculates pagination boundaries, and manages the hasMore state effectively. The scroll event handling and cleanup is implemented correctly.


149-170: Well-implemented accessible AppBar with proper navigation.

The AppBar correctly implements a back button with proper accessibility (aria-label) and clean styling. The inline SVG for the back arrow is appropriate and the layout structure is well-organized.


174-194: Smart conditional header with appropriate button placement.

The header correctly shows notification count and conditionally renders the "Mark All Read" button only when notifications exist. The dynamic styling based on totalCount ensures proper layout in both states.


196-234: Comprehensive notification list with good UX patterns.

The notification list implementation effectively handles both empty and populated states. Each notification item has clear visual indicators for read/unread status, proper key usage for React rendering, and the loading indicator provides good feedback during infinite scroll operations.


145-240: Well-structured component following coding guidelines.

The component properly uses Tailwind CSS for styling as required by the coding guidelines, maintains good semantic HTML structure, and implements comprehensive accessibility features. The separation of concerns between layout, interaction, and content rendering is well-executed.

devgony added 7 commits July 23, 2025 09:37
- Add validation for falsy and non-string inputs
- Add invalid date detection using isNaN check
- Return consistent '--.--.--' fallback for invalid inputs
- Add explicit string return type annotation
- Prevent "NaN.NN.NN" display in UI for malformed dates
- Change Notification type property to use strict typing with keyof typeof NOTIFICATION_TYPE_MESSAGES
- Remove fragile regex parsing for nickname extraction
- Consolidate similar notification types into single case handling
- Simplify content rendering to directly use notification content
- Reduce code complexity and potential parsing errors
- Add useRef to track timeout IDs and prevent memory leaks
- Add useEffect cleanup to clear timeouts on component unmount
- Standardize toast timeout duration to 2000ms for consistency
- Apply proper timeout management to all toast scenarios
- Prevent hanging timeouts after component cleanup
- Add null initial value to useRef<NodeJS.Timeout | null>(null)
- Fix missing required argument error in TypeScript compilation
- Maintain type safety for timeout reference handling
- Add Suspense import back to React imports
- Wrap JoinRandomGroupContent component that uses useSearchParams()
- Add appropriate Loading fallback for page loading state
- Resolve Next.js warning about missing Suspense boundary
- Add Primary story as default example following coding guidelines
- Use 25 notifications for moderate, typical usage demonstration
- Include proper documentation with story description
- Position Primary story before other variants as main example
- Add toast notification for poke send failures
- Use consistent timeout reference management for error case
- Provide user feedback for failed poke attempts instead of silent failures
- Maintain consistent 2000ms timeout duration across all scenarios
Copy link
Collaborator

@Hys-Lee Hys-Lee left a comment

Choose a reason for hiding this comment

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

참여중인 그룹이랑 참여 대기중 목표에 같은 데이터들이 있습니다.
백엔드 문제 같긴해서 말씀 드리면 될 것 같아요

</div>
</div>

{!isMe && (

This comment was marked as resolved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

현재일로부터 일주일 이상 접속하지 않은 사람에게는 [찌르기] 버튼이 활성화 됩니다.
이거 말씀하시는거죠?
날짜비교야 어렵지 않겠지만.. 그렇게하면 시연이 불가능하지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일주일로 반영 했습니다.

setShowLeaveModal(true);
};

const handleLeaveConfirm = async () => {

This comment was marked as resolved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일부는 되고 일부는 안되는 걸로 보여 백엔드 문의 했습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

해결되었습니다.

<BellIcon
width={20}
height={20}
<BellIcon
Copy link
Collaborator

Choose a reason for hiding this comment

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

벨 누르면 알림 끄는 기능도 디자인에 있는 것 같슴다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

API 가 미개발이래요
image

import { Loading } from "@/components/shared";

// Component that uses useSearchParams
function JoinRandomGroupContent() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

그룹 랜덤 조인하면 다음 에러 뜹니다

stitched-error.ts:23 Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

Check the render method of JoinRandomGroupContent.
at JoinRandomGroupContent (page.tsx:78:12)
at JoinRandomGroupPage (page.tsx:189:7)
<...>
JoinRandomGroupContent @ page.tsx:78

JoinRandomGroupPage @ page.tsx:189
await in fetchServerResponse
onClick @ GroupList.tsx:55

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

혹시 어떤 목표에서 하셨나요? groupId 를 주시면 더 좋을거같습니다~

devgony added 2 commits July 24, 2025 09:36
- Extract member list functionality from page into reusable GroupMemberList component
- Add conditional poke button visibility based on 7-day inactivity threshold
- Create comprehensive Storybook stories for different member activity scenarios
- Update Storybook config to include app directory stories for better component testing
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
app/group/[id]/member/page.stories.tsx (2)

4-32: Consider adding a Primary story variant.

Based on previous learnings, the Default example of Storybook should be named Primary. Consider adding a Primary story variant that represents the most common use case.

+export const Primary: Story = {
+    parameters: {
+        nextjs: {
+            navigation: {
+                pathname: "/group/test-group/member",
+            },
+        },
+        mockData: [
+            // Add standard mock data here
+        ],
+    },
+};

42-200: Consider extracting common mock data to reduce duplication.

The story variants provide excellent test coverage for different member activity scenarios. However, there's significant duplication in the mock data structure across stories.

Consider extracting common mock data:

+const baseMockData = [
+    {
+        url: "/api/groups/test-group",
+        method: "GET",
+        status: 200,
+        response: {
+            name: "스터디 그룹",
+        },
+    },
+    {
+        url: "/api/user/profile",
+        method: "GET",
+        status: 200,
+        response: {
+            nickname: "현재사용자",
+        },
+    },
+];

+const createMembersStory = (members: any[]): Story => ({
+    parameters: {
+        nextjs: {
+            navigation: {
+                pathname: "/group/test-group/member",
+            },
+        },
+        mockData: [
+            ...baseMockData,
+            {
+                url: "/api/groups/test-group/members",
+                method: "GET",
+                status: 200,
+                response: members,
+            },
+        ],
+    },
+});
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 0022dce and 05ffa5f.

📒 Files selected for processing (2)
  • app/group/[id]/member/page.stories.tsx (1 hunks)
  • app/group/[id]/member/page.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
app/group/[id]/member/page.stories.tsx (2)

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : If Component is created, add storybook files as well.

Learnt from: CR
PR: prography/10th-Motimo-FE#0
File: .cursor/rules/global-rule.mdc:0-0
Timestamp: 2025-07-21T07:45:37.405Z
Learning: Applies to components/**/*.stories.{js,jsx,ts,tsx} : The Default example of storybook should be named Primary

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: chromatic-test
🔇 Additional comments (6)
app/group/[id]/member/page.stories.tsx (1)

37-40: LGTM! Clean date calculation logic.

The date calculations are correct and follow a consistent pattern for creating test scenarios with different member activity states.

app/group/[id]/member/page.tsx (5)

1-26: LGTM! Well-structured component setup.

The component follows Next.js 13+ conventions with proper async params handling, custom hooks for data fetching, and appropriate state management. Good use of useRef for timeout cleanup.


28-81: Excellent improvements addressing past review feedback!

The event handlers now properly address the timeout cleanup issues and error handling concerns raised in previous reviews:

  • ✅ Uses timeoutRef.current to prevent memory leaks
  • ✅ Consistent 2000ms timeout duration across all handlers
  • ✅ Comprehensive error handling with user-facing toast notifications
  • ✅ Proper error feedback for poke functionality

The implementation is now robust and user-friendly.


83-90: Perfect cleanup implementation!

The useEffect cleanup properly addresses the memory leak concerns from previous reviews by clearing any active timeouts when the component unmounts.


146-156: LGTM! Correct poke button logic implementation.

The conditional rendering properly implements the business logic:

  • Excludes current user from poke functionality
  • Shows poke button only for members inactive for more than 7 days
  • Accurate time calculation and proper event handling

92-220: Excellent UI implementation with good accessibility practices.

The component provides a well-structured, accessible interface with:

  • Semantic HTML structure and proper ARIA labels
  • Responsive design with consistent styling
  • Good UX patterns (confirmation modals, toast notifications)
  • Proper overlay management and z-indexing

The implementation follows design system conventions and accessibility best practices.

@devgony devgony requested a review from Hys-Lee July 24, 2025 01:00
devgony added 2 commits July 24, 2025 10:10
…boarding

- Replace two separate custom spinner divs with consistent Loading component
- Merge duplicate loading conditions into single conditional statement
- Improve code maintainability and UI consistency
- Split merged loading condition into separate hydration and login checks
- Ensure non-logged-in users see LoginScreen instead of infinite loading
- Improve loading state logic clarity and maintainability
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
components/group/GroupMemberList.tsx (2)

25-43: Refactor to eliminate code duplication in timeout handling.

The function correctly handles success and error cases, but the timeout logic is duplicated. Consider extracting this into a helper function.

  const handlePokeUser = async (userId: string, nickname: string) => {
+   const showToast = (message: string) => {
+     setToastMessage(message);
+     setToastVisible(true);
+     timeoutRef.current = setTimeout(() => {
+       setToastVisible(false);
+     }, 2000);
+   };
+
    try {
      await groupApi.sendPokeNotification(groupId, userId);
-     setToastMessage(`${nickname}님에게 찌르기를 보냈어요!`);
-     setToastVisible(true);
-     
-     timeoutRef.current = setTimeout(() => {
-       setToastVisible(false);
-     }, 2000);
+     showToast(`${nickname}님에게 찌르기를 보냈어요!`);
    } catch (error) {
      console.error("찌르기 전송 실패:", error);
-     setToastMessage("찌르기 전송에 실패했습니다.");
-     setToastVisible(true);
-     
-     timeoutRef.current = setTimeout(() => {
-       setToastVisible(false);
-     }, 2000);
+     showToast("찌르기 전송에 실패했습니다.");
    }
  };

81-82: Improve readability of the date calculation logic.

The 7-day offline check logic is correct but could be more readable.

Consider extracting the date calculation:

+ const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000;
+ const isOfflineMoreThan7Days = (lastOnlineDate: string) => {
+   return new Date().getTime() - new Date(lastOnlineDate).getTime() > SEVEN_DAYS_MS;
+ };

  {!isMe && 
-   new Date().getTime() - new Date(member.lastOnlineDate).getTime() > 7 * 24 * 60 * 60 * 1000 && (
+   isOfflineMoreThan7Days(member.lastOnlineDate) && (
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 05ffa5f and 3835c40.

📒 Files selected for processing (5)
  • .storybook/main.ts (3 hunks)
  • app/group/[id]/member/page.tsx (1 hunks)
  • app/onboarding/page.tsx (2 hunks)
  • components/group/GroupMemberList.stories.tsx (1 hunks)
  • components/group/GroupMemberList.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • components/group/GroupMemberList.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/group/[id]/member/page.tsx
  • .storybook/main.ts
🧰 Additional context used
📓 Path-based instructions (3)
components/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For every styling, use tailwind css.

Files:

  • components/group/GroupMemberList.tsx
{components/**/*.{js,jsx,ts,tsx},**/*.css}

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

For tokens, use tokens.css (mdc:app/tokens.css).

Files:

  • components/group/GroupMemberList.tsx
components/**/*

📄 CodeRabbit Inference Engine (.cursor/rules/global-rule.mdc)

Component should be created in components/

Files:

  • components/group/GroupMemberList.tsx
🧬 Code Graph Analysis (1)
components/group/GroupMemberList.tsx (1)
api/service.ts (1)
  • groupApi (48-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: chromatic-test
🔇 Additional comments (5)
app/onboarding/page.tsx (2)

11-11: LGTM! Good use of shared component.

The import statement correctly references the shared Loading component, promoting code reusability and consistency across the application.


99-107: Excellent refactoring to use shared Loading component.

The loading display logic has been properly refactored to use the centralized Loading component while maintaining the same conditional behavior. This improves code consistency and maintainability. The className usage ensures visual consistency across both loading states.

components/group/GroupMemberList.tsx (3)

1-18: Well-structured imports and type definitions.

The imports are clean and appropriate, and the TypeScript interfaces are properly defined with clear, descriptive property names.


20-23: Proper state management implementation.

Good use of useState for toast state and useRef for timeout management to prevent memory leaks.


45-52: Excellent cleanup implementation.

Proper cleanup of timeouts on component unmount prevents memory leaks. Well done!

return (
<div
key={member.userId}
className="flex items-center justify-between bg-[#F7F7F8] rounded-lg p-3"
Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Use design tokens from tokens.css instead of hardcoded colors.

The component uses hardcoded hex color values. According to the coding guidelines, tokens should be used from tokens.css (mdc:app/tokens.css).

Consider replacing hardcoded colors with design tokens:

- className="flex items-center justify-between bg-[#F7F7F8] rounded-lg p-3"
+ className="flex items-center justify-between bg-surface-variant rounded-lg p-3"

- className="font-SUIT_Variable font-bold text-sm leading-[1.4] tracking-[-0.01em] text-[#1E2124]"
+ className="font-SUIT_Variable font-bold text-sm leading-[1.4] tracking-[-0.01em] text-on-surface"

- className="flex items-center text-sm leading-[1.4] tracking-[-0.01em] text-[#464C53]"
+ className="flex items-center text-sm leading-[1.4] tracking-[-0.01em] text-on-surface-variant"

- className="bg-[#33363D] text-white px-3 py-1 rounded-lg text-sm font-Pretendard font-semibold"
+ className="bg-primary text-on-primary px-3 py-1 rounded-lg text-sm font-Pretendard font-semibold"

Also applies to: 67-67, 71-71, 75-75, 87-87


🏁 Script executed:

#!/bin/bash
# Check if tokens.css exists and what color tokens are available
fd -n tokens.css
if [ -f "app/tokens.css" ]; then
  echo "=== Available color tokens in tokens.css ==="
  cat app/tokens.css | grep -E "^\s*--.*color|^\s*--.*bg|^\s*--.*surface" | head -20
fi

Length of output: 1128


🏁 Script executed:

#!/bin/bash
# Search for semantic token definitions in tokens.css
grep -n -E "surface|primary|on-surface" app/tokens.css || echo "No semantic tokens found"

Length of output: 534


Replace hardcoded hex colors with CSS variables from tokens.css

According to project styling guidelines, all color values should be pulled from app/tokens.css via CSS variables rather than hardcoded hex. The following mappings match the existing colors:

  • #F7F7F8var(--color-Color-gray-5)
  • #1E2124var(--color-Color-gray-90)
  • #464C53var(--color-Color-gray-70)
  • #33363Dvar(--color-Color-gray-80)
  • #FFFFFFvar(--color-Color-white)

Apply these changes in the component:

- className="flex items-center justify-between bg-[#F7F7F8] rounded-lg p-3"
+ className="flex items-center justify-between bg-[var(--color-Color-gray-5)] rounded-lg p-3"

- className="font-SUIT_Variable font-bold text-sm leading-[1.4] tracking-[-0.01em] text-[#1E2124]"
+ className="font-SUIT_Variable font-bold text-sm leading-[1.4] tracking-[-0.01em] text-[var(--color-Color-gray-90)]"

- className="flex items-center text-sm leading-[1.4] tracking-[-0.01em] text-[#464C53]"
+ className="flex items-center text-sm leading-[1.4] tracking-[-0.01em] text-[var(--color-Color-gray-70)]"

- className="bg-[#33363D] text-white px-3 py-1 rounded-lg text-sm font-Pretendard font-semibold"
+ className="bg-[var(--color-Color-gray-80)] text-[var(--color-Color-white)] px-3 py-1 rounded-lg text-sm font-Pretendard font-semibold"

Affected lines: 64, 67, 71, 75, 87, 99–101.

🤖 Prompt for AI Agents
In components/group/GroupMemberList.tsx at lines 64, 67, 71, 75, 87, and 99 to
101, replace all hardcoded hex color values with the corresponding CSS variables
from app/tokens.css as per project styling guidelines. For example, change
#F7F7F8 to var(--color-Color-gray-5), #1E2124 to var(--color-Color-gray-90),
#464C53 to var(--color-Color-gray-70), #33363D to var(--color-Color-gray-80),
and #FFFFFF to var(--color-Color-white). Update the className strings or style
attributes accordingly to use these CSS variables instead of hex codes.

Copy link
Collaborator

@Hys-Lee Hys-Lee left a comment

Choose a reason for hiding this comment

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

동작 확인했습니다

@devgony devgony merged commit 555e13f into prography:main Jul 24, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Groups 목록 페이지 구현

3 participants