Feat-10 #11
Feat-10 #11
Conversation
๐ WalkthroughWalkthroughThis PR introduces a new Next.js app structure with a main layout system, multiple reusable UI components for displaying books, news, profiles, searches, and settings, a header with navigation, extended CSS with custom properties and typography utilities, and configuration updates. A large set of base-level UI components is added across several functional areas. Changes
Estimated code review effort๐ฏ 3 (Moderate) | โฑ๏ธ ~25 minutes Poem
Pre-merge checks and finishing touchesโ Failed checks (1 warning, 1 inconclusive)
โ Passed checks (1 passed)
โจ 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. Comment |
Summary of ChangesHello @hongik-luke, 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! ์ด ํ ๋ฆฌํ์คํธ๋ Highlights
๐ง New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
โป๏ธ Duplicate comments (2)
src/components/base-ui/home/notification_element.tsx (1)
1-47: Duplicate ofProfile/notification_element.tsx.As noted in the Profile variant, consolidate into a single shared component. The implementation logic is correct.
src/components/base-ui/News/recommendbook_element.tsx (1)
5-56: Code duplication: nearly identical to Search/search_recommendbook.tsx.This component duplicates
src/components/base-ui/Search/search_recommendbook.tsxwith only minor dimensional differences. See the review comment on that file for a detailed refactoring approach to eliminate this duplication.
๐ก Minor comments (9)
src/components/base-ui/Profile/others_profile.tsx-17-17 (1)
17-17: Unused prop:onSettingsis defined but never used.The
onSettingscallback is declared in the props type but is never referenced in the component body. Either remove it from the type or wire it to a settings button.๐ Proposed fix
type OthersProfileProps = { profileImgSrc?: string; // default: /profile.svg name: string; followingCount: number; // ๊ตฌ๋ ์ค followerCount: number; // ๊ตฌ๋ ์ isSubscribed: boolean; onToggleSubscribe: (newState: boolean) => void; intro: string; - onSettings?: () => void; onReportClick?: () => void; className?: string; };src/components/base-ui/home/home_bookclub.tsx-72-72 (1)
72-72: Remove duplicate class nameBody_1_2.The className contains
Body_1_2twice, which is redundant.๐ Proposed fix
- <span className="text-[color:var(--Gray_7)] Body_1_2 Body_1_2">์ ์ฒด๋ณด๊ธฐ</span> + <span className="text-[color:var(--Gray_7)] Body_1_2">์ ์ฒด๋ณด๊ธฐ</span>src/components/base-ui/home/list_subscribe_element.tsx-8-8 (1)
8-8: Fix the comment to match the actual default value.The comment states the default is
"/profile.svg", but Line 17 actually defaults to"/profile2.svg".๐ Proposed fix
- profileSrc?: string; // ๊ธฐ๋ณธ: "/profile.svg" (public) + profileSrc?: string; // ๊ธฐ๋ณธ: "/profile2.svg" (public)src/components/base-ui/home/list_subscribe_element.tsx-24-33 (1)
24-33: Correct thesizesprop to match the actual container dimensions.The
sizesprop is set to"42px", but the container isw-[32px] h-[32px](32 pixels). This mismatch can cause Next.js to generate and load unnecessarily large images, impacting performance.๐ Proposed fix
sizes="42px" + sizes="32px"src/components/base-ui/Search/search_bookresult.tsx-69-74 (1)
69-74: Add descriptive alt text for interactive button icons.Both the heart icon (line 69-74) and pencil icon (line 89) have empty alt attributes. Screen reader users need descriptive text to understand the button's purpose. Add meaningful alt text or use
aria-labelon the buttons.๐ Suggested fix
<Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} - alt="" + alt={liked ? "Unlike" : "Like"} width={24} height={24} />- <Image src="/pencil_icon.svg" alt="" width={20} height={20} /> + <Image src="/pencil_icon.svg" alt="Edit" width={20} height={20} />Also applies to: 89-89
src/components/base-ui/News/recommendbook_element.tsx-46-46 (1)
46-46: Add descriptive alt text for the like button icon.The heart icon uses an empty alt attribute. Provide descriptive text for screen reader accessibility.
- <Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} alt="" width={24} height={24} /> + <Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} alt={liked ? "Unlike" : "Like"} width={24} height={24} />src/components/base-ui/Search/search_recommendbook.tsx-46-46 (1)
46-46: Add descriptive alt text for the like button icon.The heart icon has an empty alt attribute, which provides insufficient context for screen reader users. Consider adding descriptive text like "Like" or "Unlike" based on the current state, or mark the button itself with
aria-label.๐ Suggested fix
- <Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} alt="" width={24} height={24} /> + <Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} alt={liked ? "Unlike" : "Like"} width={24} height={24} />Or add aria-label to the button:
<button type="button" + aria-label={liked ? "Unlike this book" : "Like this book"} onClick={(e) => {src/app/globals.css-70-71 (1)
70-71: Reconsidertruncatein.Body_1utility class.Line 70 includes
truncatein the.Body_1class, while.Body_1_1(with identical typography specs) does not. Includingtruncatein a base typography utility can cause unexpected text clipping when the class is used in various contexts.Consider removing
truncatefrom.Body_1and applying it explicitly where text truncation is needed (e.g., via a separate utility class or inline).๐ Proposed fix
- .Body_1 { @apply text-[14px] font-semibold leading-[145%] tracking-[-0.014px] truncate; } + .Body_1 { @apply text-[14px] font-semibold leading-[145%] tracking-[-0.014px]; } .Body_1_1 { @apply text-[14px] font-semibold leading-[145%] tracking-[-0.014px]; }Apply
truncateexplicitly where needed:<div className="Body_1 truncate">...</div>src/components/layout/Header.tsx-36-39 (1)
36-39: Fix potential active state mismatch for nested routes.The current logic uses
pathname.startsWith(item.href)for non-root routes, which will incorrectly mark a navigation item as active for nested or similar paths. For example, "/groups" would remain active when viewing "/groups/123" (which may or may not be intended), and could also match "/groups-archive" (unintended).๐ Proposed fix for more precise matching
If you want nav items active only on exact matches:
const active = - item.href === "/" - ? pathname === "/" - : pathname.startsWith(item.href); + pathname === item.href;If you want nav items active for their entire section (e.g., "/groups" stays active on "/groups/123"):
const active = item.href === "/" ? pathname === "/" - : pathname.startsWith(item.href); + : pathname === item.href || pathname.startsWith(item.href + "/");
๐งน Nitpick comments (19)
src/components/base-ui/Profile/mypage_profile.tsx (2)
36-43: Fixed dimensions may limit responsiveness.The container has fixed
w-[734px] h-[244px]dimensions. If this is intentional for a specific design system breakpoint, consider documenting it. Otherwise, consider using responsive utilities or accepting width/height as props for flexibility.
111-140: Consider making button labels configurable.The CTA button labels ("์ฑ ์ด์ผ๊ธฐ ์ฐ๊ธฐ", "์์ ๋ฌธ์ํ๊ธฐ") are hardcoded. For reusability, consider accepting these as props with defaults.
๐ Proposed enhancement
type MypageProfileProps = { // ... existing props onLeftButtonClick?: () => void; onRightButtonClick?: () => void; + leftButtonLabel?: string; + rightButtonLabel?: string; className?: string; }; export default function MypageProfile({ // ... existing destructuring onLeftButtonClick, onRightButtonClick, + leftButtonLabel = "์ฑ ์ด์ผ๊ธฐ ์ฐ๊ธฐ", + rightButtonLabel = "์์ ๋ฌธ์ํ๊ธฐ", className = "", }: MypageProfileProps) {src/components/base-ui/Profile/others_profile.tsx (1)
34-81: Structural duplication withMypageProfile.The avatar + name + follower counts + intro section (lines 42-81) is nearly identical to
MypageProfile. Consider extracting a sharedProfileHeadercomponent to reduce duplication.src/components/base-ui/Profile/subscribe_element.tsx (1)
38-52: LGTM with minor note on typography consistency.The optional chaining on
onToggleSubscribe?.(!isSubscribed)is correctly implemented. Consider using the design system typography class (e.g.,Subhead_4_1) instead of inlinetext-[12px] font-semiboldfor consistency with other components in this PR.src/components/base-ui/home/list_subscribe_element.tsx (2)
37-37: Remove trailing whitespace in className.There's a trailing space after
Body_1in the className string.๐ Proposed fix
- <p className="text-[color:var(--Gray_7)] Body_1 "> + <p className="text-[color:var(--Gray_7)] Body_1">
49-49: Simplify redundant color declarations in button className.The button className contains both
text-whiteandtext-[color:var(--White,#FFF)], which are redundant. Use one consistent approach for better maintainability.๐ Proposed fix
- className="flex px-[17px] py-[8px] justify-center items-center gap-[10px] rounded-[8px] bg-[#9A7A6B] text-white text-[color:var(--White,#FFF)] text-[12px] font-semibold leading-[100%] tracking-[-0.012px] whitespace-nowrap" + className="flex px-[17px] py-[8px] justify-center items-center gap-[10px] rounded-[8px] bg-[#9A7A6B] text-white text-[12px] font-semibold leading-[100%] tracking-[-0.012px] whitespace-nowrap"src/components/base-ui/home/home_bookclub.tsx (1)
30-34: Use Next.jsImagecomponent for consistency and optimization.Line 32 uses the native
<img>tag, which misses Next.js's automatic image optimization (lazy loading, responsive sizes, modern formats). The rest of the codebase consistently uses theImagecomponent fromnext/image.๐ Proposed fix
{count === 0 && ( <div className=""> - <img src="logo2.svg" alt= "๋ก๊ณ " className="mx-auto mb-4 mt-[118px]" /> + <Image src="/logo2.svg" alt="๋ก๊ณ " width={100} height={100} className="mx-auto mb-4 mt-[118px]" /> </div> )}Note: Adjust
widthandheightto match the actual logo dimensions.src/components/base-ui/Search/search_recommendbook.tsx (1)
35-35: Consider adding priority prop for above-the-fold images.If this book cover card appears in the initial viewport, adding
priorityto the Next.js Image component will prevent it from being lazy-loaded, improving Largest Contentful Paint (LCP).<Image src={coverSrc} alt={title} fill sizes="244px" priority={/* determine if above fold */} className="object-cover" />src/components/base-ui/Search/search_bookresult.tsx (1)
33-33: Extract magic number to a named constant.The detail truncation limit of 500 characters is a magic number. Consider extracting it to a named constant for better maintainability and documentation of the business rule.
+const MAX_DETAIL_LENGTH = 500; + export default function SearchBookResult({ // ... }: SearchBookResultProps) { const coverSrc = imgUrl && imgUrl.length > 0 ? imgUrl : "/booksample.svg"; - const clippedDetail = detail.length > 500 ? detail.slice(0, 500) + "..." : detail; + const clippedDetail = detail.length > MAX_DETAIL_LENGTH + ? detail.slice(0, MAX_DETAIL_LENGTH) + "..." + : detail;src/app/(main)/ui-test/page.tsx (2)
2-2: Remove redundant React import.With the modern JSX transform (enabled by default in Next.js 16), explicitly importing React is no longer necessary unless you're using React hooks or other exports. Since you're only using
useState(imported separately on line 3), this import can be removed.-import React from "react"; import { useState } from "react";
4-4: Remove unused imports and state.Both
Subscribe_elements(line 4) and thesubsstate variable (line 8) are declared but never used in the component. If these are placeholders for future testing, consider adding a comment; otherwise, remove them to keep the code clean.๐ Suggested cleanup
-import Subscribe_elements from "@/components/base-ui/Profile/subscribe_element"; import Setting_ReportList from "@/components/base-ui/Settings/setting_report_list"; export default function UiTestClient() { - const [subs, setSubs] = useState(false); - return (Or if these are intentional placeholders:
// TODO: Add Subscribe_elements testing // const [subs, setSubs] = useState(false);Also applies to: 8-8
src/components/base-ui/News/news_list.tsx (1)
42-42: Use CSS variable for consistency instead of hardcoded color.Line 42 uses a hardcoded color value
#000, while other text elements consistently use CSS variables likevar(--Gray_4,#8D8D8D)(line 43) andvar(--Gray_3,#BBB)(line 50). For consistency with the project's color token system, consider using a CSS variable here as well.- <p className="text-[#000] Subhead_3 truncate">{title}</p> + <p className="text-[color:var(--Gray_7,#2C2C2C)] Subhead_3 truncate">{title}</p>Or define a
--Blackvariable if pure black is the intended semantic color.src/components/base-ui/Settings/setting_report_list.tsx (1)
22-28: Consider responsive width for better mobile experience.The fixed
w-[1000px]width may cause horizontal scrolling on smaller screens or tablets. Consider usingmax-w-[1000px] w-fullto allow the component to adapt to narrower viewports while maintaining the maximum width.๐ Suggested change
<div className={` - flex w-[1000px] p-[20px] items-start gap-[40px] + flex w-full max-w-[1000px] p-[20px] items-start gap-[40px] rounded-[8px] border border-[color:var(--Subbrown_4,#EAE5E2)] bg-[color:var(--White,#FFF)] ${className} `} >src/components/base-ui/Settings/setting_news_list.tsx (1)
4-10: Consider renaming type to match component naming pattern.The type
NewsListPropsdoesn't fully match the component nameSetting_NewsList. For consistency with the sibling component (which usesSettingReportListPropsforSetting_ReportList), consider renaming toSettingNewsListProps.๐ Suggested change
-type NewsListProps = { +type SettingNewsListProps = { imageUrl: string; title: string; content: string; date: string; className?: string; }; export default function Setting_NewsList({ imageUrl, title, content, date, className = "", -}: NewsListProps) { +}: SettingNewsListProps) {src/app/globals.css (1)
4-33: Consider consistent naming convention for CSS custom properties.The color tokens mix naming conventions (e.g.,
--Whitevs--primary_3). For better maintainability, consider adopting a consistent convention throughout:
- Option 1: All lowercase with underscores:
--white,--gray_1,--primary_3- Option 2: kebab-case:
--white,--gray-1,--primary-3Also, lines 10, 23, and 29 contain unnecessary blank lines that could be removed for cleaner formatting.
src/components/base-ui/button_without_img.tsx (1)
58-83: Consider style composition approach.The component mixes inline styles (for dynamic colors and dimensions) with Tailwind utility classes (for layout and transitions). This works but can lead to specificity conflicts. The current implementation appears intentional for dynamic theming, which is a valid use case.
One minor point: the
transition-colorsutility may not smoothly transition the inlinestyleproperties. Consider using CSS custom properties on the button element if smooth color transitions are required:style={{ '--btn-width': `${width}px`, '--btn-height': `${height}px`, '--btn-bg': bg, '--btn-border': border, '--btn-text': textColor, }} className="... [width:var(--btn-width)] [height:var(--btn-height)] [background-color:var(--btn-bg)] ..."However, the current approach is simpler and acceptable for most use cases.
src/app/(main)/layout.tsx (1)
1-11: LGTM! Consider adding layout constraints.The layout structure correctly composes the Header with page content. The implementation is clean and follows Next.js app router conventions.
Optional: Consider adding layout constraints to the
<main>element for consistent content width and padding:<main className="max-w-7xl mx-auto px-4 py-6"> {children} </main>This ensures consistent spacing across pages, but defer to your design system requirements.
src/components/layout/NavItem.tsx (1)
11-16: Consider usingclsxor template literals for cleaner conditional className composition.The
array.join(" ")pattern works but is less idiomatic in modern React. Also, the active link should includearia-current="page"for better accessibility.๐ Suggested improvements
Option 1: Using template literals
<Link href={href} - className={[ - "flex w-32 items-center justify-center gap-2.5 p-2.5", - active ? "border-b-2 border-white" : "border-b-2 border-transparent", - ].join(" ")} + className={`flex w-32 items-center justify-center gap-2.5 p-2.5 border-b-2 ${ + active ? "border-white" : "border-transparent" + }`} + aria-current={active ? "page" : undefined} >Option 2: Using clsx (recommended if already a dependency)
+import clsx from "clsx"; + export function NavItem({ href, label, active }: NavItemProps) { return ( <Link href={href} - className={[ - "flex w-32 items-center justify-center gap-2.5 p-2.5", - active ? "border-b-2 border-white" : "border-b-2 border-transparent", - ].join(" ")} + className={clsx( + "flex w-32 items-center justify-center gap-2.5 p-2.5 border-b-2", + active ? "border-white" : "border-transparent" + )} + aria-current={active ? "page" : undefined} >src/components/layout/Header.tsx (1)
55-91: Consider extracting icon links to reduce duplication.The three action icons (search, notification, profile) follow identical patterns. Additionally, using
priorityon all icons may not be necessary โ priority should be reserved for above-the-fold critical images like the logo.๐ Suggested refactor
Define an array and map over it:
+const ACTION_ICONS = [ + { href: "/search", icon: "/search.svg", label: "๊ฒ์" }, + { href: "/notification", icon: "/notification.svg", label: "์๋ฆผ" }, + { href: "/profile", icon: "/profile.svg", label: "ํ๋กํ" }, +]; + export default function Header() { const pathname = usePathname(); return ( <header className="w-full bg-[#7B6154]"> ... {/*์์ด์ฝ*/} <div className="flex items-center gap-4"> - <Link href="/search" aria-label="๊ฒ์" className="relative h-6 w-6"> - <Image - src="/search.svg" - alt="๊ฒ์" - fill - className="object-contain" - priority - /> - </Link> - - <Link - href="/notification" - aria-label="์๋ฆผ" - className="relative h-6 w-6" - > - <Image - src="/notification.svg" - alt="์๋ฆผ" - fill - className="object-contain" - priority - /> - </Link> - - <Link - href="/profile" - aria-label="ํ๋กํ" - className="relative h-6 w-6" - > - <Image - src="/profile.svg" - alt="ํ๋กํ" - fill - className="object-contain" - priority - /> - </Link> + {ACTION_ICONS.map((item) => ( + <Link + key={item.href} + href={item.href} + aria-label={item.label} + className="relative h-6 w-6" + > + <Image + src={item.icon} + alt={item.label} + fill + className="object-contain" + /> + </Link> + ))} </div>Note: Removed
priorityfrom action icons as they're not critical for initial page render.
๐ Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
โ Files ignored due to path filters (20)
package-lock.jsonis excluded by!**/package-lock.jsonpublic/ArrowDown.svgis excluded by!**/*.svgpublic/ArrowTop.svgis excluded by!**/*.svgpublic/Edit_icon.svgis excluded by!**/*.svgpublic/Setting_icon.svgis excluded by!**/*.svgpublic/booksample.svgis excluded by!**/*.svgpublic/gray_heart.svgis excluded by!**/*.svgpublic/icon_plus.svgis excluded by!**/*.svgpublic/logo.svgis excluded by!**/*.svgpublic/logo2.svgis excluded by!**/*.svgpublic/notification.svgis excluded by!**/*.svgpublic/pencil_icon.svgis excluded by!**/*.svgpublic/plus.svgis excluded by!**/*.svgpublic/profile.svgis excluded by!**/*.svgpublic/profile2.svgis excluded by!**/*.svgpublic/profile3.svgis excluded by!**/*.svgpublic/profile4.svgis excluded by!**/*.svgpublic/profile5.svgis excluded by!**/*.svgpublic/red_heart.svgis excluded by!**/*.svgpublic/search.svgis excluded by!**/*.svg
๐ Files selected for processing (27)
.github/ISSUE_TEMPLATE/custom.mdsrc/app/(main)/layout.tsxsrc/app/(main)/page.tsxsrc/app/(main)/ui-test/page.tsxsrc/app/globals.csssrc/app/layout.tsxsrc/app/page.backup.tsxsrc/components/base-ui/BookStory/bookstory_choosebook.tsxsrc/components/base-ui/BookStory/bookstory_detail.tsxsrc/components/base-ui/BookStory/bookstory_text.tsxsrc/components/base-ui/News/news_list.tsxsrc/components/base-ui/News/recommendbook_element.tsxsrc/components/base-ui/Profile/mypage_profile.tsxsrc/components/base-ui/Profile/notification_element.tsxsrc/components/base-ui/Profile/others_profile.tsxsrc/components/base-ui/Profile/subscribe_element.tsxsrc/components/base-ui/Search/search_bookresult.tsxsrc/components/base-ui/Search/search_recommendbook.tsxsrc/components/base-ui/Settings/setting_news_list.tsxsrc/components/base-ui/Settings/setting_report_list.tsxsrc/components/base-ui/button_without_img.tsxsrc/components/base-ui/home/home_bookclub.tsxsrc/components/base-ui/home/list_subscribe_element.tsxsrc/components/base-ui/home/notification_element.tsxsrc/components/layout/Header.tsxsrc/components/layout/NavItem.tsxtsconfig.json
๐ค Files with no reviewable changes (1)
- src/app/layout.tsx
๐งฐ Additional context used
๐งฌ Code graph analysis (3)
src/app/(main)/layout.tsx (1)
src/components/layout/Header.tsx (1)
Header(15-97)
src/components/layout/Header.tsx (1)
src/components/layout/NavItem.tsx (1)
NavItem(9-28)
src/app/(main)/ui-test/page.tsx (1)
src/components/base-ui/Settings/setting_report_list.tsx (1)
Setting_ReportList(14-65)
๐ Additional comments (23)
tsconfig.json (2)
4-8: Formatting improvements to TypeScript configuration.The reformatting of the
lib,paths, andexcludesections to multi-line arrays improves readability and is consistent with modern config formatting practices. These changes are purely cosmetic and do not affect functionality.Also applies to: 26-28, 41-43
37-39: Good: Added Next.js 15 type definitions and ESM support.The inclusion of
.next/types/**/*.ts,.next/dev/types/**/*.ts, and**/*.mtsensures TypeScript has access to Next.js auto-generated type definitions and ESM module support, which aligns with Next.js 15 and modern TypeScript practices.src/components/base-ui/Profile/mypage_profile.tsx (1)
6-22: UnusedonSettingsprop in type definition.The type defines
onSettings(line 16), but at line 17 there's also a reference to settings. Verify thatonSettingsis correctly wired to the settings button at line 100.src/components/base-ui/home/home_bookclub.tsx (1)
1-11: LGTM!The imports and type definitions are clean and appropriate.
src/components/base-ui/Search/search_bookresult.tsx (1)
19-95: Good use of event propagation control.The
stopPropagation()calls on the like and pencil buttons (lines 64, 80) correctly prevent triggering the card'sonCardClickhandler when interacting with nested controls. This is a solid interaction pattern.src/components/base-ui/News/news_list.tsx (1)
12-55: Well-structured responsive layout.The component effectively uses flexbox with proper overflow handling (min-w-0, flex-1) and responsive image loading via Next.js Image. The layout gracefully handles text truncation and maintains fixed spacing for the date column.
src/components/base-ui/Settings/setting_news_list.tsx (2)
28-36: LGTM! Proper Next.js Image usage.The Image component is correctly configured with:
fillprop with relative positioned parent containersizesattribute for optimal image loadingobject-coverfor consistent aspect ratio- Meaningful alt text using the dynamic title
19-53: LGTM! Well-structured responsive layout.The layout implementation demonstrates good practices:
- Uses
max-w-[1000px] w-fullfor responsive width- Proper flex patterns with
min-w-0for text truncationline-clamp-6for content overflow handling- Fixed minimum width for date column maintains layout integrity
src/components/base-ui/BookStory/bookstory_choosebook.tsx (2)
1-11: LGTM!The imports and type definition are well-structured and appropriate for the component's needs.
26-34: LGTM!The Next.js Image component is configured correctly with the
fillprop, explicitsizesattribute, and meaningful alt text frombookName.src/components/base-ui/BookStory/bookstory_text.tsx (4)
1-10: LGTM!The "use client" directive is correctly placed, and the component properly uses React hooks for client-side interactivity. The controlled component pattern in the props type is well-structured.
18-27: LGTM!The auto-resize logic correctly uses
useLayoutEffectto synchronously adjust textarea height before browser paint. The technique of resetting to "0px" before measuringscrollHeightensures accurate height calculation.
29-50: LGTM!The custom Tab-to-indent implementation is well-designed. Key strengths:
preventDefault()correctly overrides default tab navigationrequestAnimationFrame()properly defers cursor positioning until after React's re-render- Dependencies include both
detailandonChangeDetailto avoid stale closure issues
52-93: LGTM!The render output correctly wires up the controlled inputs with the auto-resize ref and Tab handler. The
rows={6}attribute provides a sensible initial size before theuseLayoutEffecttakes over height management.src/components/base-ui/BookStory/bookstory_detail.tsx (2)
1-24: LGTM!The type definition is comprehensive and well-documented. The flexibility of
authorId: string | numberaccommodates different ID systems, and inline comments clearly document default values for optional props.
42-107: LGTM!The render logic is well-structured with proper use of Next.js Image and Link components. The layout correctly handles text overflow with
truncateandline-clamp-2utilities. Note that this file correctly uses--primary_2(line 82), unlike the typo inbookstory_choosebook.tsx.src/app/(main)/page.tsx (1)
1-3: Placeholder implementation noted.The HomePage component is currently a stub. This is consistent with the PR objectives indicating that base UI components are being established. Consider adding a comment or TODO to track the intended implementation.
.github/ISSUE_TEMPLATE/custom.md (2)
5-7: LGTM!The frontmatter formatting is now consistent with proper YAML syntax using quoted empty strings.
11-15: LGTM!Adding the "๊ธฐํ (์ค๋ช ์ถ๊ฐ)" option improves flexibility for issue categorization.
src/components/base-ui/button_without_img.tsx (3)
25-31: LGTM!The
toCssColorhelper correctly handles various color input formats (CSS variables, direct colors, and var() syntax).
5-51: LGTM!The type definition and prop defaults are well-structured. Good practices:
- Sensible defaults for colors and dimensions
- Optional
onClickaccommodates submit buttons- Support for HTML button types
52-56: LGTM!The hover state logic correctly applies hover colors when provided and falls back to default colors appropriately.
src/components/layout/Header.tsx (1)
1-6: LGTM! The directives and imports are correct.The
"use client"directive is necessary for usingusePathname()in Next.js App Router, and all imports (next/link,next/navigation,next/image) are valid in Next.js 16.0.1. No breaking changes affect these directives or imports.
| background: var(--background); | ||
| color: var(--foreground); | ||
| font-family: Arial, Helvetica, sans-serif; | ||
| font-family: "Pretendard Variable", Pretendard, system-ui, sans-serif; |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
#!/bin/bash
# Search for Pretendard font references and loading
rg -n "Pretendard" -C 2
# Also search for @font-face declarations
rg -n "@font-face" -C 2
# Check for Next.js font imports
rg -n "next/font" -C 2
# Check the main layout/app files
fd -e "layout\.(tsx|ts|jsx|js)" -e "app\.(tsx|ts|jsx|js)" | head -10 | xargs -I {} echo "=== {} ===" && cat -n "{}"Repository: checkmo2025/FE
Length of output: 587
๐ Script executed:
#!/bin/bash
# Read the full layout.tsx file
echo "=== src/app/layout.tsx ==="
cat -n src/app/layout.tsx
echo -e "\n=== Search for @import statements with Pretendard ==="
rg "@import.*Pretendard" -A 1 -B 1
echo -e "\n=== Search for font-face in CSS files ==="
rg "@font-face" --type=css -A 5
echo -e "\n=== Check globals.css for font imports ==="
head -60 src/app/globals.css | cat -n
echo -e "\n=== Search for Pretendard in package.json ==="
rg "pretendard" -i -C 2Repository: checkmo2025/FE
Length of output: 3087
๐ Script executed:
#!/bin/bash
# Check for font files in public folder
echo "=== Checking for font files in public ==="
find public -name "*pretendard*" -o -name "*.ttf" -o -name "*.woff*" 2>/dev/null | head -20
echo -e "\n=== Check next.config.js ==="
find . -name "next.config.*" -type f | head -5 | xargs cat -n 2>/dev/null
echo -e "\n=== Check if there are other CSS files that might load fonts ==="
fd -e "\.css" -e "\.scss" | xargs rg -l "font-family|@font-face|@import" | head -10
echo -e "\n=== Look for Pretendard in HTML or tsx files ==="
rg "Pretendard" --type=tsx --type=ts --type=htmlRepository: checkmo2025/FE
Length of output: 522
Add font loading for Pretendard Variable.
The body font-family on line 52 references "Pretendard Variable" but no font loading mechanism exists in the codebase. There is no @font-face declaration, no font files in the public folder, and no import statements. Only the Geist fonts are currently loaded via the Next.js font API.
Add font loading via:
- A
@font-facerule with font files in the public folder, or - An
@import url()statement to a CDN (e.g., Google Fonts, Noto Sans KR), or - A Next.js font import if Pretendard is available on
next/font
Without proper loading, users will see a flash of unstyled text (FOUT) or fallback to system fonts.
๐ค Prompt for AI Agents
In src/app/globals.css around line 52, the body references "Pretendard Variable"
but no font is loaded; add a font-loading mechanism before the font-family
usage: either (A) place Pretendard variable font files in public/fonts and add a
corresponding @font-face declaration in globals.css pointing to those files with
font-display: swap and the correct font-weight/format, (B) add an @import
url(...) at the top of globals.css to a reliable CDN that serves Pretendard, or
(C) import Pretendard via next/font in _app or layout and apply the returned
className; ensure the font is loaded prior to usage, include font-display: swap
to avoid blocking, and update the font-family fallback order if needed.
| .Headline_1 { @apply text-[48px] font-bold leading-[135%] tracking-[-0.1%]; } | ||
| .Headline_2 { @apply text-[36px] font-bold leading-[135%] tracking-[-0.1%]; } | ||
| .Headline_3 { @apply text-[32px] font-bold leading-[135%] tracking-[-0.1%]; } | ||
|
|
||
| /* Subhead */ | ||
| .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];} | ||
| .Subhead_3_1 { @apply text-[18px] font-semibold leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_3_2 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_4_1 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.018px]; } | ||
|
|
There was a problem hiding this comment.
Fix invalid letter-spacing values in typography utilities.
Several utility classes use tracking-[-0.1%] which is invalid CSS. The letter-spacing property does not accept percentage values.
Lines with invalid tracking:
- Lines 57-59:
tracking-[-0.1%]in Headline classes - Lines 62-63, 65-66:
tracking-[-0.1%]in Subhead classes
Additionally, there's inconsistency in tracking units across utilities:
- Some use
tracking-[-0.02px],tracking-[-0.018px],tracking-[-0.014px](px units - valid) - Others use
tracking-[-0.1%](percentage - invalid)
๐ Proposed fix for tracking values
Replace percentage tracking with valid units. If the design intent is -0.1% of font size, use em units instead:
/* Headline */
- .Headline_1 { @apply text-[48px] font-bold leading-[135%] tracking-[-0.1%]; }
- .Headline_2 { @apply text-[36px] font-bold leading-[135%] tracking-[-0.1%]; }
- .Headline_3 { @apply text-[32px] font-bold leading-[135%] tracking-[-0.1%]; }
+ .Headline_1 { @apply text-[48px] font-bold leading-[135%] tracking-[-0.001em]; }
+ .Headline_2 { @apply text-[36px] font-bold leading-[135%] tracking-[-0.001em]; }
+ .Headline_3 { @apply text-[32px] font-bold leading-[135%] tracking-[-0.001em]; }
/* Subhead */
- .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.1%]; }
- .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1%]; }
+ .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.001em]; }
+ .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1em]; }
.Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];}
- .Subhead_3_1 { @apply text-[18px] font-semibold leading-[135%] tracking-[-0.1%]; }
- .Subhead_3_2 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.1%]; }
+ .Subhead_3_1 { @apply text-[18px] font-semibold leading-[135%] tracking-[-0.001em]; }
+ .Subhead_3_2 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.001em]; }
.Subhead_4_1 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.018px]; }Verify the intended tracking values with your design team.
๐ 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.
| .Headline_1 { @apply text-[48px] font-bold leading-[135%] tracking-[-0.1%]; } | |
| .Headline_2 { @apply text-[36px] font-bold leading-[135%] tracking-[-0.1%]; } | |
| .Headline_3 { @apply text-[32px] font-bold leading-[135%] tracking-[-0.1%]; } | |
| /* Subhead */ | |
| .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.1%]; } | |
| .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1%]; } | |
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];} | |
| .Subhead_3_1 { @apply text-[18px] font-semibold leading-[135%] tracking-[-0.1%]; } | |
| .Subhead_3_2 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.1%]; } | |
| .Subhead_4_1 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.018px]; } | |
| /* Headline */ | |
| .Headline_1 { @apply text-[48px] font-bold leading-[135%] tracking-[-0.001em]; } | |
| .Headline_2 { @apply text-[36px] font-bold leading-[135%] tracking-[-0.001em]; } | |
| .Headline_3 { @apply text-[32px] font-bold leading-[135%] tracking-[-0.001em]; } | |
| /* Subhead */ | |
| .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.001em]; } | |
| .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1em]; } | |
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];} | |
| .Subhead_3_1 { @apply text-[18px] font-semibold leading-[135%] tracking-[-0.001em]; } | |
| .Subhead_3_2 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.001em]; } | |
| .Subhead_4_1 { @apply text-[18px] font-medium leading-[135%] tracking-[-0.018px]; } |
๐ค Prompt for AI Agents
In src/app/globals.css around lines 57 to 68 the typography utilities use
invalid percentage values for letter-spacing (e.g. tracking-[-0.1%]) which CSS
does not accept; replace those percentage-based tracking values with valid units
(preferably relative units like em for proportional spacing or px for absolute
spacing) and make tracking units consistent across all Headline/Subhead classes
(convert every tracking-[-0.1%] to an appropriate value such as -0.001em or a
matching px value agreed with design), ensuring the small px values (e.g.
-0.02px, -0.018px) are normalized to the chosen unit system.
| <button | ||
| type="button" | ||
| onClick={onButtonClick} | ||
| className=" | ||
| flex w-[132px] h-[44px] | ||
| px-[16px] py-[12px] | ||
| justify-center items-center gap-[10px] | ||
| rounded-[8px] | ||
| border border-[color:var(--premary_2,#9A7A6B)] | ||
| bg-[color:var(--background,#F9F7F6)] | ||
| text-[color:var(--premary_2,#9A7A6B)] | ||
| Body_1_2 | ||
| whitespace-nowrap | ||
| " | ||
| > | ||
| ๋ณ๊ฒฝํ๊ธฐ | ||
| </button> |
There was a problem hiding this comment.
Fix typo in CSS variable name.
Lines 61, 62, and 63 use --premary_2 but should be --primary_2 (note the missing 'i'). This typo will prevent the intended colors from applying. The correct spelling is used in bookstory_detail.tsx line 82.
๐ Proposed fix
className="
flex w-[132px] h-[44px]
px-[16px] py-[12px]
justify-center items-center gap-[10px]
rounded-[8px]
- border border-[color:var(--premary_2,#9A7A6B)]
- bg-[color:var(--background,#F9F7F6)]
- text-[color:var(--premary_2,#9A7A6B)]
+ border border-[color:var(--primary_2,#9A7A6B)]
+ bg-[color:var(--background,#F9F7F6)]
+ text-[color:var(--primary_2,#9A7A6B)]
Body_1_2
whitespace-nowrap
"๐ 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.
| <button | |
| type="button" | |
| onClick={onButtonClick} | |
| className=" | |
| flex w-[132px] h-[44px] | |
| px-[16px] py-[12px] | |
| justify-center items-center gap-[10px] | |
| rounded-[8px] | |
| border border-[color:var(--premary_2,#9A7A6B)] | |
| bg-[color:var(--background,#F9F7F6)] | |
| text-[color:var(--premary_2,#9A7A6B)] | |
| Body_1_2 | |
| whitespace-nowrap | |
| " | |
| > | |
| ๋ณ๊ฒฝํ๊ธฐ | |
| </button> | |
| <button | |
| type="button" | |
| onClick={onButtonClick} | |
| className=" | |
| flex w-[132px] h-[44px] | |
| px-[16px] py-[12px] | |
| justify-center items-center gap-[10px] | |
| rounded-[8px] | |
| border border-[color:var(--primary_2,#9A7A6B)] | |
| bg-[color:var(--background,#F9F7F6)] | |
| text-[color:var(--primary_2,#9A7A6B)] | |
| Body_1_2 | |
| whitespace-nowrap | |
| " | |
| > | |
| ๋ณ๊ฒฝํ๊ธฐ | |
| </button> |
๐ค Prompt for AI Agents
In src/components/base-ui/BookStory/bookstory_choosebook.tsx around lines 53 to
69 the CSS variable is misspelled as --premary_2 in three places (border,
bg/text color entries); update each occurrence to --primary_2 so the intended
theme colors apply (replace --premary_2 with --primary_2 in the className
string).
| </div> | ||
|
|
||
| </div> | ||
|
|
||
| ); | ||
| } |
There was a problem hiding this comment.
Remove extra closing </div> tag.
Line 72 contains an extraneous closing </div> tag that breaks the JSX structure. The correct structure should close the main container at line 72, but there's an extra </div> before it.
๐ Proposed fix
</button>
</div>
- </div>
-
);
}๐ 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.
| </div> | |
| </div> | |
| ); | |
| } | |
| </div> | |
| ); | |
| } |
๐ค Prompt for AI Agents
In src/components/base-ui/BookStory/bookstory_choosebook.tsx around lines 70 to
75, there is an extra closing </div> that breaks the JSX hierarchy; remove the
extraneous </div> (the one before the main container's closing tag) so the JSX
tags properly nest and the main container closes only once.
| export default function BookstoryDetail({ | ||
| imageUrl = "/bookstory_example.svg", | ||
| authorName, | ||
| authorNickname, | ||
| authorId, | ||
| profileImgSrc = "/profile2.svg", | ||
| subscribeText = "๊ตฌ๋ ", | ||
| onSubscribeClick, | ||
| bookTitle, | ||
| bookAuthor, | ||
| bookDetail, | ||
| authorHref, | ||
| className = "", | ||
| }: BookstoryDetailProps) { | ||
| const href = authorHref ?? `/profile/${authorId}`; |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
#!/bin/bash
# Check if public directory exists and find the default image files
find . -type d -name "public" 2>/dev/null
# Search for the specific image files mentioned in the review
echo "=== Searching for bookstory_example.svg ==="
fd -t f "bookstory_example.svg" . 2>/dev/null || echo "Not found"
echo "=== Searching for profile2.svg ==="
fd -t f "profile2.svg" . 2>/dev/null || echo "Not found"
# Also check what's in public directory if it exists
echo "=== Contents of public directory ==="
find ./public -type f 2>/dev/null | head -20 || echo "public directory not found or empty"Repository: checkmo2025/FE
Length of output: 635
Fix missing default image file.
The imageUrl default prop references /bookstory_example.svg which does not exist in the public directory. This will result in broken images when the component is used without an imageUrl prop. Either add the missing file or use an existing alternative like /booksample.svg.
The profileImgSrc default (/profile2.svg) exists and is correct.
๐ค Prompt for AI Agents
In src/components/base-ui/BookStory/bookstory_detail.tsx around lines 26 to 40,
the default imageUrl points to /bookstory_example.svg which isn't present in
public causing broken images; either add the missing
/public/bookstory_example.svg file or change the default value to an existing
asset (e.g., /booksample.svg) and update any import/path references accordingly
so the component uses a valid image when imageUrl is omitted.
| text-[13px] flex items-center justify-center gap-2" | ||
| > | ||
| <Image src="/icon_plus.svg" alt="icon_plus" width={24} height={24} /> | ||
| <span className="text-color-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> |
There was a problem hiding this comment.
Use standard Tailwind class text-white instead of text-color-white.
The class text-color-white is not a standard Tailwind utility. Use text-white for white text color.
๐ Proposed fix
- <span className="text-color-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span>
+ <span className="text-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span>๐ 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.
| <span className="text-color-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> | |
| <span className="text-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> |
๐ค Prompt for AI Agents
In src/components/base-ui/home/home_bookclub.tsx around line 97, the span uses a
non-standard Tailwind class "text-color-white"; replace it with the standard
utility "text-white" to ensure the text color applies correctly and aligns with
Tailwind conventions.
| import React from "react"; | ||
|
|
||
| type NotificationType = "like" | "comment"; | ||
|
|
||
| type NotificationElementProps = { | ||
| nickname: string; | ||
| type: NotificationType; | ||
| commentContent?: string; | ||
| timeLabel: string; // "์ง๊ธ", "2๋ถ ์ " | ||
| isLatest?: boolean; | ||
| }; | ||
|
|
||
| export default function NotificationElement({ | ||
| nickname, | ||
| type, | ||
| commentContent, | ||
| timeLabel, | ||
| isLatest = false, | ||
| }: NotificationElementProps) { | ||
| const textColorClass = isLatest | ||
| ? "text-[color:var(--Gray_7,#2C2C2C)]" | ||
| : "text-[color:var(--Gray_3,#BBB)]"; | ||
|
|
||
| const message = | ||
| type === "like" | ||
| ? `${nickname}๋์ด ์ข์์๋ฅผ ๋๋ ์ต๋๋ค.` | ||
| : `${nickname}๋์ด ๋๊ธ์ ๋จ๊ฒผ์ต๋๋ค.${commentContent ? `: ${commentContent}` : ""}`; | ||
|
|
||
| return ( | ||
| <div className="flex w-[364px] px-[16px] py-[20px] justify-between items-center border-b border-b-[color:var(--Subbrown_4,#EAE5E2)] bg-[color:var(--White,#FFF)]"> | ||
| {/* left */} | ||
| <div className="flex items-center gap-[12px] min-w-0"> | ||
|
|
||
| {isLatest ? ( | ||
| <span className="w-[12px] h-[12px] rounded-full bg-red-500 shrink-0" /> | ||
| ) : (<span className="w-[12px] h-[12px] shrink-0" />)} | ||
|
|
||
| <p className={`${textColorClass} Body_1_2 truncate`}>{message}</p> | ||
| </div> | ||
|
|
||
| {/* right */} | ||
| <span className="shrink-0 text-[color:var(--Gray_3,#BBB)] Body_1_2"> | ||
| {timeLabel} | ||
| </span> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
๐ ๏ธ Refactor suggestion | ๐ Major
Duplicate component: identical to home/notification_element.tsx.
This file is virtually identical to src/components/base-ui/home/notification_element.tsx. Extract a single shared NotificationElement component to a common location (e.g., src/components/base-ui/common/) and import it where needed.
๐ Suggested structure
src/components/base-ui/common/notification_element.tsx # single source of truth
src/components/base-ui/Profile/ # re-export or import from common
src/components/base-ui/home/ # re-export or import from common
๐ค Prompt for AI Agents
In src/components/base-ui/Profile/notification_element.tsx lines 1-47, this
component is duplicated from
src/components/base-ui/home/notification_element.tsx; extract a single shared
NotificationElement into src/components/base-ui/common/notification_element.tsx
(export the component and its props/type), update both Profile and home modules
to import that common component (or create small re-export files under Profile/
and home/ that import and re-export the common one), and remove or replace the
duplicate implementation so there is one source of truth; ensure TypeScript
types are exported/imported and update any relative import paths accordingly.
| type BookCoverCardProps = { | ||
| imgUrl?: string; | ||
| title: string; | ||
| author: string; | ||
|
|
||
| liked: boolean; | ||
| onLikeChange: (next: boolean) => void; | ||
|
|
||
| onCardClick?: () => void; | ||
| className?: string; | ||
| }; | ||
|
|
||
| export default function Search_BookCoverCard({ | ||
| imgUrl, | ||
| title, | ||
| author, | ||
| liked, | ||
| onLikeChange, | ||
| onCardClick, | ||
| className = "", | ||
| }: BookCoverCardProps) { | ||
| const coverSrc = imgUrl && imgUrl.length > 0 ? imgUrl : "/booksample.svg"; | ||
|
|
||
| return ( | ||
| <div | ||
| onClick={onCardClick} | ||
| className={`relative flex w-[332px] h-[436px] p-[16px] flex-col justify-end items-start gap-[12px] overflow-hidden ${ | ||
| onCardClick ? "cursor-pointer" : "" | ||
| } ${className}`} | ||
| > | ||
| <Image src={coverSrc} alt={title} fill sizes="244px" className="object-cover" /> | ||
|
|
||
| <div className="relative z-[1] flex flex-col items-start gap-[10px]"> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| onLikeChange(!liked); | ||
| }} | ||
| className="w-[24px] h-[24px] shrink-0" | ||
| > | ||
| <Image src={liked ? "/red_heart.svg" : "/gray_heart.svg"} alt="" width={24} height={24} /> | ||
| </button> | ||
|
|
||
| <div className="flex flex-col items-start gap-[6px] min-w-0"> | ||
| <p className="text-[color:var(--White,#FFF)] Subhead_1 truncate">{title}</p> | ||
| <p className="text-[color:var(--White,#FFF)] Subhead_4 truncate">{author}</p> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
๐ ๏ธ Refactor suggestion | ๐ Major
Significant code duplication with News/recommendbook_element.tsx.
This component is nearly identical to src/components/base-ui/News/recommendbook_element.tsx, differing only in:
- Component name (Search_BookCoverCard vs BookCoverCard)
- Dimensions (w-[332px] h-[436px] vs w-[244px] h-[320px])
- Gap spacing (gap-[6px] vs gap-[4px])
Consider extracting a shared base component and accepting dimension/spacing props, or creating a single component with variant configurations to eliminate duplication and improve maintainability.
๐ Suggested approach
Create a shared component:
// src/components/base-ui/shared/BookCoverCard.tsx
type BookCoverCardProps = {
imgUrl?: string;
title: string;
author: string;
liked: boolean;
onLikeChange: (next: boolean) => void;
onCardClick?: () => void;
className?: string;
variant?: 'search' | 'news'; // or pass dimensions directly
};
export default function BookCoverCard({ variant = 'news', ...props }: BookCoverCardProps) {
const dimensions = variant === 'search'
? { width: 'w-[332px]', height: 'h-[436px]', gap: 'gap-[6px]' }
: { width: 'w-[244px]', height: 'h-[320px]', gap: 'gap-[4px]' };
// ... rest of implementation
}Then re-export from Search and News directories with appropriate defaults.
๐ค Prompt for AI Agents
In src/components/base-ui/Search/search_recommendbook.tsx (lines 5-56), this
component duplicates src/components/base-ui/News/recommendbook_element.tsx with
only size/spacing and name differences; extract a shared BookCoverCard component
(e.g., src/components/base-ui/shared/BookCoverCard.tsx) that accepts imgUrl,
title, author, liked, onLikeChange, onCardClick, className and either a variant
prop ('search'|'news') or explicit dimension/spacing props, implement the shared
markup/behavior (preserve Image fallback, stopPropagation in like button, click
handling), then update both Search/search_recommendbook.tsx and
News/recommendbook_element.tsx to import the shared component and re-export thin
wrappers that pass the correct variant or className for their sizes/gaps so
duplication is removed and behavior remains identical.
| <div className="flex-1 min-w-0"> | ||
| <div className="flex items-start justify-between gap-[12px]"> | ||
| <div className="flex items-center gap-[8px] min-w-0"> | ||
| <Image src="/profile5.svg" alt="" width={24} height={24} /> |
There was a problem hiding this comment.
Hard-coded profile image breaks reporter differentiation.
The profile image is hard-coded to /profile5.svg for all reporters, which prevents visual differentiation between different reporters. Additionally, the empty alt attribute fails to provide context for screen readers.
๐ Recommended fix
Add a profileImageUrl prop and provide meaningful alt text:
type SettingReportListProps = {
badgeText: string;
reporterName: string;
reportedAt: string;
content: string;
+ profileImageUrl?: string;
className?: string;
};
export default function Setting_ReportList({
badgeText,
reporterName,
reportedAt,
content,
+ profileImageUrl = "/profile5.svg",
className = "",
}: SettingReportListProps) {
return (
// ...
- <Image src="/profile5.svg" alt="" width={24} height={24} />
+ <Image src={profileImageUrl} alt={`${reporterName}์ ํ๋กํ`} width={24} height={24} />Committable suggestion skipped: line range outside the PR's diff.
๐ค Prompt for AI Agents
In src/components/base-ui/Settings/setting_report_list.tsx around line 46, the
Image is hard-coded to /profile5.svg and has an empty alt; change the component
to accept a profileImageUrl (or use reporter.profileImageUrl) and render <Image
src={profileImageUrl || '/default-profile.svg'} alt={`${reporter.name ||
'Reporter'} profile`} width={24} height={24} /> so each reporter can show a
unique image and screen readers get meaningful context; also update any parent
callers to pass profileImageUrl or ensure the reporter object includes it.
| "checkmo/.next/types/**/*.ts", | ||
| "checkmo/.next/dev/types/**/*.ts", |
There was a problem hiding this comment.
๐งฉ Analysis chain
๐ Script executed:
# First, let's read the tsconfig.json file to see the actual content
cat -n tsconfig.jsonRepository: checkmo2025/FE
Length of output: 1157
๐ Script executed:
# Check if the "checkmo" directory exists in the project
find . -type d -name "checkmo" 2>/dev/null | head -20Repository: checkmo2025/FE
Length of output: 40
๐ Script executed:
# Search for "checkmo" references throughout the codebase
rg "checkmo" --type json --type yaml --type ts --type js 2>/dev/null | head -30Repository: checkmo2025/FE
Length of output: 275
๐ Script executed:
# Check the project structure, particularly looking for monorepo indicators
ls -la
cat package.json 2>/dev/null | head -50Repository: checkmo2025/FE
Length of output: 2039
Remove leftover "checkmo/" paths from include section.
Lines 35โ36 reference "checkmo/.next/types/**/*.ts" and "checkmo/.next/dev/types/**/*.ts", but the checkmo/ directory does not exist in the project. The project is a single Next.js application with the package name "checkmo", not a monorepo. The correct paths without the checkmo/ prefix already exist on lines 38โ39.
Remove lines 35โ36 to eliminate dead includes and avoid type resolution confusion.
๐ค Prompt for AI Agents
In tsconfig.json around lines 35 to 36, remove the two leftover include entries
"checkmo/.next/types/**/*.ts" and "checkmo/.next/dev/types/**/*.ts" because the
project is not a monorepo and those paths don't exist; delete those lines so the
existing correct includes (without the "checkmo/" prefix on lines 38โ39) remain
and prevent dead includes/type resolution confusion.
![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
์ ๋ฐ์ ์ผ๋ก ์๋ก์ด UI ์ปดํฌ๋ํธ๋ค์ ๋ง์ด ์ถ๊ฐํด์ฃผ์ จ๋ค์. ๊ณ ์ํ์ จ์ต๋๋ค! ์ฝ๋ ๋ฆฌ๋ทฐ์์๋ ์ฃผ๋ก CSS ๋ณ์๋ช ์คํ, Tailwind CSS ๋ฌธ๋ฒ ์ค๋ฅ, ํ๋์ฝ๋ฉ๋ ๊ฐ ์ฌ์ฉ, ๊ทธ๋ฆฌ๊ณ ์ปดํฌ๋ํธ ์ค๋ณต๊ณผ ๊ฐ์ ๋ถ๋ถ์ ์ค์ฌ์ผ๋ก ํผ๋๋ฐฑ์ ๋๋ ธ์ต๋๋ค. ์ ์๋ ์์ ์ฌํญ๋ค์ ๋ฐ์ํ๋ฉด ์ฝ๋์ ์ผ๊ด์ฑ๊ณผ ์ ์ง๋ณด์์ฑ์ด ๋ ํฅ์๋ ๊ฒ์ ๋๋ค.
| flex w-[60px] px-[4px] py-[4px] | ||
| justify-center items-center gap-[10px] shrink-0 | ||
| rounded-[4px] | ||
| bg-[color:var(--red,#FF5151)] |
There was a problem hiding this comment.
์ ์๋์ง ์์ CSS ๋ณ์ --red๋ฅผ ์ฌ์ฉํ๊ณ ์์ต๋๋ค. ์ด๋ก ์ธํด ๋ฐฐ๊ฒฝ์์ด ์ ์ฉ๋์ง ์์ต๋๋ค. globals.css์ --Red ๋๋ ๋น์ทํ ์ด๋ฆ์ ๋ณ์๋ฅผ ์ถ๊ฐํ๊ฑฐ๋, #FF5151๊ณผ ๊ฐ์ ์ค์ ์์ ๊ฐ์ ์ง์ ์ฌ์ฉํด์ผ ํฉ๋๋ค.
| bg-[color:var(--red,#FF5151)] | |
| bg-[#FF5151] |
| /* Subhead */ | ||
| .Subhead_1 { @apply text-[24px] font-semibold leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_2 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.1%]; } | ||
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];} |
There was a problem hiding this comment.
.Subhead_3 ํด๋์ค ์ ์์ ๋ฌธ๋ฒ ์ค๋ฅ๊ฐ ์์ต๋๋ค. @apply ์ง์์ด ๋ค์ ์ธ๋ฏธ์ฝ๋ก (;)์ด ์๊ณ , ๋ฐ๋ก ๋ค์ ๋ซ๋ ์ค๊ดํธ(})๊ฐ ๋ถ์ด ์์ด CSS ํ์ฑ ์ค๋ฅ๋ฅผ ์ ๋ฐํฉ๋๋ค. ์ฌ๋ฐ๋ฅธ ํ์์ผ๋ก ์์ ํด์ฃผ์ธ์.
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px];} | |
| .Subhead_3 { @apply text-[20px] font-semibold leading-[135%] tracking-[-0.02px]; } |
| buttonText = "๊ตฌ๋ ", | ||
| }: ListSubscribeElementProps) { | ||
| return ( | ||
| <div className="flex w-[296px] h-[66px] px-[14px] py-[8px] items-center gap-[8px] rounded-[8px] border border-[var(--Subbrown_4)] bg-white"> |
There was a problem hiding this comment.
Tailwind CSS์์ CSS ๋ณ์๋ฅผ ์ฌ์ฉํ์ฌ ํ
๋๋ฆฌ ์์์ ์ง์ ํ ๋ border-[color:var(--...)] ํ์์ ์ฌ์ฉํด์ผ ํฉ๋๋ค. ํ์ฌ border-[var(--Subbrown_4)]๋ ์๋ชป๋ ๋ฌธ๋ฒ์
๋๋ค.
| <div className="flex w-[296px] h-[66px] px-[14px] py-[8px] items-center gap-[8px] rounded-[8px] border border-[var(--Subbrown_4)] bg-white"> | |
| <div className="flex w-[296px] h-[66px] px-[14px] py-[8px] items-center gap-[8px] rounded-[8px] border border-[color:var(--Subbrown_4)] bg-white"> |
| text-[13px] flex items-center justify-center gap-2" | ||
| > | ||
| <Image src="/icon_plus.svg" alt="icon_plus" width={24} height={24} /> | ||
| <span className="text-color-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> |
There was a problem hiding this comment.
text-color-white๋ ์ ํจํ Tailwind ํด๋์ค๊ฐ ์๋๋๋ค. text-white๋ฅผ ์ฌ์ฉํด์ผ ํฉ๋๋ค.
| <span className="text-color-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> | |
| <span className="text-white Subhead_4_1">๋ชจ์ ์์ฑํ๊ธฐ</span> |
| > | ||
| {open ? ( | ||
| <div className="flex items-center justify-center gap-1"> | ||
| <span className="text-[color:var(--Gray_7)] Body_1_2 font-weight: 500">์ ๊ธฐ</span> |
There was a problem hiding this comment.
font-weight: 500์ ์ ํจํ Tailwind ํด๋์ค๊ฐ ์๋๋๋ค. font-medium ํด๋์ค๋ฅผ ์ฌ์ฉํด์ผ ํฉ๋๋ค.
| <span className="text-[color:var(--Gray_7)] Body_1_2 font-weight: 500">์ ๊ธฐ</span> | |
| <span className="text-[color:var(--Gray_7)] Body_1_2 font-medium">์ ๊ธฐ</span> |
| </div> | ||
| ) : ( | ||
| <div className="flex items-center justify-center gap-1"> | ||
| <span className="text-[color:var(--Gray_7)] Body_1_2 Body_1_2">์ ์ฒด๋ณด๊ธฐ</span> |
There was a problem hiding this comment.
Body_1_2 ํด๋์ค๊ฐ ์ค๋ณต์ผ๋ก ์ฌ์ฉ๋์์ต๋๋ค. ํ๋๋ ์ ๊ฑฐํด๋ ๋ฉ๋๋ค.
| <span className="text-[color:var(--Gray_7)] Body_1_2 Body_1_2">์ ์ฒด๋ณด๊ธฐ</span> | |
| <span className="text-[color:var(--Gray_7)] Body_1_2">์ ์ฒด๋ณด๊ธฐ</span> |
| className?: string; | ||
| }; | ||
|
|
||
| export default function Subscribe({ |
There was a problem hiding this comment.
์ปดํฌ๋ํธ ์ด๋ฆ(Subscribe)๊ณผ ํ์ผ ์ด๋ฆ(subscribe_element.tsx)์ด ์ผ์นํ์ง ์์ต๋๋ค. ํผ๋์ ํผํ๊ธฐ ์ํด ์ด๋ฆ์ ํต์ผํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
| <p className="text-[#000] Subhead_3 truncate">{bookName}</p> | ||
| <p className="text-[#757575] Subhead_4_1 truncate">{author}</p> |
There was a problem hiding this comment.
ํ๋์ฝ๋ฉ๋ ์์ ๊ฐ(#000, #757575)์ ์ฌ์ฉํ๊ณ ์์ต๋๋ค. globals.css์ ์ ์๋ ์์ ํ ํฐ(์: var(--Black), var(--Gray_5))์ ์ฌ์ฉํ๋ฉด ๋์์ธ ์ผ๊ด์ฑ์ ์ ์งํ๊ณ ์ ์ง๋ณด์ํ๊ธฐ ์ฉ์ดํด์ง๋๋ค.
| <p className="text-[#000] Subhead_3 truncate">{bookName}</p> | |
| <p className="text-[#757575] Subhead_4_1 truncate">{author}</p> | |
| <p className="text-[color:var(--Black)] Subhead_3 truncate">{bookName}</p> | |
| <p className="text-[color:var(--Gray_5)] Subhead_4_1 truncate">{author}</p> |
| <button | ||
| type="button" | ||
| onClick={onSubscribeClick} | ||
| className="flex px-[17px] py-[8px] justify-center items-center gap-[10px] rounded-[8px] bg-[#9A7A6B] text-white text-[color:var(--White,#FFF)] text-[12px] font-semibold leading-[100%] tracking-[-0.012px] whitespace-nowrap" |
There was a problem hiding this comment.
ํ๋์ฝ๋ฉ๋ ์์ ๊ฐ bg-[#9A7A6B]์ ์ฌ์ฉํ๊ณ ์์ต๋๋ค. globals.css์ ์ ์๋ --primary_2 ๋ณ์๋ฅผ ์ฌ์ฉํด์ฃผ์ธ์.
| className="flex px-[17px] py-[8px] justify-center items-center gap-[10px] rounded-[8px] bg-[#9A7A6B] text-white text-[color:var(--White,#FFF)] text-[12px] font-semibold leading-[100%] tracking-[-0.012px] whitespace-nowrap" | |
| className="flex px-[17px] py-[8px] justify-center items-center gap-[10px] rounded-[8px] bg-[color:var(--primary_2)] text-white text-[color:var(--White,#FFF)] text-[12px] font-semibold leading-[100%] tracking-[-0.012px] whitespace-nowrap" |
| </p> | ||
| </div> | ||
|
|
||
|
|
There was a problem hiding this comment.
์ด ๋ถ๋ถ์ ๋น์ด์๋ ์ฝ๋๊ฐ ์์ต๋๋ค. ์๋๋ ๊ฒ์ธ์ง ํ์ธํ๊ณ , ๋ถํ์ํ๋ค๋ฉด ์ ๊ฑฐํด์ฃผ์ธ์.
4 participants
๐ก To Reviewers
์ด PR์ feat-10 ์์ ์ ๋๋ค.
์๋ณธ ๋ ํฌ์ push ๊ถํ์ด ์์ด์ fork์์ PR ์ฌ๋ ธ์ต๋๋ค
๐ฅ ์์ ๋ด์ฉ (๊ฐ๋ฅํ ๊ตฌ์ฒด์ ์ผ๋ก ์์ฑํด ์ฃผ์ธ์)
ํ์ผ๊ตฌ์กฐ
[global css] ๊ธ์จ์ฒด, ์ ํธ์ถ๋ฐฉ๋ฒ
button_without_img
๊ฐ ํ์ด์ง ๋ณ ์์ฑํ ui
ํ์ผ๊ตฌ์กฐ
src/components/base_ui
-> ํ์
/home
/Auth
/Group-search
/Group-create
/BookStory
/News
/Search
/Profile
/Settings
/button_without_img.tsx
[global css] ๊ธ์จ์ฒด, ์ ํธ์ถ๋ฐฉ๋ฒ
ex)
Subhead_4_1 ->
/* Subhead_4.1 /
font-family: "Pretendard Variable";
font-size: 18px;
font-style: normal;
font-weight: 500;
line-height: 135%; / 24.3px /
letter-spacing: -0.018px;
๊ฐ ํธ์ถ๋จ
๊ธ์จ์ฒด ์์์ / Subhead_4.1 */ ์ด๋ ๊ฒ ํ๊ธฐ๋์ด ์์ผ๋ ํธ์ถํ์๊ณ ์๋ง ์ ๊ฒฝ์จ์ฃผ์๋ฉด ๋ฉ๋๋ค.
ํธ์ถ ๊ท์น
์ด๋ฆ: ๋์๋ฌธ์ ํฌํจ ๊ทธ๋๋ก
[.] ์ _๋ก ๋์ฒด
์ ํ ํฐ์ Gray_7 ๊ฐ์ ํ์ ( ex text-[color:var(--Gray_7)]
button_without_img
์ด๋ฏธ์ง๊ฐ ์์ ์์ ์ฌ์ฉํ ์ ์๋ ๋ฒํผ ํผ
์ฌ์ฉ ์์ (๋น์ด์์ ์ ๊ธฐ๋ณธ๊ฐ์ด ๋ค์ด๊ฐ)
์๋ฌด ๋ด์ฉ ์์ ์์ ๊ธฐ๋ณธ๊ฐ์ด ๋ค์ด๊ฐ
// ์๋ ๊ธฐ๋ณธ๊ฐ ๊ฒธ ์ฌ์ฉ๋ฐฉ๋ฒ
<ButtonWithoutImg
text="๋ฑ๋ก"
onClick={() => {} }
bgColorVar="--premary_2"
borderColorVar="--premary_2"
textColorVar="--White"
hoverBgColorVar="--premary_2"
hoverBorderColorVar="--premary_2"
hoverTextColorVar="--White"
width={132}
height={44}
/>
์ฑ ๋ชจ_ํํ๋ฉด
๋ก๊ทธ์ธ/ํ์๊ฐ์
๊ฐ๋ณ๋ก ๋ง๋๋๊ฒ ๋์๋ณด์ฌ ์๋ตํ์์ต๋๋ค.
๋ชจ์ ๊ฒ์ / ๋ชจ์ ์์ฑ
-> ์ ์ชฝ์ด๋ผ์ ์ผ๋จ ๋ณด๋ฅํ์ต๋๋ค.
์ฑ ์ด์ผ๊ธฐ
์์
๊ฒ์
๋ง์ดํ์ด์ง&๋ค๋ฅธ์ฌ๋ ํ๋กํ
notification_element
ํํ๋ฉด์ ์๋ ๊ฒ๊ณผ ๊ฐ์ต๋๋ค ๊ณ ๋ก ์๋ตํฉ๋๋ค notification_element
subscribe
์ค์
setting_news_list
์์์ news_list์ maxํฌ๊ธฐ ๋นผ๊ณค ๋ค๋ฅด์ง ์์ต๋๋ค ๊ณ ๋ก ์๋ตํฉ๋๋ค.
setting_report_list
๐ค ์ถํ ์์ ์์
๋ชจ์ ๊ฒ์ / ๋ชจ์ ์์ฑ
๐ธ ์์ ๊ฒฐ๊ณผ (์คํฌ๋ฆฐ์ท)
๐ ๊ด๋ จ ์ด์
Feat-10
Summary by CodeRabbit
New Features
Style
โ๏ธ Tip: You can customize this high-level summary in your review settings.