[Design] 메인페이지 트레이더섹션 CSS 스타일 구현 #117
Conversation
리뷰어 가이드 by Sourcery이 PR은 메인 페이지의 트레이더 섹션 스타일링과 기능을 구현합니다. 변경 사항에는 반응형 디자인을 갖춘 새로운 트레이더 섹션 추가, 트레이더 통계를 위한 API 통합 구현, 기존 투자자 섹션 스타일 재구성이 포함됩니다. 변경 사항이 간단해 보이며 시각적 표현이 필요하지 않으므로 다이어그램이 생성되지 않았습니다. 파일 수준 변경 사항
관련 문제일 가능성 있음
팁 및 명령어Sourcery와 상호작용하기
경험 맞춤화하기대시보드에 접속하여:
도움 받기Original review guide in EnglishReviewer's Guide by SourceryThis PR implements the trader section styling and functionality on the main page. The changes include adding a new trader section with responsive design, implementing API integration for trader statistics, and reorganizing the existing investor section styles. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🚀 Deployed on https://67396ad8858afc3415f0c1a4--sysmetic.netlify.app |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
안녕하세요 @Panda-raccoon - 변경 사항을 검토했습니다. 다음은 피드백입니다:
전반적인 의견:
- API 가져오기 로직을 사용자 정의 훅(예:
useTraderStats)으로 이동하여 관심사를 분리하고 유지 보수성을 향상시키는 것을 고려해보세요. 또한 API 엔드포인트 이름을 일관되게 사용하세요 (trader-Strategyvstrader-strategy). - 투자자와 트레이더 섹션 간에 유사한 패턴이 있습니다. 코드 중복을 줄이고 유지 보수성을 향상시키기 위해 지금 공유 컴포넌트를 추출하는 것을 고려해보세요.
검토 중에 살펴본 내용입니다
- 🟡 일반 문제: 1개의 문제 발견
- 🟢 보안: 모두 양호
- 🟢 테스트: 모두 양호
- 🟡 복잡성: 1개의 문제 발견
- 🟢 문서화: 모두 양호
더 유용하게 도와주세요! 각 댓글에 👍 또는 👎를 클릭해 주시면 피드백을 사용하여 리뷰를 개선하겠습니다.
Original comment in English
Hey @Panda-raccoon - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider moving the API fetching logic into a custom hook (e.g.,
useTraderStats) to separate concerns and improve maintainability. Also ensure consistent API endpoint naming (trader-Strategyvstrader-strategy). - There are similar patterns between investor and trader sections. Consider extracting shared components now rather than later to reduce code duplication and improve maintainability.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import { ROUTES } from '@/constants/routes'; | ||
| import theme from '@/styles/theme'; | ||
|
|
||
| const HomePage = () => { |
There was a problem hiding this comment.
issue (complexity): 공유 스타일과 데이터 가져오기 로직을 재사용 가능한 컴포넌트와 훅으로 추출하는 것을 고려해보세요
코드는 두 가지 주요 영역에서 단순화될 수 있습니다:
- 스타일 중복을 줄이기 위해 공유 스타일 섹션 컴포넌트를 만드세요:
const SharedSection = styled.section`
position: relative;
max-width: ${theme.layout.width.content};
margin: 0 auto;
// 두 섹션에 공통된 스타일
.content {
display: flex;
gap: 20px;
height: 894px;
}
.text-content {
padding: 88px 0;
}
`;
// 사용 예:
<SharedSection className={isTrader ? 'trader-section' : 'investor-section'}>
<div className="content">
<div className="text-content">
{/* 내용 */}
</div>
</div>
</SharedSection>- 트레이더 통계 가져오기를 사용자 정의 훅으로 추출하세요:
const useTraderStats = () => {
const [stats, setStats] = useState({ traderCount: '0', strategyCount: '0' });
useEffect(() => {
const fetchStats = async () => {
try {
const { traderCount, strategyCount } = await fetch('/api/trader-Strategy')
.then(res => res.json());
setStats({ traderCount, strategyCount });
} catch (error) {
console.error('Failed to fetch trader stats:', error);
}
};
fetchStats();
}, []);
return stats;
};
// 사용 예:
const { traderCount, strategyCount } = useTraderStats();이러한 변경 사항은 기능을 유지하면서 코드 중복을 줄이고 유지 보수성을 향상시킬 것입니다.
Original comment in English
issue (complexity): Consider extracting shared styles and data fetching logic into reusable components and hooks
The code could be simplified in two key areas:
- Create a shared styled section component to reduce style duplication:
const SharedSection = styled.section`
position: relative;
max-width: ${theme.layout.width.content};
margin: 0 auto;
// Common styles for both sections
.content {
display: flex;
gap: 20px;
height: 894px;
}
.text-content {
padding: 88px 0;
}
`;
// Usage:
<SharedSection className={isTrader ? 'trader-section' : 'investor-section'}>
<div className="content">
<div className="text-content">
{/* Content */}
</div>
</div>
</SharedSection>- Extract trader stats fetching to a custom hook:
const useTraderStats = () => {
const [stats, setStats] = useState({ traderCount: '0', strategyCount: '0' });
useEffect(() => {
const fetchStats = async () => {
try {
const { traderCount, strategyCount } = await fetch('/api/trader-Strategy')
.then(res => res.json());
setStats({ traderCount, strategyCount });
} catch (error) {
console.error('Failed to fetch trader stats:', error);
}
};
fetchStats();
}, []);
return stats;
};
// Usage:
const { traderCount, strategyCount } = useTraderStats();These changes will reduce code duplication while maintaining functionality and improving maintainability.
There was a problem hiding this comment.
api 호출 하는 부분 맞게 잘 쓰신 것 같아요!! 저희 api 호출할 때 axios를 자주 사용해서 호출할텐데 이 부분 찾아보시고 적용해봐도 좋을 것 같아욥!! 코드가 쪼끔 더 간단해진답니다
There was a problem hiding this comment.
useEffect(() => {
const fetchTraderStats = async () => {
try {
const { data } = await axios.get('/api/trader-Strategy');
setTraderCount(data.traderCount);
setStrategyCount(data.strategyCount);
} catch (error) {
console.error('트레이더 통계 조회 실패:', error);
}
};
fetchTraderStats();
}, []);
요렇게용?
There was a problem hiding this comment.
일단 그렇게 바꿔두세요... 근데 아마 또 axios 호출하는 부분이 바뀔수 있어요. 그때가서 또 변경하라고 말씀드릴께요!
There was a problem hiding this comment.
넵 알겠습니다!
지금은 axios로 적용했습니다! 바꿨습니다!
seoyoonyi
left a comment
seoyoonyi
left a comment
There was a problem hiding this comment.
rankingButtonStyle에 border: none; 있어요. 그거 0으로 바꿔주세요
There was a problem hiding this comment.
일단 그렇게 바꿔두세요... 근데 아마 또 axios 호출하는 부분이 바뀔수 있어요. 그때가서 또 변경하라고 말씀드릴께요!
src/pages/HomePage.tsx
Outdated
| 투자자 가입하기 | ||
| </Button> | ||
| <Button variant='secondary' size='xl' width={208} onClick={handleStrategyListClick}> | ||
| <Button variant='secondary' size='xl' width={208} onClick={handleTraderListClick}> |
There was a problem hiding this comment.
트레이더 리스트 아니고 전략쪽 페이지로 이동해야 됩니다
There was a problem hiding this comment.
const handleTraderListClick = () => {
navigate(ROUTES.STRATEGY.LIST);
};
전략랭킹 버튼 누르면 전략랭킹 리스트로 가게끔 바꿨습니다!
|
🚀 Deployed on https://6739ead1d624efe0b83c6e9f--sysmetic.netlify.app |
|
🚀 Deployed on https://6739eb95808ab5be30c2451b--sysmetic.netlify.app |
|
🚀 Deployed on https://6739ef2ce563dcc2fdd6e082--sysmetic.netlify.app |
🚀 풀 리퀘스트 제안
📋 작업 내용
메인페이지(홈페이지) 트레이더 섹션 부분 CSS 스타일링 했습니다.
투자자섹션과 같은 부분은 나중에 컴포넌트 분리예정입니다.
API 적용 코드 작성했습니다.
🔧 변경 사항
주요 변경 사항을 요약해 주세요.
📸 스크린샷 (선택 사항)
📄 기타
API적용 하는 코드 맞는지 확인 부탁드립니다
Sourcery에 의한 요약
홈페이지의 트레이더 섹션에 대한 CSS 스타일링을 구현하고 트레이더 및 전략 통계를 가져오는 API를 통합합니다.
새로운 기능:
개선 사항:
Original summary in English
Summary by Sourcery
Implement CSS styling for the trader section on the homepage and integrate API to fetch trader and strategy statistics.
New Features:
Enhancements: