[Feature/#53] record book page UI / 아직 완성 XXXX#75
Closed
holdn2 wants to merge 5 commits into
Closed
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
Disabled knowledge base sources:
📝 WalkthroughWalkthroughRecordBook에 탭 전환, 필터/정렬, 게시글 렌더 및 관련 모달/바텀시트를 추가했습니다. RecordBookTopTabBar는 그룹 기록/내 기록 탭과 애니메이션 언더라인을 제공하고, RecordBookFilter는 페이지 범위 입력 모드와 칩+정렬 드롭다운 모드를 조건부로 렌더합니다. 타입 및 정렬 상수와 더미 데이터가 추가되며, RecordBookPostItem은 투표 퍼센트 계산과 댓글/옵션/모달 연동을 담당합니다. RecordBookScreen은 이들 컴포넌트를 로컬 상태로 통합해 FlatList로 표시합니다. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
screens/record-book/constants/index.ts (1)
2-15: ⚡ Quick win[P4] 타입 안정성 개선으로 서버 연동 준비 👍
TODO 주석에서 언급하신 것처럼 추후 서버 타입과 맞출 예정이시군요! 그 전에
as const단언을 추가하면 타입 추론이 더 정확해지고, 나중에 서버 타입과 통합할 때도 수월합니다.제안하는 개선사항
// TODO: 추후 type은 서버에서 사용하는 것으로 수정 필요 export const GROUP_RECORD_SORT = [ { label: "최신순", type: "latest", }, { label: "인기순", type: "like", }, { label: "댓글 많은순", type: "comment", }, -]; +] as const;이렇게 하면
type필드가string이 아닌 리터럴 타입("latest" | "like" | "comment")으로 추론됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@screens/record-book/constants/index.ts` around lines 2 - 15, The GROUP_RECORD_SORT constant should be made readonly so its item 'type' fields infer literal types instead of general strings; update the GROUP_RECORD_SORT declaration (symbol: GROUP_RECORD_SORT) to use a const assertion (add as const) so TypeScript infers the exact literal union ("latest" | "like" | "comment") for safer typing and easier future server type alignment.screens/record-book/record-book-screen.tsx (2)
39-47: ⚡ Quick win[P4] 페이지 칩 선택 로직 가독성 개선 제안
Lines 40-46의 조건부 로직이 약간 복잡해 보이네요. 주석을 추가하거나 조건을 분리하면 의도가 더 명확해질 것 같습니다.
개선 제안
const handlePressPageChip = () => { + // 이미 페이지가 설정되어 있고 현재 선택되지 않은 경우, 다시 선택 상태로 전환 if (selectedChip !== "page") { if (selectedPages.start !== null || selectedPages.end !== null) { setSelectedChip("page"); return; } } + // 그 외의 경우 페이지 설정 모드로 진입 setPageSettingMode(true); };또는 early return 패턴으로:
const handlePressPageChip = () => { + // 페이지 범위가 이미 설정되어 있다면 칩을 활성화 const hasSelectedPages = selectedPages.start !== null || selectedPages.end !== null; if (selectedChip !== "page" && hasSelectedPages) { setSelectedChip("page"); return; } + + // 페이지 설정 UI 표시 setPageSettingMode(true); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@screens/record-book/record-book-screen.tsx` around lines 39 - 47, The conditional in handlePressPageChip is hard to read; refactor it to clarify intent by using early returns or extracting the condition into a named boolean helper (e.g., hasSelectedPages = selectedPages.start !== null || selectedPages.end !== null), then rewrite the flow to first handle the case where selectedChip !== "page" and hasSelectedPages (call setSelectedChip("page") and return), otherwise setPageSettingMode(true); keep references to selectedChip, selectedPages, setSelectedChip, and setPageSettingMode and add a short comment explaining the branch.
120-125: ⚡ Quick win[P4] 임시 FlatList 구조 개선 고려
현재
data={[1]}과 빈renderItem으로 헤더만 표시하는 구조인데요, 실제 데이터 연동 전까지는 괜찮지만 나중을 위해 주석을 추가하거나ListEmptyComponent를 활용하는 것도 좋은 방법입니다! 🎯대안 제안
방법 1: 주석 추가
+{/* TODO: 실제 기록 데이터로 교체 필요 */} <FlatList contentContainerStyle={styles.list} data={[1]} renderItem={() => <></>} ListHeaderComponent={RecordListHeader} />방법 2: ListEmptyComponent 활용
<FlatList contentContainerStyle={styles.list} - data={[1]} - renderItem={() => <></>} + data={[]} + renderItem={() => null} ListHeaderComponent={RecordListHeader} + ListEmptyComponent={<View />} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@screens/record-book/record-book-screen.tsx` around lines 120 - 125, The FlatList is currently using a dummy data prop (data={[1]}) and an empty renderItem to show only the header; replace this temporary pattern with a clearer intent by either adding an inline comment above the FlatList noting it is a placeholder for future data or, better, wire up a proper empty-state using FlatList's ListEmptyComponent and a real data prop (or empty array) so RecordListHeader remains as ListHeaderComponent while an explicit empty view handles the no-data case; update references to FlatList, data, renderItem, ListEmptyComponent, and RecordListHeader accordingly.screens/record-book/components/record-book-filter/index.tsx (1)
58-78: ⚡ Quick win[P4] TextInput 숫자 입력 개선 제안
keyboardType="number-pad"만으로는 완벽한 검증이 어려우니,onChangeText에서 숫자만 허용하도록 필터링하면 더 나은 UX를 제공할 수 있습니다.제안하는 개선사항
+const handleStartPageChange = (text: string) => { + // 숫자만 허용 + const numericValue = text.replace(/[^0-9]/g, ''); + setStartPage(numericValue); +}; + +const handleEndPageChange = (text: string) => { + const numericValue = text.replace(/[^0-9]/g, ''); + setEndPage(numericValue); +}; + // ... JSX 내부 <TextInput style={styles.input} value={startPage} - onChangeText={setStartPage} + onChangeText={handleStartPageChange} cursorColor={colors.neongreen} selectionColor={colors.neongreen} keyboardType="number-pad" /> // ... <TextInput style={styles.input} value={endPage} - onChangeText={setEndPage} + onChangeText={handleEndPageChange} cursorColor={colors.neongreen} selectionColor={colors.neongreen} keyboardType="number-pad" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@screens/record-book/components/record-book-filter/index.tsx` around lines 58 - 78, The numeric TextInput fields currently use keyboardType="number-pad" but still accept non-digit input; update the onChangeText handlers used with the TextInput components (the ones referencing startPage/setStartPage and endPage/setEndPage) to sanitize the incoming text by stripping non-digit characters (allowing empty string) before calling the state setters, so only digits are stored and rendered; ensure you apply the same filtering logic for both setStartPage and setEndPage handlers to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@screens/record-book/components/record-book-filter/index.tsx`:
- Around line 44-48: handleApply currently just converts startPage/endPage with
Number and calls handleApplyPage; add input validation so non-numeric,
zero/negative, and inverted ranges are rejected before calling handleApplyPage.
In the handleApply function validate that startPage/endPage (when non-empty)
parse to finite integers > 0 (use Number.isInteger after parsing or parseInt and
check isFinite), and if both present ensure start <= end; when validation fails,
surface an error to the user (e.g., set a local error state or show a toast) and
do not call handleApplyPage. Keep references to startPage, endPage, handleApply,
and handleApplyPage when implementing.
In `@screens/record-book/components/record-book-top-tab-bar/index.tsx`:
- Around line 11-15: The interface name FeedTopTabBarProps does not match the
component RecordBookTopTabBar; rename the interface to RecordBookTopTabBarProps
and update all usages/types accordingly (including props typing on the
RecordBookTopTabBar function/component and any imports/exports that reference
FeedTopTabBarProps) so the prop type and component name are consistent.
- Around line 22-31: The indicator's Animated.Value translateX is always
initialized at 0 but must reflect the initial prop isMyRecord: when the
component mounts and isMyRecord is true set translateX to INDICATOR_MOVE_X
(otherwise 0); add a useEffect that checks isMyRecord on mount and calls either
animateIndicator(INDICATOR_MOVE_X) or sets translateX value accordingly (or call
translateX.setValue(...)) so the indicator is synchronized with the initial
state; reference translateX, animateIndicator, INDICATOR_MOVE_X and the
isMyRecord prop to locate and update the component.
In `@screens/record-book/record-book-screen.tsx`:
- Around line 83-97: The RecordListHeader component currently returns undefined
when isMyRecord is true; update the early exit in RecordListHeader to explicitly
return null (e.g., replace the bare "return;" with "return null;") so the
component returns null instead of undefined when nothing should be rendered.
---
Nitpick comments:
In `@screens/record-book/components/record-book-filter/index.tsx`:
- Around line 58-78: The numeric TextInput fields currently use
keyboardType="number-pad" but still accept non-digit input; update the
onChangeText handlers used with the TextInput components (the ones referencing
startPage/setStartPage and endPage/setEndPage) to sanitize the incoming text by
stripping non-digit characters (allowing empty string) before calling the state
setters, so only digits are stored and rendered; ensure you apply the same
filtering logic for both setStartPage and setEndPage handlers to keep behavior
consistent.
In `@screens/record-book/constants/index.ts`:
- Around line 2-15: The GROUP_RECORD_SORT constant should be made readonly so
its item 'type' fields infer literal types instead of general strings; update
the GROUP_RECORD_SORT declaration (symbol: GROUP_RECORD_SORT) to use a const
assertion (add as const) so TypeScript infers the exact literal union ("latest"
| "like" | "comment") for safer typing and easier future server type alignment.
In `@screens/record-book/record-book-screen.tsx`:
- Around line 39-47: The conditional in handlePressPageChip is hard to read;
refactor it to clarify intent by using early returns or extracting the condition
into a named boolean helper (e.g., hasSelectedPages = selectedPages.start !==
null || selectedPages.end !== null), then rewrite the flow to first handle the
case where selectedChip !== "page" and hasSelectedPages (call
setSelectedChip("page") and return), otherwise setPageSettingMode(true); keep
references to selectedChip, selectedPages, setSelectedChip, and
setPageSettingMode and add a short comment explaining the branch.
- Around line 120-125: The FlatList is currently using a dummy data prop
(data={[1]}) and an empty renderItem to show only the header; replace this
temporary pattern with a clearer intent by either adding an inline comment above
the FlatList noting it is a placeholder for future data or, better, wire up a
proper empty-state using FlatList's ListEmptyComponent and a real data prop (or
empty array) so RecordListHeader remains as ListHeaderComponent while an
explicit empty view handles the no-data case; update references to FlatList,
data, renderItem, ListEmptyComponent, and RecordListHeader accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1861dd01-bba2-4b6e-9013-45f56b22747d
📒 Files selected for processing (5)
screens/record-book/components/index.tsscreens/record-book/components/record-book-filter/index.tsxscreens/record-book/components/record-book-top-tab-bar/index.tsxscreens/record-book/constants/index.tsscreens/record-book/record-book-screen.tsx
Comment on lines
+83
to
+97
| const RecordListHeader = () => { | ||
| if (isMyRecord) return; | ||
| return ( | ||
| <View style={styles.listHeader}> | ||
| <IcAlertGrey /> | ||
| <AppText weight="regular" size="xs" color={colors.grey[200]}> | ||
| {selectedChip === "page" | ||
| ? "페이지별 보기는 입력한 페이지의 글만 노출됩니다." | ||
| : selectedChip === "overview" | ||
| ? "총평 보기는 스포일러가 포함 될 수도 있습니다." | ||
| : "내 진행도에 따라 일부 댓글은 블러처리됩니다."} | ||
| </AppText> | ||
| </View> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
[P3] 컴포넌트에서 명시적으로 null 반환하기
React 19에서는 컴포넌트가 아무것도 렌더링하지 않을 때 명시적으로 null을 반환하는 것이 권장됩니다. 현재 Line 84에서 return 문만 있어 undefined가 반환되는데, return null로 명확히 해주세요.
제안하는 수정사항
const RecordListHeader = () => {
- if (isMyRecord) return;
+ if (isMyRecord) return null;
return (
<View style={styles.listHeader}>📝 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.
Suggested change
| const RecordListHeader = () => { | |
| if (isMyRecord) return; | |
| return ( | |
| <View style={styles.listHeader}> | |
| <IcAlertGrey /> | |
| <AppText weight="regular" size="xs" color={colors.grey[200]}> | |
| {selectedChip === "page" | |
| ? "페이지별 보기는 입력한 페이지의 글만 노출됩니다." | |
| : selectedChip === "overview" | |
| ? "총평 보기는 스포일러가 포함 될 수도 있습니다." | |
| : "내 진행도에 따라 일부 댓글은 블러처리됩니다."} | |
| </AppText> | |
| </View> | |
| ); | |
| }; | |
| const RecordListHeader = () => { | |
| if (isMyRecord) return null; | |
| return ( | |
| <View style={styles.listHeader}> | |
| <IcAlertGrey /> | |
| <AppText weight="regular" size="xs" color={colors.grey[200]}> | |
| {selectedChip === "page" | |
| ? "페이지별 보기는 입력한 페이지의 글만 노출됩니다." | |
| : selectedChip === "overview" | |
| ? "총평 보기는 스포일러가 포함 될 수도 있습니다." | |
| : "내 진행도에 따라 일부 댓글은 블러처리됩니다."} | |
| </AppText> | |
| </View> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@screens/record-book/record-book-screen.tsx` around lines 83 - 97, The
RecordListHeader component currently returns undefined when isMyRecord is true;
update the early exit in RecordListHeader to explicitly return null (e.g.,
replace the bare "return;" with "return null;") so the component returns null
instead of undefined when nothing should be rendered.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
🔔 ETC
Summary by CodeRabbit
새로운 기능