Skip to content

Feat/35 : 코드레빗 리뷰용#43

Open
Han-wo wants to merge 7 commits intodevelopfrom
feat/35
Open

Feat/35 : 코드레빗 리뷰용#43
Han-wo wants to merge 7 commits intodevelopfrom
feat/35

Conversation

@Han-wo
Copy link
Collaborator

@Han-wo Han-wo commented Dec 5, 2024

#️⃣ 이슈

  • close #이슈번호

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.

  • Todo
  • Todo
  • Todo

📸 스크린샷

✅ 체크 리스트

  • 적절한 Title 작성
  • 적절한 Label 지정
  • Assignee 및 Reviewer 지정
  • 로컬 작동 확인
  • Merge 되는 브랜치 확인

👩‍💻 공유 포인트 및 논의 사항

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 사용자 계좌에 대한 주식 요약 정보를 표시하는 StockSummary 컴포넌트 추가.
    • 주식 정보를 표 형식으로 표시하는 StockTable 컴포넌트 추가.
    • 사용자 주식 포트폴리오를 표시하는 StockPortfolioPage 컴포넌트 추가.
    • "내 계좌" 페이지를 위한 레이아웃 컴포넌트 추가.
  • 버그 수정

    • 내비게이션 메뉴의 "내 계좌" 링크 수정.
  • 문서화

    • 새로운 타입스크립트 인터페이스 StockTotalStocks 추가.
  • 스타일

    • 반응형 디자인의 화면 크기 조정.

@Han-wo Han-wo added the ✨ FEAT This doesn't seem right label Dec 5, 2024
@Han-wo Han-wo self-assigned this Dec 5, 2024
@coderabbitai
Copy link

coderabbitai bot commented Dec 5, 2024

Walkthrough

이 변경 사항은 사용자 계좌 정보를 표시하기 위한 여러 React 컴포넌트를 추가합니다. StockSummaryStockTable 컴포넌트가 새로운 파일에 도입되어 각각 주식 요약 및 주식 목록을 표시합니다. 또한, layout.tsx 파일이 추가되어 "내 계좌" 페이지의 레이아웃을 정의하고, page.tsx에서 사용자 주식 포트폴리오를 표시하는 컴포넌트가 추가되었습니다. 타입스크립트 인터페이스도 새로 정의되어 데이터 구조를 명확히 하고, 내비게이션 메뉴의 링크가 업데이트되었습니다.

Changes

파일 경로 변경 요약
src/app/my-account/_components/stock-summary.tsx 새로운 컴포넌트 StockSummary 추가, totalStocks prop 수용, 주식 요약 표시
src/app/my-account/_components/stock-table.tsx 새로운 컴포넌트 StockTable 추가, 주식 정보를 테이블 형식으로 표시
src/app/my-account/layout.tsx 새로운 레이아웃 컴포넌트 추가, 페이지 메타데이터 정의
src/app/my-account/page.tsx 새로운 컴포넌트 StockPortfolioPage 추가, API로부터 주식 데이터 가져오기
src/app/my-account/types/index.ts 새로운 인터페이스 StockTotalStocks 추가
src/app/page.tsx Home 컴포넌트의 레이아웃 및 스타일 수정
src/components/nav-bar/_components/nav-menu.tsx 내비게이션 링크 수정, "/mypage"에서 "/my-account"로 변경
tailwind.config.ts 반응형 디자인 브레이크포인트 수정, lgxl 크기 변경

Poem

🐰 변화의 바람이 불어와
주식 요약, 테이블로 쫙!
내 계좌가 반짝반짝,
새로운 레이아웃에 웃음꽃!
모두 함께 즐겨요,
주식의 세계로 뛰어들어요! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@netlify
Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for growfoilo ready!

Name Link
🔨 Latest commit db4356f
🔍 Latest deploy log https://app.netlify.com/sites/growfoilo/deploys/6751aa634f9e3a0008e78357
😎 Deploy Preview https://deploy-preview-43--growfoilo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

github-actions bot commented Dec 5, 2024

💄 Storybook: undefined

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (6)
src/app/my-account/layout.tsx (2)

3-6: 메타데이터 설명 개선이 필요합니다.

현재 설명이 너무 간단합니다. SEO를 위해 더 자세한 설명을 추가하는 것이 좋겠습니다.

 export const metadata: Metadata = {
   title: "내 계좌 페이지 | 온라인 투자플랫폼",
-  description: "내 계좌 페이지입니다",
+  description: "온라인 투자플랫폼에서 내 주식 포트폴리오, 계좌 현황 및 투자 실적을 확인하실 수 있습니다.",
 };

14-14: 스타일링 개선 제안

하드코딩된 색상값과 임의의 max-width 값이 있습니다. 다음 사항들을 고려해보세요:

  1. 테일윈드 설정에서 색상 변수를 정의하여 재사용
  2. max-width 값을 더 표준적인 값으로 조정
-    <div className="lg:max-w-2000 flex h-screen flex-col bg-[#F5F6F8]">
+    <div className="lg:max-w-7xl flex h-screen flex-col bg-gray-50">
src/app/my-account/types/index.ts (1)

11-19: 불변성 보장을 위한 readonly 수정자 추가 제안

재무 데이터의 무결성을 보장하기 위해 readonly 수정자를 추가하는 것이 좋습니다.

 export interface TotalStocks {
-  memberNickname: string;
-  deposit: number; // 예수금
-  totalEvaluationProfit: number; // 총 평가손익
+  readonly memberNickname: string;
+  readonly deposit: number; // 예수금
+  readonly totalEvaluationProfit: number; // 총 평가손익
   // ... 나머지 속성들에도 readonly 적용
 }
src/app/my-account/page.tsx (1)

51-59: 접근성 및 반응형 디자인 개선이 필요합니다.

현재 마크업에서 다음 사항들을 개선하면 좋겠습니다:

 return (
-  <div className="p-30">
+  <main className="p-4 md:p-6 lg:p-8">
-    <h1 className="mb-30 ml-20 text-24-700 ">내 계좌</h1>
+    <h1 className="mb-6 ml-4 text-2xl font-bold">내 계좌</h1>
     <div className="mx-auto max-w-1200 rounded-10 bg-white p-20">
       <StockSummary totalStocks={totalStocks} />
       <StockTable stocks={stocks} />
     </div>
-  </div>
+  </main>
 );
src/app/my-account/_components/stock-summary.tsx (1)

49-59: 조건부 스타일링 로직 개선이 필요합니다

현재 구현은 중복된 널 체크와 복잡한 조건부 로직을 포함하고 있습니다.

다음과 같이 개선하는 것을 제안합니다:

-className={clsx(
-  "border border-gray-200 p-20 text-center text-20-700",
-  {
-    "text-red-500": (totalStocks?.totalEvaluationProfit ?? 0) > 0,
-    "text-blue-500": (totalStocks?.totalEvaluationProfit ?? 0) < 0,
-  },
-)}
+const profit = totalStocks?.totalEvaluationProfit ?? 0;
+className={clsx(
+  "border border-gray-200 p-20 text-center text-20-700",
+  {
+    "text-red-500": profit > 0,
+    "text-blue-500": profit < 0,
+  },
+)}
src/app/my-account/_components/stock-table.tsx (1)

73-130: 반복되는 조건부 스타일링 로직을 추상화해야 합니다

현재 구현은 여러 곳에서 동일한 조건부 스타일링 로직을 반복하고 있습니다.

다음과 같은 유틸리티 함수 추출을 제안합니다:

const getProfitColorClass = (value: number) => 
  clsx({
    "text-red-500": value > 0,
    "text-blue-500": value < 0,
  });

그리고 다음과 같이 사용:

-className={clsx(
-  "border-r border-gray-200 p-8 text-right text-16-600",
-  {
-    "text-red-500": stock.ProfitRate > 0,
-    "text-blue-500": stock.ProfitRate < 0,
-  },
-)}
+className={clsx(
+  "border-r border-gray-200 p-8 text-right text-16-600",
+  getProfitColorClass(stock.ProfitRate)
+)}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 3023523 and db4356f.

📒 Files selected for processing (8)
  • src/app/my-account/_components/stock-summary.tsx (1 hunks)
  • src/app/my-account/_components/stock-table.tsx (1 hunks)
  • src/app/my-account/layout.tsx (1 hunks)
  • src/app/my-account/page.tsx (1 hunks)
  • src/app/my-account/types/index.ts (1 hunks)
  • src/app/page.tsx (1 hunks)
  • src/components/nav-bar/_components/nav-menu.tsx (1 hunks)
  • tailwind.config.ts (1 hunks)
🔇 Additional comments (2)
src/app/page.tsx (1)

44-44: 패딩 값의 적절성 검토 필요

xl:px-230 패딩 값이 매우 큰 것으로 보입니다. 이는 다음과 같은 문제를 일으킬 수 있습니다:

  • 작은 화면에서 콘텐츠 영역이 지나치게 좁아질 수 있음
  • 반응형 디자인의 일관성이 저하될 수 있음

더 작은 패딩 값을 사용하거나, 미디어 쿼리 중단점을 추가하여 점진적으로 패딩을 조정하는 것을 고려해보세요.

Also applies to: 46-46

src/components/nav-bar/_components/nav-menu.tsx (1)

33-33: 라우트 변경에 따른 영향도 확인 필요

"/mypage"에서 "/my-account"로의 경로 변경이 확인되었습니다. 다음 사항들을 검토해주세요:

  • 기존 "/mypage" 경로를 사용하는 다른 컴포넌트나 링크가 있는지 확인
  • 필요한 경우 리다이렉트 설정 검토
  • SEO 영향도 고려
✅ Verification successful

경로 변경이 안전하게 이루어졌음을 확인했습니다

코드베이스 분석 결과:

  • /mypage 경로는 아이콘 import에만 사용되고 있어 라우팅에는 영향이 없습니다
  • /my-account 페이지가 src/app/my-account/page.tsx에 이미 구현되어 있습니다
  • 기존 라우트를 참조하는 다른 컴포넌트가 없어 리다이렉트 설정이 필요하지 않습니다
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 기존 mypage 경로 사용 여부 확인
rg -l "\/mypage"

Length of output: 93


Script:

#!/bin/bash
# Search for "mypage" path usage across the codebase
rg "/mypage"

# Search for "my-account" path usage to verify the new route
rg "/my-account"

# Check for any route configurations
fd "route|router" --type f

Length of output: 371


Script:

#!/bin/bash
# Check the route configuration in the members API route
cat src/api/members/route.ts

# Check for any Next.js page files that might be related to these routes
fd "page.tsx" --type f

Length of output: 1355

Comment on lines +1 to +9
export interface Stock {
stockName: string; // 종목명
currentPrice: number; // 현재가
stockCount: number; // 보유수량
prevChangeRate: number; // 전일 대비 등락률
EvaluationProfit: number; // 평가손익
ProfitRate: number; // 수익률
purchaseAmount: number; // 매입단가
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Stock 인터페이스 네이밍 컨벤션 개선이 필요합니다.

속성 이름에 일관성이 없습니다. 다음과 같이 개선하는 것을 추천드립니다:

 export interface Stock {
   stockName: string; // 종목명
   currentPrice: number; // 현재가
   stockCount: number; // 보유수량
   prevChangeRate: number; // 전일 대비 등락률
-  EvaluationProfit: number; // 평가손익
-  ProfitRate: number; // 수익률
+  evaluationProfit: number; // 평가손익
+  profitRate: number; // 수익률
   purchaseAmount: number; // 매입단가
 }
📝 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
export interface Stock {
stockName: string; // 종목명
currentPrice: number; // 현재가
stockCount: number; // 보유수량
prevChangeRate: number; // 전일 대비 등락률
EvaluationProfit: number; // 평가손익
ProfitRate: number; // 수익률
purchaseAmount: number; // 매입단가
}
export interface Stock {
stockName: string; // 종목명
currentPrice: number; // 현재가
stockCount: number; // 보유수량
prevChangeRate: number; // 전일 대비 등락률
evaluationProfit: number; // 평가손익
profitRate: number; // 수익률
purchaseAmount: number; // 매입단가
}

Comment on lines +47 to +50
export default async function StockPortfolioPage() {
const stocks = await getStocks();
const totalStocks = await getTotalStocks();

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

병렬 처리 및 로딩 상태 처리가 필요합니다.

현재 API 호출이 순차적으로 실행되고 있으며, 로딩 상태 처리가 없습니다.

 export default async function StockPortfolioPage() {
-  const stocks = await getStocks();
-  const totalStocks = await getTotalStocks();
+  const [stocks, totalStocks] = await Promise.all([
+    getStocks(),
+    getTotalStocks(),
+  ]).catch((error) => {
+    console.error("데이터 로딩 실패:", error);
+    return [[], {}];
+  });
📝 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
export default async function StockPortfolioPage() {
const stocks = await getStocks();
const totalStocks = await getTotalStocks();
export default async function StockPortfolioPage() {
const [stocks, totalStocks] = await Promise.all([
getStocks(),
getTotalStocks(),
]).catch((error) => {
console.error("데이터 로딩 실패:", error);
return [[], {}];
});

Comment on lines +6 to +22
async function getStocks() {
try {
const token = await getCookie("token");
const response = await fetch(
`${process.env.NEXT_PUBLIC_API_URL}/account/stocks`,
{
headers: {
Authorization: `Bearer ${token}`,
},
},
);
return await response.json();
} catch (error) {
console.error("주식 데이터 조회 실패:", error); //eslint-disable-line
return [];
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

API 호출 부분의 개선이 필요합니다.

현재 구현에서 다음과 같은 보완이 필요합니다:

  1. 응답 타입이 명시되어 있지 않습니다.
  2. HTTP 상태 코드 처리가 누락되었습니다.
  3. 토큰 만료 처리가 필요합니다.
 async function getStocks() {
   try {
     const token = await getCookie("token");
+    if (!token) {
+      throw new Error("인증 토큰이 없습니다");
+    }
     const response = await fetch(
       `${process.env.NEXT_PUBLIC_API_URL}/account/stocks`,
       {
         headers: {
           Authorization: `Bearer ${token}`,
         },
       },
     );
+    if (!response.ok) {
+      throw new Error(`API 오류: ${response.status}`);
+    }
-    return await response.json();
+    const data = await response.json() as Stock[];
+    return data;
   } catch (error) {
-    console.error("주식 데이터 조회 실패:", error);
+    console.error("주식 데이터 조회 실패:", error);
+    // TODO: 에러 추적을 위한 로깅 서비스 연동 필요
     return [];
   }
 }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +18 to +19
lg: { min: "1440px" },
xl: { min: "1900px" },
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

브레이크포인트 값 재검토 필요

현재 설정된 브레이크포인트가 일반적인 화면 크기보다 매우 큽니다:

  • lg: 1440px (일반적으로 1024px나 1200px 사용)
  • xl: 1900px (일반적으로 1280px나 1440px 사용)

이로 인해 발생할 수 있는 문제점:

  • 1440px 미만의 일반적인 데스크톱 화면에서 모바일/태블릿 레이아웃이 표시될 수 있음
  • 1900px의 매우 큰 브레이크포인트는 대부분의 사용자에게 적용되지 않을 수 있음

다음과 같은 일반적인 브레이크포인트 사용을 권장드립니다:

-      lg: { min: "1440px" },
-      xl: { min: "1900px" },
+      lg: { min: "1024px" },
+      xl: { min: "1280px" },
📝 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
lg: { min: "1440px" },
xl: { min: "1900px" },
lg: { min: "1024px" },
xl: { min: "1280px" },

Comment on lines +12 to +15
const getValueWithSign = (value: number | null | undefined) => {
if (!value) return "0";
return `${value > 0 ? "+" : ""}${value.toLocaleString()}`;
};
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

getValueWithSign 함수의 타입 처리 개선이 필요합니다

현재 구현은 null과 undefined를 동일하게 처리하고 있지만, 이는 명시적으로 구분되어야 합니다.

다음과 같이 개선하는 것을 제안합니다:

-const getValueWithSign = (value: number | null | undefined) => {
-  if (!value) return "0";
-  return `${value > 0 ? "+" : ""}${value.toLocaleString()}`;
+const getValueWithSign = (value: number | null | undefined) => {
+  if (value === null || value === undefined) return "0";
+  return `${value > 0 ? "+" : ""}${value.toLocaleString()}`;
};
📝 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 getValueWithSign = (value: number | null | undefined) => {
if (!value) return "0";
return `${value > 0 ? "+" : ""}${value.toLocaleString()}`;
};
const getValueWithSign = (value: number | null | undefined) => {
if (value === null || value === undefined) return "0";
return `${value > 0 ? "+" : ""}${value.toLocaleString()}`;
};

Comment on lines +18 to +36
<div className="w-full">
<table className="mb-30 w-full border-collapse">
<tbody>
<tr>
<td className="w-1/4 border border-gray-200 bg-green-50 p-10 text-center text-16-500">
닉네임
</td>
<td className="w-1/4 border border-gray-200 p-10 text-center text-16-500">
{totalStocks?.memberNickname}
</td>
<td className="w-1/4 border border-gray-200 bg-green-50 p-10 text-center text-16-500">
예수금
</td>
<td className="w-1/4 border border-gray-200 p-10 text-center text-16-500">
{totalStocks?.deposit?.toLocaleString() ?? 0}
</td>
</tr>
</tbody>
</table>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

테이블의 접근성 개선이 필요합니다

현재 테이블 구조에 의미론적 요소가 부족합니다.

다음 개선사항을 제안합니다:

  1. 테이블에 캡션 추가
  2. aria-label 속성 추가
  3. 테이블 헤더(th) 사용
-<table className="mb-30 w-full border-collapse">
+<table className="mb-30 w-full border-collapse" aria-label="사용자 계좌 정보">
+  <caption className="sr-only">사용자 기본 정보</caption>
   <tbody>
     <tr>
-      <td className="w-1/4 border border-gray-200 bg-green-50 p-10 text-center text-16-500">
+      <th scope="row" className="w-1/4 border border-gray-200 bg-green-50 p-10 text-center text-16-500">
         닉네임
-      </td>
+      </th>

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +23 to +27
// 수익률의 부호에 따라 평가손익의 부호를 보정
const getEvaluationProfitWithSign = (profit: number, rate: number) => {
const absProfit = Math.abs(profit);
return rate < 0 ? -absProfit : absProfit;
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

getEvaluationProfitWithSign 함수의 예외 처리가 필요합니다

현재 구현은 입력값의 유효성을 검사하지 않습니다.

다음과 같이 개선하는 것을 제안합니다:

-const getEvaluationProfitWithSign = (profit: number, rate: number) => {
+const getEvaluationProfitWithSign = (profit: number | undefined | null, rate: number | undefined | null) => {
+  if (profit === undefined || profit === null || rate === undefined || rate === null) {
+    return 0;
+  }
   const absProfit = Math.abs(profit);
   return rate < 0 ? -absProfit : absProfit;
 };
📝 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 getEvaluationProfitWithSign = (profit: number, rate: number) => {
const absProfit = Math.abs(profit);
return rate < 0 ? -absProfit : absProfit;
};
// 수익률의 부호에 따라 평가손익의 부호를 보정
const getEvaluationProfitWithSign = (profit: number | undefined | null, rate: number | undefined | null) => {
if (profit === undefined || profit === null || rate === undefined || rate === null) {
return 0;
}
const absProfit = Math.abs(profit);
return rate < 0 ? -absProfit : absProfit;
};

Comment on lines +31 to +71
<table className="w-full border-collapse">
<thead>
<tr>
<th
rowSpan={2}
className="w-[16%] border-r border-gray-200 bg-green-50 p-8 text-left text-16-600"
>
종목명
</th>
<th
colSpan={2}
className="w-[42%] border-b border-r border-gray-200 bg-green-50 p-8 text-center text-16-600"
>
평가손익
</th>
<th
rowSpan={2}
className="w-[16%] border-r border-gray-200 bg-green-50 p-8 text-center text-16-600"
>
보유수량
</th>
<th
colSpan={2}
className="w-[26%] border-b border-gray-200 bg-green-50 p-8 text-center text-16-600"
>
평가금액
</th>
</tr>
<tr className="bg-green-50">
<th className="border-r border-gray-200 p-8 text-center text-16-600">
금액
</th>
<th className="border-r border-gray-200 p-8 text-center text-16-600">
수익률
</th>
<th className="border-r border-gray-200 p-8 text-center text-16-600">
매입가
</th>
<th className="p-8 text-center text-16-600">현재가</th>
</tr>
</thead>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

테이블 헤더의 접근성 개선이 필요합니다

복잡한 테이블 구조에서 스크린 리더 사용자를 위한 접근성이 부족합니다.

다음 개선사항을 제안합니다:

  1. 테이블에 설명적인 캡션 추가
  2. 복잡한 헤더 구조에 대한 aria-describedby 추가
  3. scope 속성의 적절한 사용
-<table className="w-full border-collapse">
+<table 
+  className="w-full border-collapse"
+  aria-label="주식 포트폴리오 현황"
+>
+  <caption className="sr-only">보유 주식 상세 정보</caption>

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +15 to +21
const stockRows = useMemo(() => {
if (!stocks || !Array.isArray(stocks)) return [];
return stocks.map((stock) => ({
...stock,
id: `${tableId}-${stock.stockName}`,
}));
}, [stocks, tableId]);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

stockRows 메모이제이션 최적화가 필요합니다

현재 구현은 불필요하게 tableId를 의존성 배열에 포함하고 있습니다.

다음과 같이 개선하는 것을 제안합니다:

-const stockRows = useMemo(() => {
-  if (!stocks || !Array.isArray(stocks)) return [];
-  return stocks.map((stock) => ({
-    ...stock,
-    id: `${tableId}-${stock.stockName}`,
-  }));
-}, [stocks, tableId]);
+const stockRows = useMemo(() => {
+  if (!stocks || !Array.isArray(stocks)) return [];
+  return stocks.map((stock) => ({
+    ...stock,
+    id: stock.stockName, // stockName은 고유해야 합니다
+  }));
+}, [stocks]);
📝 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 stockRows = useMemo(() => {
if (!stocks || !Array.isArray(stocks)) return [];
return stocks.map((stock) => ({
...stock,
id: `${tableId}-${stock.stockName}`,
}));
}, [stocks, tableId]);
const stockRows = useMemo(() => {
if (!stocks || !Array.isArray(stocks)) return [];
return stocks.map((stock) => ({
...stock,
id: stock.stockName, // stockName은 고유해야 합니다
}));
}, [stocks]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ FEAT This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant