Github Star Ranking #6

yurim22 · 2020-04-05T11:37:46Z

Gitstar Ranking

React 환경에서 구현하였습니다.

실행방법

npm install
npm start

아쉬운점

react-async 패키지 활용해서 api 요청 상태 관리하는 것 혹은 custom hook 만들어서 도전
각 repository별 star 수를 아이콘과 함께 오른쪽에 배치하고 싶은데 왜 안될까요...

lallaheeee

LGTM👍🏻🤩
코드가 깔끔해서 읽기 좋네유

lallaheeee · 2020-04-14T12:41:58Z

src/App.js

+  });
+
+  const getUserName = e => {
+    e.preventDefault();


e.preventDefault() 해주신 있을까요?! 궁금합니다 !

e.preventDefault() 경우, 사용하는 경우가 많아서 Vue에서는 :click.prevent와 같이 이벤트 수식어를 지원해주고 있는데요. 혹시 리액트에는 없을까요?

@joi0104

네 !! 거기다 리액트는 return false를 해도 e.preventDefalut가 되지 않아요!! ㅎㅎ
https://ko.reactjs.org/docs/handling-events.html

오오 신기하네요 ㅎㅎ

lallaheeee · 2020-04-14T12:43:43Z

src/App.css

+  to {
+    transform: rotate(360deg);
+  }
+}


이 파일은 없어도 될거같아요! ㅎㅎ

mango906 · 2020-04-14T12:47:45Z

src/components/RepoDetails/RepoDetail.js

+const StarImg = styled.div`
+  margin-left: 2rem;
+  margin-right: 1rem;
+`;


Suggested change

const StarImg = styled.div`

margin-left: 2rem;

margin-right: 1rem;

`;

const StarImg = styled.div`

margin-left: auto;

`;

각 repository별 star 수를 아이콘과 함께 오른쪽에 배치하는건 이렇게 하면 될듯 합니다!

(아이콘과 스타 수를 div로 한번 더 묶으면 더 좋을거 같아요)

호오,, 대박! 진짜 감사합니당👍👍

mango906 · 2020-04-14T12:54:43Z

src/components/RepoDetails/RepoDetail.js

+const Container = styled.div`
+  border: 1px solid;
+  display: flex;
+  flex-direction: row;
+  width: 70%;
+  padding: 1rem;
+  margin-bottom: -2px;
+`;


border들이 겹치면서 2px로 나오는 경우가 있는데 이렇게 수정하면 좋을듯 합니다!

Suggested change

const Container = styled.div`

border: 1px solid;

display: flex;

flex-direction: row;

width: 70%;

padding: 1rem;

margin-bottom: -2px;

`;

const Container = styled.div`

border: 1px solid;

display: flex;

flex-direction: row;

width: 70%;

padding: 1rem;

& + div {

margin-top: -1px;

}

`;

(사진으론 잘 안보이네요)

넹! 말씀하신부분 참고해서 수정했어요~~👍

NuclearPunch · 2020-04-15T04:48:07Z

src/App.js

+  };
+
+  const getUserInfo = async () => {
+    const response = await axios.get(`${gitURL}/${userName}/repos`);


이렇게 분해할당 가능할 것 같습니다!

Suggested change

const response = await axios.get(`${gitURL}/${userName}/repos`);

const { data } = await axios.get(`${gitURL}/${userName}/repos`);

YukJiSoo · 2020-04-15T15:44:24Z

src/App.js

+    <div className="App">
+      <Container>
+        <Header inputHandler={getUserName} clickHandler={getUserInfo} />
+        <UserName>{userInfo.name}</UserName>
+        <RepoInfo>
+          {userInfo.repo} Repositories | {userInfo.star} stars
+        </RepoInfo>
+        <hr />
+        <div className="list">
+          <RepoDetailList repos={repos} />
+        </div>
+      </Container>
+    </div>


semantic한 html tag를 사용하는 것도 좋을것 같아요!

전에 좋은 글을 읽어서 공유할게요~
https://medium.com/@soeunlee/%EC%8B%9C%EB%A7%A8%ED%8B%B1%ED%95%98%EA%B2%8C-html%EC%9D%84-%EC%A7%A0%EB%8B%A4%EB%8A%94-%EA%B2%83-90612ffc988e

YukJiSoo · 2020-04-15T15:51:35Z

src/App.js

+  const getUserInfo = async () => {
+    const response = await axios.get(`${gitURL}/${userName}/repos`);
+    const { data } = response;


API로직을 관리하는 별도의 파일로 분리하는 것은 어떤가요??
component 파일은 최대한 view 관련 로직만 다루는 것이 좋다고 생각해요! 유지보수할 때도 좋을것 같고요!

폴더 구조 설계하는것도 더 공부를 해봐야 될 것 같아요😢 너무 어렵...
좋은 의견 감사합니다! 참고해서 수정해볼게욥😉

YukJiSoo · 2020-04-15T15:53:42Z

src/components/RepoDetails/RepoDetail.js

+    <>
+      <Container>
+        <div>{name}</div>
+        <StarImg>
+          <img src={star} alt="star_image" width="20px" />
+        </StarImg>
+
+        <div>{stargazers_count}</div>
+      </Container>
+    </>


Container가 상위에서 하위 컴포넌트들을 감싸고 있기 때문에 Fragment를 사용하지 않아도 될 것 같아요!

YukJiSoo · 2020-04-15T15:58:52Z

src/components/RepoDetails/RepoDetailList.js

+  return (
+    <div>
+      {repos.map(repo => (
+        <RepoDetail
+          key={repo.id}
+          name={repo.name}
+          stargazers_count={repo.stargazers_count}
+        />
+      ))}
+    </div>
+  );
+};


Suggested change

return (

<div>

{repos.map(repo => (

<RepoDetail

key={repo.id}

name={repo.name}

stargazers_count={repo.stargazers_count}

/>

))}

</div>

);

};

return repos.map(repo => <RepoDetail key={repo.id} name={repo.name} stargazers_count={repo.stargazers_count}/>);

};

이렇게는 안될까요??

오옹!! 훨씬 더 보기 간편하네요ㅎㅎ return을 항상 저런식으로 하다보니까 똑같이 맞췄던것 같아요... 감사합니다👍

props가 2개 이상이 되거나 안에 내용이 길어져서 한 눈에 보이지 않는다면 줄 바꿈이 있는게 가독성이 더 높지 않을까요??

저도 Yuni-Q님 말씀처럼 여러개의 props가 있고 내용이 길어져, 줄바꿈 하는 것이 좀 더 가독성이 높지 않을까 생각했습니다.

소소한 tip이라면 아래와 같이 구조분해 해서 사용할 수 도있을 것 같아요

Suggested change

return (

<div>

{repos.map(repo => (

<RepoDetail

key={repo.id}

name={repo.name}

stargazers_count={repo.stargazers_count}

/>

))}

</div>

);

};

return (

<div>

{repos.map(({id, name, stargazers_count}) => (

<RepoDetail

key={id}

name={name}

stargazers_count={stargazers_count}

/>

))}

</div>

);

};

danivelop · 2020-04-16T18:05:21Z

src/App.js

+  const getUserName = e => {
+    e.preventDefault();
+    setUserName(e.target.value);
+  };


Suggested change

const getUserName = e => {

e.preventDefault();

setUserName(e.target.value);

};

const getUserName = useCallback(e => {

e.preventDefault();

setUserName(e.target.value);

}, []);

getUserName에 함수를 할당할 때 useCallback 이나 lodash의 memoize 을 사용해 메모이제이션을 해주는게 좋을거 같아요! 메모이제이션을 해주지 않으면 username을 입력할 때마다 App컴포넌트는 리렌더링 되는데 그때마다 getUserName에도 새로운 함수가 할당되고 이를 속성으로 넘겨준 모든컴포넌트가 불필요하게 리렌더링 될 수 있을거같습니다

처음에 useCallback 함수 활용해서 구현하려고 했는데 그땐 두번째 인자에 대한 이해가 부족해서 그냥 넘겼었네요ㅠㅜ 그래도 이렇게 짚어주신김에 더 찾아보니까 조금은 알게된 것 같아요😊 효율성을 위해 적절하게 사용하도록 하겠습니다!

danivelop · 2020-04-16T18:09:23Z

src/components/RepoDetails/RepoDetailList.js

+const RepoDetailList = props => {
+  const { repos } = props;


Suggested change

const RepoDetailList = props => {

const { repos } = props;

const RepoDetailList = ({ repos }) => {

로 줄일수 있을거 같습니다!

eastroots92

전체적으로 비즈니스 로직과, 뷰, 스타일 등이 구분이 되면 좋겠다는 생각이 들었습니다.

하나의 관점을 가지고 폴더 구조와 파일구조를 나누면 어떨까 생각됩니다.
직접 해보시는 것을 매우 추천드리나 만약 너무 막막하다면 구글에 리액트 폴더구조, 디렉토리 구조 등등을 검색해보시면 좋을 것 같아요.

eastroots92 · 2020-04-17T12:50:23Z

src/components/RepoDetails/RepoDetailList.js

+  return (
+    <div>
+      {repos.map(repo => (
+        <RepoDetail
+          key={repo.id}
+          name={repo.name}
+          stargazers_count={repo.stargazers_count}
+        />
+      ))}
+    </div>
+  );
+};


저도 Yuni-Q님 말씀처럼 여러개의 props가 있고 내용이 길어져, 줄바꿈 하는 것이 좀 더 가독성이 높지 않을까 생각했습니다.

eastroots92 · 2020-04-17T12:50:42Z

src/components/RepoDetails/RepoDetailList.js

+  return (
+    <div>
+      {repos.map(repo => (
+        <RepoDetail
+          key={repo.id}
+          name={repo.name}
+          stargazers_count={repo.stargazers_count}
+        />
+      ))}
+    </div>
+  );
+};


소소한 tip이라면 아래와 같이 구조분해 해서 사용할 수 도있을 것 같아요

Suggested change

return (

<div>

{repos.map(repo => (

<RepoDetail

key={repo.id}

name={repo.name}

stargazers_count={repo.stargazers_count}

/>

))}

</div>

);

};

return (

<div>

{repos.map(({id, name, stargazers_count}) => (

<RepoDetail

key={id}

name={name}

stargazers_count={stargazers_count}

/>

))}

</div>

);

};

kand193 · 2020-04-15T13:28:47Z

package.json

+  "dependencies": {
+    "@testing-library/jest-dom": "^4.2.4",
+    "@testing-library/react": "^9.3.2",
+    "@testing-library/user-event": "^7.1.2",


testing-library는 devDependencies로 분리하는게 좋아보여요!

gitstar commit

aa50f83

lallaheeee reviewed Apr 14, 2020

View reviewed changes

mango906 reviewed Apr 14, 2020

View reviewed changes

NuclearPunch reviewed Apr 15, 2020

View reviewed changes

YukJiSoo reviewed Apr 15, 2020

View reviewed changes

danivelop reviewed Apr 16, 2020

View reviewed changes

eastroots92 reviewed Apr 17, 2020

View reviewed changes

kand193 reviewed Apr 17, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Github Star Ranking #6

Github Star Ranking #6

yurim22 commented Apr 5, 2020

lallaheeee left a comment

lallaheeee Apr 14, 2020

joi0104 Apr 16, 2020 •

edited

Loading

lallaheeee Apr 16, 2020

joi0104 Apr 17, 2020

lallaheeee Apr 14, 2020

mango906 Apr 14, 2020

mango906 Apr 14, 2020 •

edited

Loading

yurim22 Apr 17, 2020

mango906 Apr 14, 2020

mango906 Apr 14, 2020

mango906 Apr 14, 2020

yurim22 Apr 17, 2020

NuclearPunch Apr 15, 2020

YukJiSoo Apr 15, 2020

YukJiSoo Apr 15, 2020

yurim22 Apr 16, 2020

YukJiSoo Apr 15, 2020

YukJiSoo Apr 15, 2020

yurim22 Apr 16, 2020

Yuni-Q Apr 16, 2020

eastroots92 Apr 17, 2020

eastroots92 Apr 17, 2020

danivelop Apr 16, 2020 •

edited

Loading

yurim22 Apr 17, 2020

danivelop Apr 16, 2020

eastroots92 left a comment

eastroots92 Apr 17, 2020

eastroots92 Apr 17, 2020

kand193 Apr 15, 2020

	const response = await axios.get(`${gitURL}/${userName}/repos`);
	const { data } = await axios.get(`${gitURL}/${userName}/repos`);

	const RepoDetailList = props => {
	const { repos } = props;
	const RepoDetailList = ({ repos }) => {

Github Star Ranking #6

Are you sure you want to change the base?

Github Star Ranking #6

Conversation

yurim22 commented Apr 5, 2020

Gitstar Ranking

실행방법

아쉬운점

lallaheeee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joi0104 Apr 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mango906 Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danivelop Apr 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eastroots92 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joi0104 Apr 16, 2020 •

edited

Loading

mango906 Apr 14, 2020 •

edited

Loading

danivelop Apr 16, 2020 •

edited

Loading