[Feature] 카테고리 관리 화면에서, 내가 만든 목표와 그룹에 속한 목표를 분리#91
Conversation
Summary of ChangesHello @dioo1461, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 카테고리 관리 화면의 사용자 경험을 개선하기 위해 목표를 '내가 만든 목표'와 '그룹에 속한 목표'로 시각적으로 분리하는 기능을 구현합니다. 이를 통해 사용자는 자신의 목표와 그룹 목표를 쉽게 구분할 수 있으며, 그룹 목표에 대한 불필요한 수정/삭제 작업을 방지합니다. 또한, 그룹 색상 관리 로직을 최적화하여 애플리케이션 전반의 성능과 일관성을 향상시켰습니다. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이번 PR은 카테고리 관리 화면에서 사용자가 직접 생성한 목표와 그룹에 속한 목표를 분리하는 기능을 성공적으로 구현했습니다. 특히 useGroupColorKey를 useGroupColorKeyMap으로 리팩토링하여 모든 그룹 색상을 한 번에 가져오도록 한 것은 좋은 성능 개선입니다. 새로운 컴포넌트와 훅 또한 새로운 기능을 지원하기 위해 잘 구조화되었습니다.
다만 몇 가지 개선점을 발견했습니다. useSplitCategoriesByGroup 훅에 그룹 카테고리가 일시적으로 UI에서 사라질 수 있는 중요한 버그가 있습니다. 또한 새로운 GroupCategoryItem 컴포넌트에 불필요한 코드가 남아있고, CategoryItem 컴포넌트에는 잘못된 UI 요소가 포함된 것으로 보입니다. 그 외에도 가독성 향상을 위해 코드를 단순화할 수 있는 몇 군데를 발견했습니다. 자세한 내용은 아래 리뷰 코멘트를 참고해주세요.
| if (belongingGroup) { | ||
| const groupColorKey = belongingGroup ? colorKeyMap.get(belongingGroup.groupId) : null; | ||
| if (!groupColorKey) return acc; | ||
| acc.groupCategories.push({ ...category, groupColorKey: groupColorKey }); |
There was a problem hiding this comment.
if (belongingGroup) 블록 내에 있어 belongingGroup이 항상 참이므로 삼항 연산자는 불필요합니다. 코드를 단순화할 수 있습니다.
그러나 더 중요한 문제가 있습니다. groupColorKey를 찾지 못하면(예: 데이터 로딩 중) 해당 카테고리가 '내 목표'와 '그룹 목표' 목록 양쪽에서 모두 누락되어 UI에서 일시적으로 사라지게 됩니다. 이는 사용자 경험에 좋지 않은 영향을 줍니다. useGroupColorKeyMap의 isReady 플래그를 사용하여 색상 데이터가 준비되었을 때만 이 로직을 실행하도록 하여 이 문제를 해결하는 것을 고려해 보세요.
const groupColorKey = colorKeyMap.get(belongingGroup.groupId);
if (!groupColorKey) return acc;
acc.groupCategories.push({ ...category, groupColorKey });| return ( | ||
| <View style={styles.container}> | ||
| <View style={styles.content}> | ||
| <View style={styles.groupColorIndicator} /> |
There was a problem hiding this comment.
이 groupColorIndicator 뷰는 '내가 만든 목표'를 표시하는 CategoryItem에 잘못 추가된 것으로 보입니다. '내가 만든 목표'는 특정 그룹에 속하지 않으므로 그룹 색상 표시가 필요 없습니다. 이 뷰와 관련 스타일(CategoryItem/index.style.ts의 groupColorIndicator)을 제거하는 것이 좋겠습니다.
| const navigation = useStackNavigation(); | ||
| const { mutateAsync: deleteCategoryMutate } = useDeleteCategoryMutation(); | ||
| const styles = createStyles({ groupColorKey }); | ||
|
|
||
| const onEditPress = () => { | ||
| navigation.navigate('EditCategory', { | ||
| // TODO: id가 null일 때 처리 로직 추가 | ||
| categoryId: id, | ||
| temporaryName: null, | ||
| }); | ||
| }; | ||
|
|
||
| // TODO: 수정, 삭제 연속으로 누르면 둘 다 반응하는 문제 해결하기 | ||
| const onDeletePress = () => { | ||
| dialogService.add({ | ||
| content: ( | ||
| <CategoryDeletionAlert | ||
| onConfirm={async () => { | ||
| // TODO: id가 null일 때 처리 로직 추가 | ||
| await deleteCategoryMutate(id!); | ||
| }} | ||
| /> | ||
| ), | ||
| options: { | ||
| cancelable: false, | ||
| }, | ||
| }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
onEditPress, onDeletePress 함수와 useDeleteCategoryMutation 훅이 정의되어 있지만, 이 컴포넌트에서는 그룹 카테고리를 수정/삭제하는 기능이 없어 실제로는 사용되지 않습니다. 불필요한 코드이므로 제거하여 컴포넌트를 더 명확하게 만드는 것이 좋겠습니다. 관련 import 구문(useStackNavigation, useDeleteCategoryMutation, dialogService, CategoryDeletionAlert)도 함께 정리해주세요.
| const groupColor = colorKeyMap.get(params.groupId) | ||
| ? GROUP_COLORS[colorKeyMap.get(params.groupId)!] | ||
| : null; |
There was a problem hiding this comment.
groupColor를 가져오는 로직이 다소 복잡하고, colorKeyMap.get()을 두 번 호출하고 있습니다. 가독성을 높이고 불필요한 중복 조회를 피하기 위해 코드를 단순화하는 것이 좋겠습니다.
const colorKey = colorKeyMap.get(params.groupId);
const groupColor = colorKey ? GROUP_COLORS[colorKey] : null;
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
frontend/src/components/Button/Icon/index.tsx
Lines 35 to 39 in 92e0a4f
Preserve container positioning for legacy Button.Icon style
The new containerStyle prop means any callers still passing style now lose container positioning. For example, PanelHeader supplies BUTTON_ICON_STYLE with style used to absolutely position the button; after this change, that style is forwarded to the SVG via {...props} and no longer affects the touchable container. This causes the header icon to render in the wrong place whenever a caller hasn’t been migrated to containerStyle. Consider mapping legacy style to containerStyle (or updating remaining callers like PanelHeader) to avoid the layout regression.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR implements a feature that separates personal goals from group-owned goals in the category management screen. The main changes include refactoring the group color key management system from individual queries to a centralized map-based approach, and creating a distinct UI component for displaying group categories that cannot be edited or deleted.
Changes:
- Refactored
useGroupColorKeytouseGroupColorKeyMapfor batch loading group colors across all components - Added
useSplitCategoriesByGrouphook to separate categories into personal and group-owned - Created
GroupCategoryItemcomponent for read-only display of group goals - Renamed Button.Icon prop from
styletocontainerStylefor API clarity - Updated Korean localization to distinguish between "내가 만든 목표" (goals I created) and "그룹에 속한 목표" (goals in groups)
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/studySlice.ts | Added unused import for GetGroupsResponse (should be removed) |
| src/screens/group/groupDetail/components/GroupInfoPanel/DashBoardButtonGroup/index.tsx | Updated to use useGroupColorKeyMap with groupId parameter |
| src/screens/group/groupDetail/GroupDetailScreen/index.tsx | Migrated from useGroupColorKey to useGroupColorKeyMap pattern |
| src/screens/group/GroupChatScreen/components/MessageInput/index.tsx | Updated Button.Icon to use containerStyle prop |
| src/screens/category/ManageCategoryScreen/index.tsx | Added logic to split and display categories by ownership type |
| src/screens/category/ManageCategoryScreen/index.style.ts | Changed toggle button layout from space-between to space-around |
| src/screens/category/ManageCategoryScreen/components/GroupCategoryItem/index.tsx | New component for displaying group categories (contains unused edit/delete code) |
| src/screens/category/ManageCategoryScreen/components/GroupCategoryItem/index.style.ts | Styling for group category items with dynamic group color |
| src/screens/category/ManageCategoryScreen/components/CategoryItem/index.tsx | Added color indicator and updated button styling |
| src/screens/category/ManageCategoryScreen/components/CategoryItem/index.style.ts | Added styles for color indicator and icon containers |
| src/screens/bottomTab/GroupsScreen/components/JoinedGroupPanel/index.tsx | Migrated to useGroupColorKeyMap pattern |
| src/navigation/BottomTabNavigator/BottomNavHeader.tsx | Updated Button.Icon to use containerStyle prop |
| src/locales/ko/manageCategory.json | Added translations for goal type separation |
| src/hooks/useCategoryData.ts | Added useSplitCategoriesByGroup hook with redundant null check |
| src/features/groups/hooks/useGroupColorKey.ts | Refactored from single-group to map-based batch loading |
| src/features/category/ui/InlineCategoryItem/index.style.ts | Updated spacing from 100 to 200 |
| src/features/category/hooks/useCategoryColor.ts | Migrated to useGroupColorKeyMap pattern |
| src/components/Button/Icon/index.tsx | Renamed style prop to containerStyle for clarity |
| .gitignore | Added tsconfig.tsbuildinfo to ignored files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/store/studySlice.ts
Outdated
|
|
||
| import type { Category } from '@/features/category/model'; | ||
| import type { DAY_MASK_BY_UTC_CODE } from '@/features/category/model/types'; | ||
| import { GetGroupsResponse } from '@/features/groups/model/groups'; |
There was a problem hiding this comment.
Unused import: GetGroupsResponse is imported but never used in this file. Consider removing the import to keep the code clean.
| import { GetGroupsResponse } from '@/features/groups/model/groups'; |
src/hooks/useCategoryData.ts
Outdated
| group.categoryIds.includes(category.id), | ||
| ); | ||
| if (belongingGroup) { | ||
| const groupColorKey = belongingGroup ? colorKeyMap.get(belongingGroup.groupId) : null; |
There was a problem hiding this comment.
Redundant null check: belongingGroup is already checked on line 59, so the ternary operator belongingGroup ? colorKeyMap.get(belongingGroup.groupId) : null is unnecessary. Since we're inside the if (belongingGroup) block, we can directly use colorKeyMap.get(belongingGroup.groupId) without the additional check.
| const groupColorKey = belongingGroup ? colorKeyMap.get(belongingGroup.groupId) : null; | |
| const groupColorKey = colorKeyMap.get(belongingGroup.groupId); |
| import { useDeleteCategoryMutation } from '@/features/category/api/mutations'; | ||
| import type { CategoryTag } from '@/features/category/model'; | ||
| import { useStackNavigation } from '@/hooks'; | ||
| import { dialogService } from '@/utils/dialog'; | ||
| import type { Time } from '@/utils/Time'; | ||
|
|
||
| import CategoryDeletionAlert from '../CategoryDeletionAlert'; | ||
| import { | ||
| createStyles, | ||
| GOAL_NAME_TEXT_STYLE, | ||
| GOAL_TIME_TEXT_STYLE, | ||
| TAG_CONTAINER_STYLE, | ||
| TAG_TEXT_STYLE, | ||
| } from './index.style'; | ||
|
|
||
| interface CategoryItemProps { | ||
| id: number | null; | ||
| groupColorKey: GroupColorKey; | ||
| temporaryName: string | null; | ||
| title: string; | ||
| goal: Time | null; | ||
| tags?: CategoryTag[]; | ||
| } | ||
|
|
||
| const GroupCategoryItem = ({ id, groupColorKey, title, goal, tags = [] }: CategoryItemProps) => { | ||
| const navigation = useStackNavigation(); | ||
| const { mutateAsync: deleteCategoryMutate } = useDeleteCategoryMutation(); | ||
| const styles = createStyles({ groupColorKey }); | ||
|
|
||
| const onEditPress = () => { | ||
| navigation.navigate('EditCategory', { | ||
| // TODO: id가 null일 때 처리 로직 추가 | ||
| categoryId: id, | ||
| temporaryName: null, | ||
| }); | ||
| }; | ||
|
|
||
| // TODO: 수정, 삭제 연속으로 누르면 둘 다 반응하는 문제 해결하기 | ||
| const onDeletePress = () => { | ||
| dialogService.add({ | ||
| content: ( | ||
| <CategoryDeletionAlert | ||
| onConfirm={async () => { | ||
| // TODO: id가 null일 때 처리 로직 추가 | ||
| await deleteCategoryMutate(id!); | ||
| }} | ||
| /> | ||
| ), | ||
| options: { | ||
| cancelable: false, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Dead code: The GroupCategoryItem component defines onEditPress and onDeletePress handlers, and imports useStackNavigation, useDeleteCategoryMutation, dialogService, and CategoryDeletionAlert, but none of these are actually used in the component's render output. According to the PR description, group goals should not be editable/deletable from this screen. These unused imports and handlers should be removed to keep the code clean and avoid confusion.
|
|
||
| import type { TextProps } from '@/components/Text'; | ||
| import type { GroupColorKey } from '@/constants/groupColors'; | ||
| import { GROUP_COLORS } from '@/constants/groupColors'; |
There was a problem hiding this comment.
Unused import GROUP_COLORS.
| import { GROUP_COLORS } from '@/constants/groupColors'; |
| const onEditPress = () => { | ||
| navigation.navigate('EditCategory', { | ||
| // TODO: id가 null일 때 처리 로직 추가 | ||
| categoryId: id, | ||
| temporaryName: null, | ||
| }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Unused variable onEditPress.
| const onEditPress = () => { | |
| navigation.navigate('EditCategory', { | |
| // TODO: id가 null일 때 처리 로직 추가 | |
| categoryId: id, | |
| temporaryName: null, | |
| }); | |
| }; |
| }; | ||
|
|
||
| // TODO: 수정, 삭제 연속으로 누르면 둘 다 반응하는 문제 해결하기 | ||
| const onDeletePress = () => { |
There was a problem hiding this comment.
Unused variable onDeletePress.
… id prop from GroupCategoryItem
✨ 구현 기능 명세
✅ PR Point
😭 어려웠던 점