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: 충전소 상세 정보 조회기능을 개선하고, 누적 고장 신고 확인 기능을 구현한다 #245

Merged
merged 29 commits into from
Jul 30, 2023

Conversation

gabrielyoon7
Copy link
Member

@gabrielyoon7 gabrielyoon7 commented Jul 30, 2023

📄 Summary

누적 신고 횟수가 뜨는 기능 구현했습니다.
구현하면서 디자인 전면 개편했고, 일부 타입도 손봤습니다.

🕰️ Actual Time of Completion

5시간

🙋🏻 More

close #238

@gabrielyoon7 gabrielyoon7 added 🌱 기능추가 새로운 기능 요청입니다 FE 프론트엔드 관련 이슈입니다 🦜 UI UI 디자인을 수정합니다 labels Jul 30, 2023
@gabrielyoon7 gabrielyoon7 self-assigned this Jul 30, 2023
@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

Copy link
Member

@kyw0716 kyw0716 left a comment

Choose a reason for hiding this comment

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

뷰티풀샷

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

1 similar comment
@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@gabrielyoon7 gabrielyoon7 changed the title feat: 충전소 상세 정보 조회기능을 개선하고, 누적 고장 신고 확인 기능을 구현한다. feat: 충전소 상세 정보 조회기능을 개선하고, 누적 고장 신고 확인 기능을 구현한다 Jul 30, 2023
Copy link
Collaborator

@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.

리뷰 확인해주세요~

@@ -91,7 +91,7 @@ const Container = styled.div`
position: fixed;
top: 10px;
left: 180px;
z-index: 999;
z-index: 998;
Copy link
Collaborator

@feb-dain feb-dain Jul 30, 2023

Choose a reason for hiding this comment

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

우리 z index도 좀 얘기해봐야겠다ㅋㅋㅋ 그냥 뒀는데 쓰는 곳이 많아지네

Copy link
Member Author

Choose a reason for hiding this comment

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

기준을 어디로 잡아야 할지 모르겠음

Copy link
Member Author

Choose a reason for hiding this comment

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

이건 날 잡고 한번 정하자

import DetailedStation from '@ui/DetailedStationInfo/DetailedStation';

const meta = {
title: 'UI/DetailedStation',
Copy link
Collaborator

Choose a reason for hiding this comment

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

리팩터링할 때 이름 바꾸기

Copy link
Member Author

Choose a reason for hiding this comment

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

} = station;

return (
<Box px={2} pt={10} css={containerCss}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기는 어차피 수정 예정이라고 했으니 넘어갑니다~~

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 7 to 9
/**
* 이 컴포넌트는 컨테이너의 역할을 하게 될 것입니다.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

꼭 필요한 주석일까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

센트가 이어서 작업할 때 이어 붙이라고 힌트 남김

Copy link
Collaborator

Choose a reason for hiding this comment

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

아하

Comment on lines +23 to +28
css={{
position: 'fixed',
left: '41rem',
width: '34rem',
zIndex: '999',
}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

나중에 분리할 것

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines 16 to 18
/**
* TODO: 혼잡도도 외부(지금 이 컴포넌트)에서 자식에게 데이터를 주입 해서 렌더링 하는 방식으로 구현이 필요할듯 함. / 1. 데이터 페칭 관련 관심사 분리 가능, 2. 스토리북 테스트 쉬워짐
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

나중에 지울 주석이라면 jsdocs로 안 써도 되지 않을까요?? 우리 주석 컨벤션을 그냥 jsdocs로 바꿀까요? ㅋㅋㅋㅋ

Copy link
Member Author

Choose a reason for hiding this comment

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

우리 주석 컨벤션이 있었나?

Copy link
Collaborator

Choose a reason for hiding this comment

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

넹.. wiki 확인~

*/

return (
// 여기에서 쓰이는 Box 컴포넌트는 아코디언이 오기 전 까지 임시로 존재합니다. 자식과의 관심사 분리를 위해 분리 조치함.
Copy link
Collaborator

Choose a reason for hiding this comment

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

요긴 jsdocs 형태가 아닌 주석인데 위의 주석들과 차이는 뭘까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

jsdocs는 전체에 대한 설명으로 달아둔거고 이건 컴포넌트에 대한 설명으로 달아뒀음

Copy link
Member Author

Choose a reason for hiding this comment

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

아 중간에 jsdocs가 또 나오는구나
그냥 주석 제거하겠음

Comment on lines 47 to 50
<Text>{CHARGER_STATES[state as keyof typeof CHARGER_STATES]}</Text>
</FlexBox>
<article>
<Text>{CHARGER_TYPES[type as keyof typeof CHARGER_TYPES]}</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<Text>{CHARGER_STATES[state as keyof typeof CHARGER_STATES]}</Text>
</FlexBox>
<article>
<Text>{CHARGER_TYPES[type as keyof typeof CHARGER_TYPES]}</Text>
<Text>{CHARGER_STATES[state]}</Text>
</FlexBox>
<article>
<Text>{CHARGER_TYPES[type]}</Text>

Copy link
Member Author

Choose a reason for hiding this comment

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

띠용 이게 되네

Copy link
Member Author

Choose a reason for hiding this comment

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

안되는데?

Copy link
Collaborator

Choose a reason for hiding this comment

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

예? 왜 안 되죠? 저것만 빼면 당연히 오류나는데

bf9fb1b 내가 타입 수정 다 해놔서 오류 안 남!

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

1 similar comment
@github-actions
Copy link

🚀storybook: https://storybook.carffe.in/

Copy link
Collaborator

@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.

주석을 다 없애버렸네ㅋㅋㅋ

일단 굿굿 approve

@gabrielyoon7 gabrielyoon7 merged commit 22d49f8 into develop Jul 30, 2023
2 checks passed
@gabrielyoon7 gabrielyoon7 deleted the feat/238 branch July 30, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 관련 이슈입니다 🌱 기능추가 새로운 기능 요청입니다 🦜 UI UI 디자인을 수정합니다
Projects
None yet
Development

Successfully merging this pull request may close these issues.

충전소 상세 정보 조회기능을 개선하고, 누적 고장 신고 확인 기능을 구현한다.
3 participants