[Design] 전략 상세페이지 퍼블리싱 #118
Conversation
jizerozz
requested review from
Panda-raccoon,
clara-shin,
seoyoonyi and
ssuminii
리뷰어 가이드 by Sourcery이 PR은 전략 세부 페이지의 게시를 구현하며, 테이블 입력 모달, 파일 다운로드 기능, 분석 테이블 등 여러 주요 기능을 포함합니다. 구현은 데이터 관리를 위한 재사용 가능한 테이블 구성 요소와 모달 기능을 생성하는 데 중점을 두며, 사용자 경험을 향상시키기 위한 스타일링 개선도 포함합니다. 새로운 테이블 구성 요소에 대한 클래스 다이어그램classDiagram
class AnalysisTable {
+AnalysisAttribuesProps[] attributes
+AnalysisDataProps[] data
+String mode
+handleAllChecked()
+handleSelected(int idx)
+handleInputChange(InputTableProps[] updatedData)
+handleUpdateData(InputTableProps data, int idx)
+handleUpdateModal(InputTableProps data, int idx)
}
class InputTable {
+InputTableProps[] data
+handleInputChange(int idx, String field, String value)
}
class TableModal {
+String title
+JSX.Element data
+String actionButton
+onAction()
}
class useTableModalStore {
+boolean isOpen
+TableModalData tableModalData
+openTableModal(TableModalData data)
+closeTableModal()
}
AnalysisTable --> InputTable
AnalysisTable --> TableModal
TableModal --> useTableModalStore
파일 수준 변경 사항
관련된 문제일 수 있음
팁 및 명령어Sourcery와 상호작용하기
경험 맞춤화대시보드에 액세스하여:
도움 받기Original review guide in EnglishReviewer's Guide by SourceryThis PR implements the publishing of a strategy detail page with several key features including table input modals, file download functionality, and analysis tables. The implementation focuses on creating reusable table components and modal functionality for data management, along with styling improvements for better user experience. Class diagram for new table componentsclassDiagram
class AnalysisTable {
+AnalysisAttribuesProps[] attributes
+AnalysisDataProps[] data
+String mode
+handleAllChecked()
+handleSelected(int idx)
+handleInputChange(InputTableProps[] updatedData)
+handleUpdateData(InputTableProps data, int idx)
+handleUpdateModal(InputTableProps data, int idx)
}
class InputTable {
+InputTableProps[] data
+handleInputChange(int idx, String field, String value)
}
class TableModal {
+String title
+JSX.Element data
+String actionButton
+onAction()
}
class useTableModalStore {
+boolean isOpen
+TableModalData tableModalData
+openTableModal(TableModalData data)
+closeTableModal()
}
AnalysisTable --> InputTable
AnalysisTable --> TableModal
TableModal --> useTableModalStore
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
안녕하세요 @jizerozz - 변경 사항을 검토했습니다 - 다음은 피드백입니다:
전반적인 의견:
- 하드코딩된 더미 데이터(strategyDummy, statisticsData 등)를 적절한 데이터 계층이나 API 통합으로 이동하여 유지 보수성을 향상시키는 것을 고려해보세요
- 파일 다운로드 구현에는 적절한 오류 처리와 로딩 상태를 포함하여 사용자 경험을 개선해야 합니다
검토 중에 살펴본 내용입니다
- 🟡 일반 문제: 2개의 문제 발견
- 🟢 보안: 모두 양호
- 🟢 테스트: 모두 양호
- 🟡 복잡성: 1개의 문제 발견
- 🟢 문서화: 모두 양호
더 유용하게 도와주세요! 각 댓글에 👍 또는 👎를 클릭해 주시면 피드백을 사용하여 리뷰를 개선하겠습니다.
Original comment in English
Hey @jizerozz - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider moving hardcoded dummy data (strategyDummy, statisticsData, etc.) to a proper data layer or API integration to improve maintainability
- File download implementation should include proper error handling and loading states for better user experience
Here's what I looked at during the review
- 🟡 General issues: 2 issues 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.
| setSelected(new Array(data.length).fill(newSelectAll)); | ||
| }; | ||
|
|
||
| const handleSelected = (idx: number) => { |
There was a problem hiding this comment.
제안: 체크박스 선택을 위한 함수형 업데이트 패턴을 사용하여 상태 업데이트가 올바르게 처리되도록 고려해보세요
새 배열을 직접 생성하는 대신, setState 콜백 형식을 사용하세요: setSelected(prev => prev.map((val, i) => i === idx ? !val : val))
const handleSelected = (idx: number) => {
setSelected(prev => prev.map((val, i) => i === idx ? !val : val));Original comment in English
suggestion: Consider using a functional update pattern for checkbox selections to ensure state updates are handled correctly
Instead of creating a new array directly, use setState callback form: setSelected(prev => prev.map((val, i) => i === idx ? !val : val))
const handleSelected = (idx: number) => {
setSelected(prev => prev.map((val, i) => i === idx ? !val : val));| const FileDownSection = ({ fileUrl }: FileInfoProps) => { | ||
| const fileName = fileUrl.split('/').pop() || '전략파일'; | ||
|
|
||
| const handleDownload = () => { |
There was a problem hiding this comment.
제안: 파일 다운로드에 대한 오류 처리를 추가하여 잘못된 URL이나 다운로드 실패를 처리하세요
다운로드 로직을 try-catch 블록으로 감싸고 잠재적인 오류를 우아하게 처리하는 것을 고려해보세요
const handleDownload = async () => {
try {
const response = await fetch(fileUrl);
if (!response.ok) throw new Error('Download failed');
const link = document.createElement('a');
link.href = URL.createObjectURL(await response.blob());Original comment in English
suggestion: Add error handling for file downloads to handle invalid URLs or failed downloads
Consider wrapping the download logic in a try-catch block and handling potential errors gracefully
const handleDownload = async () => {
try {
const response = await fetch(fileUrl);
if (!response.ok) throw new Error('Download failed');
const link = document.createElement('a');
link.href = URL.createObjectURL(await response.blob());| import StatisticsTable from '@/components/page/strategy-detail/tabmenu/StatisticsTable'; | ||
| import theme from '@/styles/theme'; | ||
|
|
||
| const strategyDummy = [ |
There was a problem hiding this comment.
문제 (복잡성): 모의 데이터를 전용 파일로 추출하는 것을 고려해보세요
모의 데이터는 별도의 파일로 이동하여 유지 보수성과 관심사의 분리를 개선해야 합니다. 새 파일 mockData.ts를 만드세요:
export const strategyDummy = [...];
export const statisticsData = [...];
export const dailyAnalysisData = [...];
export const monthlyAnalysisData = [...];
export const dailyAttributes = [...];
export const monthlyAttributes = [...];그런 다음 데이터를 가져와서 구성 요소 파일을 단순화하세요:
import {
strategyDummy,
statisticsData,
dailyAnalysisData,
monthlyAnalysisData,
dailyAttributes,
monthlyAttributes
} from './mockData';
const tabMenu = [
{
title: '통계',
component: <StatisticsTable data={statisticsData} />,
},
// ... other tab configurations
];이 분리는 다음을 가능하게 합니다:
- 구성 요소 파일이 UI 책임에 더 집중할 수 있습니다
- 모의 데이터를 더 쉽게 유지 관리하고 업데이트할 수 있습니다
- 코드 가독성과 조직 개선
Original comment in English
issue (complexity): Consider extracting mock data into a dedicated file
The mock data should be moved to a separate file to improve maintainability and separation of concerns. Create a new file mockData.ts:
export const strategyDummy = [...];
export const statisticsData = [...];
export const dailyAnalysisData = [...];
export const monthlyAnalysisData = [...];
export const dailyAttributes = [...];
export const monthlyAttributes = [...];Then simplify the component file by importing the data:
import {
strategyDummy,
statisticsData,
dailyAnalysisData,
monthlyAnalysisData,
dailyAttributes,
monthlyAttributes
} from './mockData';
const tabMenu = [
{
title: '통계',
component: <StatisticsTable data={statisticsData} />,
},
// ... other tab configurations
];This separation will:
- Make the component file more focused on its UI responsibilities
- Make mock data easier to maintain and update
- Improve code readability and organization
| const TableModal = () => { | ||
| const { isOpen, tableModalData, closeTableModal } = useTableModalStore(); | ||
|
|
||
| if (!isOpen || !tableModalData) return null; |
There was a problem hiding this comment.
제안 (코드 품질): if, while 등에서 블록 중괄호를 사용하세요 (use-braces)
| if (!isOpen || !tableModalData) return null; | |
| if (!isOpen || !tableModalData) { |
설명
항상 중괄호를 사용하고 명시적인 문장 블록을 만드는 것이 좋습니다.허용된 구문을 사용하여 단일 문장만 작성하면 매우 혼란스러운 상황이 발생할 수 있습니다.
특히 나중에 개발자가 다른 문장을 추가하면서 중괄호를 추가하는 것을 잊어버리는 경우
(이 경우 조건에 포함되지 않음).
Original comment in English
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces)
| if (!isOpen || !tableModalData) return null; | |
| if (!isOpen || !tableModalData) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
| const updatedData = [...prevData, ...tableData]; | ||
| return updatedData; |
There was a problem hiding this comment.
제안 (코드 품질): 즉시 반환되는 변수를 인라인하세요 (inline-immediately-returned-variable)
| const updatedData = [...prevData, ...tableData]; | |
| return updatedData; | |
| return [...prevData, ...tableData]; | |
설명
사람들의 코드에서 자주 볼 수 있는 것은 결과 변수에 할당한 후 즉시 반환하는 것입니다.결과를 직접 반환하면 코드가 짧아지고 불필요한 변수가 제거되어
함수를 읽는 데 드는 정신적 부담이 줄어듭니다.
중간 변수가 유용할 수 있는 경우는 그것이 매개변수나 조건으로 사용될 때이며,
변수 이름이 변수가 나타내는 것을 주석처럼 작용할 수 있습니다. 함수를 반환하는 경우에는
함수 이름이 결과가 무엇인지 알려주므로 변수 이름이 불필요합니다.
Original comment in English
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| const updatedData = [...prevData, ...tableData]; | |
| return updatedData; | |
| return [...prevData, ...tableData]; | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
|
🚀 Deployed on https://67397297fba80e5e181bb115--sysmetic.netlify.app |
|
🚀 Deployed on https://6739833601947a651e2d2075--sysmetic.netlify.app |
|
🚀 Deployed on https://67398409d624ef6b913c6edf--sysmetic.netlify.app |
ssuminii
left a comment
ssuminii
left a comment
There was a problem hiding this comment.
와 미쳤다.......너무너무너무 고생하셨습니다.......대박.....
seoyoonyi
left a comment
seoyoonyi
left a comment
There was a problem hiding this comment.
css 하느라 고생했어요! 기능구현도 화이팅...!
🚀 풀 리퀘스트 제안
📋 작업 내용
전략 상세페이지 디자인 나온대로 퍼블리싱
테이블 입력 모달 생성, 테이블 모달 스토어 생성
첨부파일 다운로드 기능 구현 (제안서)
입력 테이블 컴포넌트 생성
분석 테이블 컴포넌트 생성
일간분석, 월간분석 탭 영역 구현
추가, 수정 버튼에 모달 연결
입력 테이블을 만드는 이슈인데요 . . 전략 상세 페이지 디자인 변경 및 추가 사항들이 생겨서 해당 이슈 내에서 한 번에 처리했습니다
🔧 변경 사항
주요 변경 사항을 요약해 주세요.
📸 스크린샷 (선택 사항)
📄 기타
추가적으로 전달하고 싶은 내용이나 특별한 요구 사항이 있으면 작성해 주세요.
Sourcery에 의한 요약
새로운 디자인에 따라 전략 세부 페이지를 게시하고, 일일 및 월간 데이터에 대한 입력 및 분석 테이블을 구현합니다. 전략 제안서에 대한 파일 다운로드 기능을 추가하고, 새로운 작업 버튼으로 전략 제목 섹션을 재설계합니다. 계정 인증 섹션을 업데이트된 지침과 스타일로 개선합니다.
새로운 기능:
개선 사항:
Original summary in English
Summary by Sourcery
Publish the strategy detail page according to the new design, including the implementation of input and analysis tables for daily and monthly data. Add a file download feature for strategy proposals and redesign the strategy title section with new action buttons. Enhance the account verification section with updated instructions and styles.
New Features:
Enhancements: