[4주차 기본/생각 과제] Github profile finder #5
Conversation
borimong
requested review from
joohaem,
NaveOWO,
seoyeun0106 and
talkingOrange
| export default function Header() { | ||
| const [array, setArray] = useState([]); | ||
| const [userId, setUserId] = useState(""); | ||
| const [showArray, setShowArray] = useState(false); |
There was a problem hiding this comment.
boolean 타입의 변수명이 어색하네요, 동사의 변수명은 함수에서 쓰이고, 의문문이나 명사 형태가 적절할 것 같아요
There was a problem hiding this comment.
오홍 글쿤여..!! isOpenModal 이런 식으로 바꿔보겠슴돠!!!!!
| useEffect(() => { | ||
| getGithubProfile(); | ||
| }, [userName]) | ||
|
|
||
| if (!getGithubProfile) return <div>Loading...</div>; |
There was a problem hiding this comment.
isLoading 이라는 state 값을 선언해서,
await 구문 전후로 setIsLoading true/false 로써 조작하면 어떨까 해요!
이 구문은 함수가 있는지 없는지를 판별해서 로딩을 나타내느 것이라 많이 어색하네요 (return 값으로 판별하는 것이 아니기 떄문에)
There was a problem hiding this comment.
헐헐 무슨 의미인지 알 것 같습돠!!! 한 번 수정해 볼게여!!!! XD
borimong
changed the title
| export default function Content() { | ||
| const { userName } = useParams(); | ||
| const navigate = useNavigate(); | ||
| const [githubProfile, setgithubProfile] = useState({login: '', name: '', avatar_url: '', html_url: '', followers: '', following: '', public_repos: ''}); |
There was a problem hiding this comment.
요런 애들은 initialState 같은걸로 분리해서 작성하면 가독성이 좀 나을듯!
There was a problem hiding this comment.
오 지금 보니까 그렇네!! 비슷한 것끼리 묶을 수 있는 경우 최대한 하나의 덩어리로 묶어야겠다!! 감쟘다 <3
| {array.map((array, index) => ( | ||
| <SearchModal key={index}> | ||
| <ModalText | ||
| onClick={() => { | ||
| navigate(`/search/${array}`); | ||
| setModalOpen(false); | ||
| }} | ||
| > |
There was a problem hiding this comment.
변수 네이밍을 좀더 직관적으로? 어떤 배열을 의미하는지 알 수 있게 지으면 좋을 것 같고
map 돌리는 변수 이름이랑 인자 이름이 똑같아서 혼란스러울 수 있으니 네이밍 신경쓰면 좋을듯!
There was a problem hiding this comment.
마자.. 맨날 변수명 대충 지어놓고 나중에 헷갈려했는데, 이번 기회에 확실히 바로잡아야겠다!! 피드백 반영할게!! <3
| <Padding></Padding> | ||
| <Blank></Blank> | ||
| <Outlet /> |
There was a problem hiding this comment.
내용물이 없는 경우 padding과 blank도 outlet처럼 클로징 태그로 써주자
There was a problem hiding this comment.
일관성있는 코드를 짜도록 노력해야겠다..! 고마워 서영언니!!!! 좜빠 것 코드리뷰까지 반영한 후 푸시할게욤 <3
✨ 구현 기능 명세
react-router-dom을 이용해 url 에 따른 컴포넌트를 나타낸다/search에서, 검색창 및 검색 history 를 포함한 내용을 나타낸다/search/:userId에서, userId 에 맞는 유저 정보를 받아온다 (GET 통신)검색하거나 다른 곳을 클릭할 때 박스를 없앱니다
/search/:userId로 이동한다🎁 PR Point
검색 기록 부분 모달로 구현했습니다! onClick 에 적용되어야 하는 이벤트가 두 개라서 함수 두 개를 저렇게 연달아 썼는데 가독성 있는 코드는 아닌 것 같아서요,,!! 혹시 더 똑똑한 방법이 있다면 알려주시면 감사하겠습니다!
Header.jsx 에서 모달 컴포넌트 렌더링하는 부분인데, 코드 길이가 가로로 너어무 길어서 가독성이 떨어지는 것 같아서 반성중입니다;;
Content.jsx 에서 로딩 부분 저렇게 구현하는 것이 맞을까요?? 제 컴이 너무 빨라서 그런 건지,, 로딩 화면이 뜨질 않아 여쭤봅니댜..!
😭 소요 시간, 어려웠던 점
10h😎 구현 결과물
2022-11-11.11.22.01.video-converter.com.mp4