feat: 팀 상태바 컴포넌트 추가#48
Conversation
Summary of ChangesHello @Jieunsse, 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
|
There was a problem hiding this comment.
Code Review
이 PR은 팀 상태를 보여주는 TeamStatusBar 컴포넌트를 추가하고, 이 컴포넌트에서 사용하는 ProgressBar의 스타일을 일부 수정합니다. 전반적으로 새로운 기능 추가가 잘 이루어졌습니다. 다만, 코드의 가독성과 유지보수성을 높이기 위해 몇 가지 개선점을 제안합니다. 주요 제안 사항은 시맨틱 HTML 구조를 올바르게 사용하고, 컴포넌트에 전달되는 불필요한 props를 정리하는 것입니다. 자세한 내용은 각 파일의 주석을 참고해주세요.
| <dl className={styles.countContainer}> | ||
| <div> | ||
| <dt className={styles.subTitle}>오늘의 할 일</dt> | ||
| <dd className={styles.taskNumber}>{taskCount}</dd> | ||
| </div> | ||
| <span className={styles.line} role="separator" aria-hidden /> | ||
| <div> | ||
| <dt className={styles.subTitle}>완료 🙌</dt> | ||
| <dd className={styles.completedNumber}>{completed}</dd> | ||
| </div> | ||
| </dl> |
There was a problem hiding this comment.
시맨틱 HTML 구조 개선을 제안합니다. 현재 <dl> 요소의 자식으로 <div>와 <span>이 함께 사용되고 있는데, 이는 HTML 명세에 맞지 않습니다. <dl> 요소는 <dt>, <dd>, 또는 이들을 감싸는 <div> 그룹만을 자식으로 가질 수 있습니다. 유효한 HTML 구조를 위해 바깥쪽 <dl>을 <div>로 변경하고, 각 용어-설명 쌍을 별도의 <dl>로 감싸는 것을 제안합니다. 이렇게 하면 스타일링을 위한 레이아웃을 유지하면서도 시맨틱 마크업을 준수할 수 있습니다.
| <dl className={styles.countContainer}> | |
| <div> | |
| <dt className={styles.subTitle}>오늘의 할 일</dt> | |
| <dd className={styles.taskNumber}>{taskCount}</dd> | |
| </div> | |
| <span className={styles.line} role="separator" aria-hidden /> | |
| <div> | |
| <dt className={styles.subTitle}>완료 🙌</dt> | |
| <dd className={styles.completedNumber}>{completed}</dd> | |
| </div> | |
| </dl> | |
| <div className={styles.countContainer}> | |
| <dl> | |
| <dt className={styles.subTitle}>오늘의 할 일</dt> | |
| <dd className={styles.taskNumber}>{taskCount}</dd> | |
| </dl> | |
| <span className={styles.line} role="separator" aria-hidden /> | |
| <dl> | |
| <dt className={styles.subTitle}>완료 🙌</dt> | |
| <dd className={styles.completedNumber}>{completed}</dd> | |
| </dl> | |
| </div> |
| export interface TeamStatusBarProps { | ||
| title: string; | ||
| percentage: number; | ||
| taskCount: number; | ||
| completed: number; | ||
| } |
There was a problem hiding this comment.
컴포넌트 API 설계에 대한 제안입니다. percentage prop은 taskCount와 completed prop으로부터 파생될 수 있는 값으로 보입니다.
데이터의 단일 공급원(Single Source of Truth) 원칙을 따르기 위해, percentage prop을 제거하고 TeamStatusBar 컴포넌트 내부에서 (completed / taskCount) * 100와 같이 직접 계산하는 것을 고려해 보세요.
이렇게 하면 다음과 같은 이점이 있습니다:
- API가 더 간결해집니다.
- 부모 컴포넌트에서
percentage를 잘못 계산하여 발생할 수 있는 데이터 불일치 버그를 예방할 수 있습니다.
이 변경을 적용하면 TeamStatusBar.tsx에서 percentage를 계산하고, ProgressBar에는 value prop 대신 done={completed}와 total={taskCount}를 전달하여 진행률을 계산하도록 할 수 있습니다. 이는 다른 리뷰 댓글에서 제안한 중복 prop 제거와도 일맥상통합니다.
Summary