[REFACTOR] 오류 해결 및 추가 요구사항 반영 (토스트, 툴팁, 경로별 할일 제한) #287
Conversation
rbgksqkr
added
fix
WalkthroughThis update introduces per-type todo item limits, enforces these limits across various components, and centralizes limit logic and messages. It adds new utility functions, refines toast and drag-and-drop UI styles, implements a one-time tooltip using localStorage, and improves conditional rendering for empty favorites and navigation. Several styled components and constants are added or updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FavoriteTab
participant Utils
participant Toast
User->>FavoriteTab: Click "Add from Favorites"
FavoriteTab->>Utils: getIsLimitTodoType(todoType)
alt Type is limited
FavoriteTab->>Utils: MAX_TODO_ITEM_LENGTH_MAP[todoType]
FavoriteTab->>FavoriteTab: Check todoList.length >= max
alt Limit reached
FavoriteTab->>Toast: Show limit message
else Not at limit
FavoriteTab->>FavoriteTab: Add todo
FavoriteTab->>Toast: Show success message
end
else Type is not limited
FavoriteTab->>FavoriteTab: Add todo
FavoriteTab->>Toast: Show success message
end
sequenceDiagram
participant User
participant PlanTooltip
participant LocalStorage
PlanTooltip->>LocalStorage: Check HAS_SHOWN_PLAN_TOOLTIP
alt Not shown before
PlanTooltip->>User: Show tooltip
User->>PlanTooltip: Close tooltip
PlanTooltip->>LocalStorage: Set HAS_SHOWN_PLAN_TOOLTIP
else Already shown
PlanTooltip->>User: Tooltip not displayed
end
Possibly related issues
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
frontend/src/components/Toast/Toast.styled.ts (1)
30-33: Shadow updated – consider centralising shadow tokens.The new 3-layer shadow is subtler, but other components might still use ad-hoc shadows. To keep visual consistency, think about exposing a
theme.shadows.toasttoken and referencing it here.- box-shadow: - 0px 2px 8px 0px rgba(0, 0, 0, 0.12), - 0px 1px 4px 0px rgba(0, 0, 0, 0.08), - 0px 0px 1px 0px rgba(0, 0, 0, 0.08); + box-shadow: ${({ theme }) => theme.shadows.toast};frontend/src/utils/getIsLimitTodoType.ts (1)
4-8: Consider using the configuration constants for better maintainability.The function logic is correct, but it hardcodes the limited todo types. Consider refactoring to use the
MAX_TODO_ITEM_LENGTH_MAPkeys for better maintainability.-export const getIsLimitTodoType = (todoType: TodoType) => { - return ( - todoType === TODO_TYPE.TODAY || todoType === TODO_TYPE.TOMORROW || todoType === TODO_TYPE.PATH - ); -}; +export const getIsLimitTodoType = (todoType: TodoType) => { + return MAX_TODO_ITEM_LENGTH_MAP[todoType] !== Infinity; +};This approach would automatically include any new limited types added to the configuration without requiring changes to this utility function.
frontend/src/components/FavoriteTab/FavoriteTab.styled.ts (1)
25-30: Minor styling inconsistency in margin-top.The
WatchMoreLinkWrapperhasmargin-top: 1.2remwhile the equivalent component inRecommendTab.styled.tsusesmargin-top: 0.8rem. Consider standardizing this value for consistency.export const WatchMoreLinkWrapper = styled.div` width: 100%; display: flex; justify-content: center; - margin-top: 1.2rem; + margin-top: 0.8rem; `;frontend/src/components/Tooltip/Tooltip.styled.ts (3)
4-7: Consider more flexible positioning strategy.The fixed
top: 0positioning might not work well in all contexts where the tooltip is used. Consider making the positioning more flexible or configurable.- top: 0; + top: -100%;This would position the tooltip above its container, which is more typical for tooltips.
22-31: Arrow positioning is hardcoded to bottom center.The CSS arrow is fixed to the bottom center position, which limits the tooltip's flexibility. Consider making the arrow position configurable based on props.
For a more flexible implementation, you could add props to control arrow position:
-export const TooltipLayout = styled.div` +export const TooltipLayout = styled.div<{ $arrowPosition?: 'top' | 'bottom' | 'left' | 'right' }>`Then conditionally render the arrow based on the prop.
20-20: Consider accessibility for tooltips.The
white-space: nowrapprevents text wrapping, which could cause overflow issues on smaller screens or with longer content.Consider allowing controlled wrapping for better responsive behavior:
- white-space: nowrap; + white-space: pre-wrap; + max-width: 25rem;frontend/src/utils/localStorage.ts (2)
5-5: Consider safer JSON parsing.The current implementation assumes all stored values are valid JSON. Consider adding validation for cases where localStorage might contain non-JSON strings.
- return item ? JSON.parse(item) : null; + if (!item) return null; + try { + return JSON.parse(item); + } catch (parseError) { + console.warn(`Invalid JSON in localStorage for key "${key}":`, parseError); + return null; + }
7-7: Consider consistent error message language.The error messages are in Korean, which might be inconsistent with the rest of the codebase if it primarily uses English. Consider standardizing the language for error messages.
If the codebase primarily uses English, consider translating:
localStorage에 해당하는 키값이 없습니다→Failed to get item from localStoragelocalStorage에 해당하는 키값을 설정할 수 없습니다→Failed to set item in localStoragelocalStorage에 해당하는 키값을 삭제할 수 없습니다→Failed to remove item from localStorageAlso applies to: 17-17, 25-25
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
frontend/src/components/FavoriteTab/FavoriteTab.styled.ts(1 hunks)frontend/src/components/FavoriteTab/index.tsx(4 hunks)frontend/src/components/PlanTooltip/index.tsx(1 hunks)frontend/src/components/RecommendTab/index.tsx(1 hunks)frontend/src/components/Toast/Toast.styled.ts(1 hunks)frontend/src/components/TodoTab/index.tsx(3 hunks)frontend/src/components/Tooltip/Tooltip.styled.ts(1 hunks)frontend/src/components/Tooltip/index.tsx(1 hunks)frontend/src/constants/config.ts(2 hunks)frontend/src/constants/message.ts(2 hunks)frontend/src/pages/EditPage/EditPage.styled.ts(1 hunks)frontend/src/pages/FavoritePage/components/FavoriteHeader/FavoriteHeader.tsx(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/PlanContent.styled.ts(1 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/index.tsx(3 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/TimeBlockItem.styled.ts(3 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/index.tsx(2 hunks)frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/index.tsx(1 hunks)frontend/src/pages/RecommendTodoPage/components/RecommendTodoContainer/RecommendTodoContainer.styled.ts(1 hunks)frontend/src/pages/RecommendTodoPage/components/RecommendTodoContainer/index.tsx(3 hunks)frontend/src/pages/RouteTodoEditPage/RouteTodoEditPage.styled.ts(2 hunks)frontend/src/utils/getIsLimitTodoType.ts(1 hunks)frontend/src/utils/getLimitMessage.ts(1 hunks)frontend/src/utils/localStorage.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
frontend/src/components/PlanTooltip/index.tsx (1)
frontend/src/utils/localStorage.ts (2)
getItem(1-11)setItem(13-19)
frontend/src/components/Tooltip/index.tsx (1)
frontend/src/styles/theme.ts (1)
colors(1-68)
frontend/src/utils/getIsLimitTodoType.ts (2)
frontend/src/types/todo.ts (1)
TodoType(13-13)frontend/src/constants/config.ts (1)
TODO_TYPE(20-25)
frontend/src/utils/getLimitMessage.ts (2)
frontend/src/types/todo.ts (1)
TodoType(13-13)frontend/src/constants/config.ts (2)
TODO_TYPE(20-25)MAX_TODO_ITEM_LENGTH_MAP(27-32)
frontend/src/constants/message.ts (2)
frontend/src/types/todo.ts (1)
TodoType(13-13)frontend/src/utils/getLimitMessage.ts (1)
getLimitMessage(4-15)
frontend/src/components/FavoriteTab/FavoriteTab.styled.ts (2)
frontend/src/components/TodoTab/TodoTab.styled.ts (2)
SloganWrapper(7-10)Slogan(12-26)frontend/src/components/RecommendTab/RecommendTab.styled.ts (2)
WatchMoreLinkWrapper(13-18)WatchMoreLink(20-31)
frontend/src/components/FavoriteTab/index.tsx (4)
frontend/src/types/todo.ts (1)
TodoType(13-13)frontend/src/constants/config.ts (2)
TODO_TYPE(20-25)MAX_TODO_ITEM_LENGTH_MAP(27-32)frontend/src/utils/getIsLimitTodoType.ts (1)
getIsLimitTodoType(4-8)frontend/src/constants/message.ts (1)
TODO_TOAST_MESSAGE(12-20)
🔇 Additional comments (39)
frontend/src/pages/RouteTodoEditPage/RouteTodoEditPage.styled.ts (1)
10-10: Scrollbar switched toauto– looks good.
overflow-y: autohides the scrollbar until needed and aligns with the rest of the refactor. No functional risks spotted.Also applies to: 85-85
frontend/src/pages/EditPage/EditPage.styled.ts (1)
8-8: Consistent scrollbar behaviour across pages – thumbs-up.The same
autobehaviour is now applied here; promotes UX consistency.frontend/src/pages/FavoritePage/components/FavoriteHeader/FavoriteHeader.tsx (1)
9-9: Static title may break context-aware headers – double-check.Previously the title reflected the selected tab via
useTabContext; now it is hard-coded. Ensure:
- The design requirement really mandates a single header title.
- No remaining code relies on dynamic titles (analytics, tests, etc.).
If dynamic behaviour is still needed, consider reinstating context logic or moving the string to a prop.
frontend/src/pages/RecommendTodoPage/components/RecommendTodoContainer/RecommendTodoContainer.styled.ts (1)
10-10: Scrollbar policy aligned – good.Matches other containers; no concerns.
frontend/src/constants/config.ts (2)
3-3: Good addition of specific limit for PATH todos.The new constant provides clear separation between different todo type limits.
27-32: Excellent centralized configuration for todo limits.The mapping provides a clean, maintainable way to configure different limits per todo type. Setting
FAVORITEtoInfinitycorrectly indicates no limit for favorites.frontend/src/utils/getLimitMessage.ts (1)
4-15: Good centralized approach for limit messages.The function provides a clean way to generate type-specific limit messages. The switch statement structure is appropriate and the Korean localization is consistent.
frontend/src/constants/message.ts (2)
3-3: Good refactoring to use centralized utility.Replacing the direct config import with the utility function import improves maintainability and separates concerns.
15-15: Excellent delegation to utility function.The change from hardcoded message generation to using
getLimitMessagecentralizes the logic and supports the broaderTodoTypeinstead of just'TODAY' | 'TOMORROW'.frontend/src/pages/RecommendTodoPage/components/RecommendTodoContainer/index.tsx (3)
13-14: Good integration of new utility functions and hooks.The imports properly bring in the necessary limit checking utilities and additional todo list hook for comprehensive limit enforcement.
Also applies to: 17-17, 23-23
58-58: Smart fallback logic for todo list.The fallback logic
currentTodoList || routeTodoListensures the component works correctly regardless of which todo list is available.
69-77: Excellent limit checking implementation.The function properly:
- Checks if the todo type is subject to limits
- Retrieves the appropriate maximum length
- Shows user-friendly toast message when limit is exceeded
- Returns early to prevent the mutation
This is a great example of defensive programming with good user feedback.
frontend/src/components/TodoTab/index.tsx (2)
8-8: LGTM: Centralized import approach.The import changes from
MAX_TODO_ITEM_LENGTHtoMAX_TODO_ITEM_LENGTH_MAPand addition ofgetIsLimitTodoTypeutility align well with the centralized limit management approach across components.Also applies to: 17-17
38-42: LGTM: Consistent limit enforcement logic.The refactored limit checking logic using
getIsLimitTodoTypeandMAX_TODO_ITEM_LENGTH_MAPprovides better type-specific control and consistency across components. The toast message usingtodoTypeinstead ofdateTypeis also more accurate.frontend/src/components/RecommendTab/index.tsx (3)
4-4: LGTM: Consistent imports for centralized limit management.The added imports for
useRouteTodoQuery,MAX_TODO_ITEM_LENGTH_MAP,useTodoListQuery,useQueryParamsDate, andgetIsLimitTodoTypealign with the centralized limit management approach being implemented across components.Also applies to: 8-8, 11-12, 15-15
23-23: LGTM: Proper todo list management.The hook usage for fetching date type, current todo list, and route todo list, along with the fallback logic in
todoList = currentTodoList || routeTodoList, provides robust todo list management for limit checking.Also applies to: 26-27, 31-31
36-43: LGTM: Consistent limit enforcement implementation.The limit checking logic before adding todos from recommendations follows the same pattern as other components, ensuring consistent behavior across the application.
frontend/src/components/FavoriteTab/FavoriteTab.styled.ts (3)
1-1: LGTM: Proper Link import.The import of
Linkfrom'react-router'is correct and necessary for theWatchMoreLinkcomponent.
4-23: LGTM: Consistent slogan styling.The
SloganWrapperandSloganstyled components follow the same pattern as inTodoTab.styled.ts, ensuring consistent empty state messaging across components.
32-43: LGTM: Consistent link styling.The
WatchMoreLinkstyled component matches the design pattern fromRecommendTab.styled.ts, ensuring consistent styling across components.frontend/src/components/FavoriteTab/index.tsx (6)
4-4: LGTM: Consistent imports for centralized limit management.The added imports align with the centralized limit management approach, bringing in necessary utilities and hooks for todo list management and limit enforcement.
Also applies to: 9-9, 15-16, 19-19
27-27: LGTM: Proper todo list management setup.The hook usage for fetching date type, current todo list, and route todo list, along with the fallback logic, provides robust todo list management for limit checking.
Also applies to: 37-38, 44-44
46-48: LGTM: Safe empty state check.The
isFavoriteTodoListcalculation using optional chaining (favoriteTodoList.pages?.[0]?.data.length > 0) safely handles the case where the pages array might be empty or undefined.
51-58: LGTM: Consistent limit enforcement.The limit checking logic before adding todos from favorites follows the same pattern as other components, ensuring consistent behavior across the application.
83-87: LGTM: Well-implemented empty state UI.The conditional rendering of the empty state message when
!isFavoriteTodoListprovides good UX feedback when no favorite todos exist.
138-142: LGTM: Dynamic "watch more" link.The "watch more" link with dynamic text based on
isFavoriteTodoListprovides contextually appropriate navigation options - "즐겨찾기 수정하기" when todos exist and "즐겨찾기 추가하기" when they don't.frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/PlanContent.styled.ts (1)
14-14: LGTM! Positioning context for tooltip.The addition of
position: relativecreates the necessary positioning context for the tooltip component, which uses absolute positioning. This is a clean and appropriate solution.frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/index.tsx (1)
19-19: LGTM! Simplified dragging detection logic.The change from searching through todos to a direct null check is a good simplification that improves performance and readability. The logic is more direct and efficient.
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/index.tsx (2)
32-32: LGTM! More precise dragging state.The change from
hasDraggingItemtoisDraggingprovides more specific information to theuseCheckTodohook, allowing it to behave differently when this specific item is being dragged versus when any item is being dragged.
53-53: LGTM! Visual feedback for dragging state.Adding the
$isDraggingprop to theCheckIconWrapperenables proper visual feedback during drag operations, enhancing the user experience.frontend/src/pages/PlanPage/components/TimeBlockContent/PlanContent/index.tsx (3)
13-13: LGTM! Improved responsiveness for long press.Reducing the long press duration from 500ms to 300ms will make the drag-and-drop interaction feel more responsive.
124-124: LGTM! Index parameter properly added.The index parameter is correctly added to enable the conditional tooltip rendering logic.
137-137: LGTM! Smart conditional tooltip rendering.Rendering the tooltip only on the first time block section provides good UX without cluttering the interface.
frontend/src/components/Tooltip/index.tsx (2)
1-32: LGTM! Well-structured reusable tooltip component.The component is well-designed with proper TypeScript typing, clean structure, and appropriate use of PropsWithChildren pattern.
21-21: Icon color contrast is sufficient (6.0:1)
Verified thatcolors.gray200(#DCDCDC) on the tooltip backgroundcolors.green800(#25574e) yields a contrast ratio of ~6.0:1, which exceeds WCAG AA (4.5:1 for text) and UI component (3:1) requirements. No changes needed.frontend/src/components/PlanTooltip/index.tsx (1)
1-35: LGTM! Well-implemented one-time tooltip with localStorage.The component properly manages tooltip visibility using localStorage to ensure it's shown only once, with clean state management and proper lifecycle handling.
frontend/src/pages/PlanPage/components/TimeBlockContent/TimeBlockList/TimeBlockItem/TimeBlockItem.styled.ts (3)
12-12: LGTM! Good default background color.Adding a default white background ensures consistent appearance across different contexts.
17-31: LGTM! Excellent dragging state styling.The comprehensive styling for the dragging state provides excellent visual feedback with appropriate width, padding, border-radius, cursor, background color, and multi-layered box shadow.
48-62: LGTM! Smart hiding of check icon during drag.Hiding the check icon wrapper during dragging prevents visual clutter and maintains clean drag interaction.
| case TODO_TYPE.TOMORROW: | ||
| return `내일 할 일은 최대 ${MAX_TODO_ITEM_LENGTH_MAP[TODO_TYPE.TOMORROW]}개까지 추가할 수 있어요`; |
There was a problem hiding this comment.
Fix inconsistent parameter usage in TOMORROW case.
Line 9 uses TODO_TYPE.TOMORROW instead of the todoType parameter, which is inconsistent with the TODAY case (line 7) and could lead to incorrect behavior.
- case TODO_TYPE.TOMORROW:
- return `내일 할 일은 최대 ${MAX_TODO_ITEM_LENGTH_MAP[TODO_TYPE.TOMORROW]}개까지 추가할 수 있어요`;
+ case TODO_TYPE.TOMORROW:
+ return `내일 할 일은 최대 ${MAX_TODO_ITEM_LENGTH_MAP[todoType]}개까지 추가할 수 있어요`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case TODO_TYPE.TOMORROW: | |
| return `내일 할 일은 최대 ${MAX_TODO_ITEM_LENGTH_MAP[TODO_TYPE.TOMORROW]}개까지 추가할 수 있어요`; | |
| case TODO_TYPE.TOMORROW: | |
| return `내일 할 일은 최대 ${MAX_TODO_ITEM_LENGTH_MAP[todoType]}개까지 추가할 수 있어요`; |
🤖 Prompt for AI Agents
In frontend/src/utils/getLimitMessage.ts at lines 8 to 9, the TOMORROW case uses
the constant TODO_TYPE.TOMORROW instead of the function parameter todoType to
access MAX_TODO_ITEM_LENGTH_MAP. Replace TODO_TYPE.TOMORROW with the todoType
parameter to ensure consistent and correct usage of the input parameter.
| export const hasLocalStorageItem = (key: string): boolean => { | ||
| return localStorage.getItem(key) !== null; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for consistency.
The hasLocalStorageItem function lacks error handling unlike the other localStorage utility functions. This could cause uncaught exceptions if localStorage is unavailable.
export const hasLocalStorageItem = (key: string): boolean => {
- return localStorage.getItem(key) !== null;
+ try {
+ return localStorage.getItem(key) !== null;
+ } catch (error) {
+ console.error(`localStorage 접근 중 오류가 발생했습니다: "${key}":`, error);
+ return false;
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const hasLocalStorageItem = (key: string): boolean => { | |
| return localStorage.getItem(key) !== null; | |
| }; | |
| export const hasLocalStorageItem = (key: string): boolean => { | |
| try { | |
| return localStorage.getItem(key) !== null; | |
| } catch (error) { | |
| console.error(`localStorage 접근 중 오류가 발생했습니다: "${key}":`, error); | |
| return false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In frontend/src/utils/localStorage.ts around lines 29 to 31, the
hasLocalStorageItem function currently accesses localStorage without error
handling, which can cause uncaught exceptions if localStorage is unavailable.
Wrap the localStorage.getItem call in a try-catch block and return false if an
error occurs, ensuring consistent error handling with other localStorage utility
functions.
Issue Number
As-Is
To-Be
Check List
Test Screenshot
(Optional) Additional Description
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores