LC-2858 user 마그넷 페이지 UI#2136
Hidden character warning
Conversation
LC-2812 HR 챌린지 상세페이지 개발
LC-2812 HR 챌린지 상세페이지 개발
- 버튼비활성화 로직 1개 -> 2개로 분리
LC-2867 0회차 제출 로직 분기 처리
Summary of ChangesHello @yeeun426, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 사용자가 다양한 커리어 관련 자료집 콘텐츠를 탐색하고 관리할 수 있는 새로운 마그넷 리스트 페이지의 사용자 인터페이스를 구현합니다. 이 페이지는 탭 기반 탐색, 카테고리 필터링, 개별 자료집 항목을 표시하는 카드 그리드, 그리고 콘텐츠를 효율적으로 탐색하기 위한 페이지네이션 기능을 제공합니다. Highlights
🧠 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. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces new UI components for a library list page, including a banner, tab navigation, filter dropdown, and library cards. The changes are well-structured and follow a component-based approach. I've identified a few areas for improvement regarding hardcoded values and potential accessibility concerns.
| <button | ||
| type="button" | ||
| className="flex items-center gap-1 rounded-xs bg-point p-2.5 text-xxsmall12 font-medium text-neutral-20" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| }} | ||
| > | ||
| <BellIcon width={16} height={16} /> | ||
| <span>알림 설정</span> | ||
| </button> |
There was a problem hiding this comment.
알림 설정 버튼의 onClick 핸들러에서 e.preventDefault()를 호출하고 있습니다. 이는 Link 컴포넌트의 기본 동작을 막기 위함으로 보입니다. 하지만 Link 컴포넌트 내부에 button이 중첩되어 있어 시맨틱적으로 올바르지 않으며, 접근성 문제가 발생할 수 있습니다. Link 컴포넌트의 href 속성과 button의 onClick 이벤트가 충돌할 가능성도 있습니다. 알림 설정 기능은 별도의 컴포넌트나 로직으로 분리하여 Link의 자식으로 두지 않는 것이 좋습니다. 예를 들어, LibraryCard 자체를 Link로 만들고, 알림 설정 버튼은 LibraryCard 외부에서 관리하거나, LibraryCard 내부에서 Link와 별개로 동작하도록 구현할 수 있습니다. (Repository Style Guide: 338, Revealing Hidden Logic (Single Responsibility))
References
- Avoid hidden side effects; functions should only perform actions implied by their signature (SRP). (link)
| {status === 'notified' && ( | ||
| <button | ||
| type="button" | ||
| className="flex items-center gap-1 rounded-xs bg-neutral-70 p-2.5 text-xxsmall12 font-medium text-white" | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| }} | ||
| > | ||
| <BellIcon width={16} height={16} /> | ||
| <span>알림 설정 완료</span> | ||
| </button> |
There was a problem hiding this comment.
이 button도 위의 upcoming 상태의 버튼과 동일하게 Link 컴포넌트 내부에 중첩되어 있어 시맨틱적으로 올바르지 않으며, 접근성 문제가 발생할 수 있습니다. Link 컴포넌트의 href 속성과 button의 onClick 이벤트가 충돌할 가능성도 있습니다. 알림 설정 완료 기능도 별도의 컴포넌트나 로직으로 분리하여 Link의 자식으로 두지 않는 것이 좋습니다. (Repository Style Guide: 338, Revealing Hidden Logic (Single Responsibility))
References
- Avoid hidden side effects; functions should only perform actions implied by their signature (SRP). (link)
| const CATEGORY_FILTER_LIST = [ | ||
| { caption: '카테고리 1', value: 'CATEGORY_1' }, | ||
| { caption: '카테고리 2', value: 'CATEGORY_2' }, | ||
| { caption: '카테고리 3', value: 'CATEGORY_3' }, | ||
| { caption: '카테고리 4', value: 'CATEGORY_4' }, | ||
| { caption: '카테고리 5', value: 'CATEGORY_5' }, |
There was a problem hiding this comment.
현재 CATEGORY_FILTER_LIST에 하드코딩된 카테고리 목록이 있습니다. 이 목록은 API 연동 시 실제 데이터로 변경되어야 합니다. 또한, TODO 주석으로 이 부분이 명시되어 있지만, 실제 API 연동이 이루어질 때까지는 이 목록이 변경되지 않을 가능성이 있으므로, 이 상수를 별도의 파일로 분리하여 관리하는 것이 좋습니다. 이렇게 하면 나중에 API 연동 시 해당 파일만 수정하면 되므로 유지보수성이 향상됩니다. (Repository Style Guide: 19, Naming Magic Numbers)
References
- Replace magic numbers with named constants for clarity. (link)
| // TODO: API 연동 시 실제 데이터로 변경 | ||
| function LibraryGrid() { | ||
| return ( | ||
| <div className="grid grid-cols-1 gap-y-[54px] md:grid-cols-4 md:gap-x-5"> | ||
| <LibraryCard | ||
| href="/library/1" | ||
| thumbnail={undefined} | ||
| category="카테고리 분류" | ||
| title="제목이 한줄 일때" | ||
| date="2025. 01. 30" | ||
| status="upcoming" | ||
| /> | ||
| <LibraryCard | ||
| href="/library/2" | ||
| thumbnail={undefined} | ||
| category="카테고리 분류" | ||
| title="제목이 들어갑니다 제목이 들어갑니다 제목이 들어갑니다 제목이 들어갑니다 제목" | ||
| date="2025. 01. 30" | ||
| status="notified" | ||
| /> | ||
| <LibraryCard | ||
| href="/library/3" | ||
| thumbnail={undefined} | ||
| category="카테고리 분류" | ||
| title="나의 경험을 200% 활용하여 제작하는 자기소개서 2주 완성 챌린지" | ||
| /> | ||
| <LibraryCard | ||
| href="/library/4" | ||
| thumbnail={undefined} | ||
| category="카테고리 분류" | ||
| title="제목이 들어갑니다 제목이 들어갑니다 제목이 들어갑니다 제목이 들어갑니다 제목" | ||
| date="2025. 01. 30" | ||
| status="published" | ||
| /> |
There was a problem hiding this comment.
현재 LibraryGrid 컴포넌트 내부에 LibraryCard 컴포넌트들이 하드코딩되어 있습니다. 이 부분은 API 연동 시 실제 데이터로 변경되어야 합니다. TODO 주석으로 이 부분이 명시되어 있지만, 실제 API 연동이 이루어질 때까지는 이 목록이 변경되지 않을 가능성이 있으므로, LibraryGrid 컴포넌트가 데이터를 props로 받아 렌더링하도록 구조를 변경하는 것이 좋습니다. 이렇게 하면 LibraryGrid 컴포넌트의 재사용성이 높아지고, Content 컴포넌트에서 데이터를 관리하게 되어 관심사의 분리가 명확해집니다. (Repository Style Guide: 40, Abstracting Implementation Details)
References
- Abstract complex logic/interactions into dedicated components/HOCs. (link)
| <img | ||
| src="/icons/banner-folder.svg" | ||
| className="absolute right-[170px] top-1/2 hidden size-[200px] -translate-y-1/2 opacity-50 md:block" | ||
| alt="" | ||
| /> |
There was a problem hiding this comment.
배너 이미지의 alt 속성이 비어 있습니다. 이미지가 순수하게 장식용이라면 alt=""로 두는 것이 맞지만, 이 이미지가 어떤 의미를 전달하거나 중요한 정보를 포함하고 있다면 적절한 alt 텍스트를 제공해야 합니다. 현재 banner-folder.svg 파일명으로 미루어 보아 폴더 이미지로 추정되는데, 이 이미지가 배너의 내용을 시각적으로 보조하는 역할을 한다면 alt 텍스트를 추가하는 것이 좋습니다. 예를 들어, "렛츠커리어 자료집 배너 이미지"와 같이 구체적인 설명을 추가할 수 있습니다.
3 participants
연관 작업