Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/components/TeamStatusBar/TeamStatusBar.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import styles from './style/TeamStatusBar.module.css';
import type { TeamStatusBarProps } from './interface/interface';
import Image from 'next/image';
import settingBigIcon from '@/assets/icons/setting/SettingBig.svg';
import ProgressBar from '../progressbar';

export default function TeamStatusBar({
title,
percentage,
taskCount,
completed,
}: TeamStatusBarProps) {
return (
<section className={styles.mainContainer} aria-label={title}>
<h2 className={styles.title}>{title}</h2>
<header className={styles.subContainer}>
<dl>
<dt className={styles.subTitle}>오늘의 진행 상황</dt>
<dd className={styles.percent}>{percentage}%</dd>
</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>
Comment on lines +21 to +31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

시맨틱 HTML 구조 개선을 제안합니다. 현재 <dl> 요소의 자식으로 <div><span>이 함께 사용되고 있는데, 이는 HTML 명세에 맞지 않습니다. <dl> 요소는 <dt>, <dd>, 또는 이들을 감싸는 <div> 그룹만을 자식으로 가질 수 있습니다. 유효한 HTML 구조를 위해 바깥쪽 <dl><div>로 변경하고, 각 용어-설명 쌍을 별도의 <dl>로 감싸는 것을 제안합니다. 이렇게 하면 스타일링을 위한 레이아웃을 유지하면서도 시맨틱 마크업을 준수할 수 있습니다.

Suggested change
<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>

</header>

<div className={styles.barContainer}>
<ProgressBar
value={percentage / 100}
done={completed}
total={taskCount}
ariaLabel="team progress"
className={styles.progressBar}
/>
<button className={styles.settingButton} aria-label="settings">
<Image src={settingBigIcon} alt="" width={24} height={24} />
</button>
</div>
</section>
);
}
6 changes: 6 additions & 0 deletions src/components/TeamStatusBar/interface/interface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export interface TeamStatusBarProps {
title: string;
percentage: number;
taskCount: number;
completed: number;
}
Comment on lines +1 to +6

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

컴포넌트 API 설계에 대한 제안입니다. percentage prop은 taskCountcompleted prop으로부터 파생될 수 있는 값으로 보입니다.

데이터의 단일 공급원(Single Source of Truth) 원칙을 따르기 위해, percentage prop을 제거하고 TeamStatusBar 컴포넌트 내부에서 (completed / taskCount) * 100와 같이 직접 계산하는 것을 고려해 보세요.

이렇게 하면 다음과 같은 이점이 있습니다:

  • API가 더 간결해집니다.
  • 부모 컴포넌트에서 percentage를 잘못 계산하여 발생할 수 있는 데이터 불일치 버그를 예방할 수 있습니다.

이 변경을 적용하면 TeamStatusBar.tsx에서 percentage를 계산하고, ProgressBar에는 value prop 대신 done={completed}total={taskCount}를 전달하여 진행률을 계산하도록 할 수 있습니다. 이는 다른 리뷰 댓글에서 제안한 중복 prop 제거와도 일맥상통합니다.

3 changes: 0 additions & 3 deletions src/components/progressbar/ProgressBar.module.css
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
.track {
width: 100%;
max-width: 322px;
height: 20px;

border-radius: 9999px;
Expand Down Expand Up @@ -29,14 +28,12 @@

@media (min-width: 744px) {
.track {
max-width: 566px;
height: 27px;
}
}

@media (min-width: 1024px) {
.track {
max-width: 1010px;
height: 27px;
}
}
Loading