Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: 동적으로 테이블이 변경되는 기능을 구현한다 #25

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

drunkenhw
Copy link
Contributor

@drunkenhw drunkenhw commented Aug 14, 2023

📄 Summary

먼저 일단 리뷰를 받고 싶어서 제출합니다. 아직 다른 api는 안만들어져있는 것 같아서 간단히 야미가 만들어놓은 기능과 합쳤습니다

🕰️ Actual Time of Completion

30시간

🙋🏻 More

close #14

@drunkenhw drunkenhw added the feat 기능 추가 label Aug 14, 2023
@drunkenhw drunkenhw requested a review from feb-dain August 14, 2023 10:04
@drunkenhw drunkenhw self-assigned this Aug 14, 2023
Copy link
Contributor

@feb-dain feb-dain left a comment

Choose a reason for hiding this comment

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

타입 오류 개선해주세요~

  • 뭔가 복잡하게 돌아간 느낌이네요🤔
    단순한 로직으로 개선할 수 있을 것 같아 보여요

))}
<Button sx={{ color: '#fff', wordBreak: 'keep-all' }} onClick={() => handleClickMenu()}>
{isLoggedIn ? '로그아웃' : '간편로그인'}
</Button>{' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

{' '} 지워주세요

Comment on lines +19 to +22
const AdminTableBody = <T, K extends keyof T>({ data, columns }: TableRowsProps<T, K>) => {
const { openModal } = modalActions;

const handleOpenModal = () => {
function handleOpenModal() {
Copy link
Contributor

@feb-dain feb-dain Aug 14, 2023

Choose a reason for hiding this comment

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

컴포넌트는 function, 이벤트 핸들러 등의 함수는 화살표 함수 써주세요

const [tableData, setTableData] = useState(stationMockData);
const token = memberTokenStore.getState();
const { lastPage, stationSummaryList } = useFetchStations(token, 1);
if (token === '') return <LoginModalContent />;
Copy link
Contributor

Choose a reason for hiding this comment

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

개행 주세여

function App() {
const [tableTitle, setTableTitle] = useState('충전소 관리');
const [tableColumn, setTableColumn] = useState(stationColumns);
const [tableData, setTableData] = useState(stationMockData);
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 밑에도 개행 줘도 될 것 같아여

Comment on lines +21 to +48
const memberColumns: ColumnType<MemberDetails, keyof MemberDetails>[] = [
{
key: 'id',
header: 'ID',
},
{
key: 'email',
header: 'E-MAIL',
},
{
key: 'role',
header: '회원 권한',
},
];
const faultReportColumns: ColumnType<FaultReportDetails, keyof FaultReportDetails>[] = [
{
key: 'id',
header: 'ID',
},
{
key: 'stationId',
header: '충전소 ID',
},
{
key: 'memberId',
header: '회원 ID',
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 만든 이유가 있을까요??

Suggested change
const memberColumns: ColumnType<MemberDetails, keyof MemberDetails>[] = [
{
key: 'id',
header: 'ID',
},
{
key: 'email',
header: 'E-MAIL',
},
{
key: 'role',
header: '회원 권한',
},
];
const faultReportColumns: ColumnType<FaultReportDetails, keyof FaultReportDetails>[] = [
{
key: 'id',
header: 'ID',
},
{
key: 'stationId',
header: '충전소 ID',
},
{
key: 'memberId',
header: '회원 ID',
},
];
const faultReportColumns = {
id: 'ID',
stationId: '충전소 ID'
....
}
const faultRepoutColumnList = getTypedObjectEntries(faultReportColumns)

이렇게 만들어서 쓰면 안되나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저는 혹시 key, header 외의 다른 속성들을 넣을 수 있지 않을까라는 생각에 했습니다. 하지만 굳이 지금 단계에선 과하고 코드의 복잡도만 높아지겠네요 수정하겠습니다

Comment on lines 24 to 25
'충전기 신고 관리',
'간편 로그인',
] as const;
Copy link
Contributor

@feb-dain feb-dain Aug 14, 2023

Choose a reason for hiding this comment

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

간편 로그인 왜 사라졌죠? 🤔 처음에 로그인 해야만 보이는 걸로 바꿔서 없앴나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아니요 간편 로그인은 클릭했을 때 간편로그인 창이 떠야하지만, 나머지 메뉴 버튼들은 table이 나와야해서 성격이 달라 map으로 적용하기가 어려운 부분이 있어서 성격이 비슷한 친구들만 묶었습니다..

}

export const useFetchStations = (token: string, page: number) => {
const [lastPage, setLastPage] = useState(1);
const [stationSummaryList, setStationSummaryList] = useState<StationProps[]>([]);
const [stationSummaryList, setStationSummaryList] = useState<StationDetails[]>([]);
Copy link
Contributor

Choose a reason for hiding this comment

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

타입 이름 stationSummary가 아닐까요?

Comment on lines +56 to +66
export const misinformnationMockData = Array.from(
{ length: ROWS_PER_PAGE },
() => misinformationMock
);

export function getMockData<T>(title: string): Array<T> {
switch (title) {
case '충전소 관리':
return stationMockData as Array<T>;
case '충전소 제보 관리':
return misinformnationMockData as Array<T>;
Copy link
Contributor

Choose a reason for hiding this comment

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

misinformation으로 바꿔주세요~ 오타났습니다

role: string;
}

const memberColumns: ColumnType<MemberDetails, keyof MemberDetails>[] = [
Copy link
Contributor

Choose a reason for hiding this comment

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

이게 왜 type에 있죠...? constant 아닌가요? 이하 동문입니다~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네..

key: 'role',
header: '회원 권한',
},
];
Copy link
Contributor

Choose a reason for hiding this comment

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

개행주세여

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

네비게이션의 메뉴를 선택하면 컴포넌트가 변경되는 기능을 구현한다
2 participants