github_star_9 김주성 과제 제출합니다. #10

jus0k · 2020-04-16T09:28:21Z

6개월 전에 작업한 것이라 부족함이 많습니닷...
다른 레포지토리에서 작업해서 커밋이 하나네요 ㅜㅜ

mango906

수고하셨습니다 👍👍

mango906 · 2020-04-16T10:58:30Z

src/components/ErrorMessage.jsx

+const S = {
+  Container: styled.div`
+    width: 100%;
+    padding: 1rem 2rem;
+    background-color: #e99002;
+    color: #fff;
+  `,
+};


오 이렇게 객체로 선언해서 쓰는건 처음보네요 🧐

Styled Component를 사용한 컴포넌트랑 일반 컴포넌트를 분리하려고 전에는 Styled 접두사를 붙였었는데요! 이렇게 하면 간단하게 구분할 수 있어서 좋은 것 같아요 ㅎㅎ 단점은 객체다 보니까 없는 속성 써도 린트에서 잡아내지 못한다는 점?이 있습니다!

mango906 · 2020-04-16T10:59:04Z

src/components/ErrorMessage.jsx

+ErrorMessage.propTypes = {
+  errorName: PropTypes.string.isRequired,
+};


propTypes 정의 해주는거 멋지십니다 👍

mango906 · 2020-04-16T11:00:44Z

src/components/Loading.jsx

+    <S.Loader
+      version="1.1"
+      id="loader-1"
+      xmlns="http://www.w3.org/2000/svg"
+      xmlnsXlink="http://www.w3.org/1999/xlink"
+      x="0px"
+      y="0px"
+      width="40px"
+      height="40px"
+      viewBox="0 0 50 50"
+      xmlSpace="preserve"
+    >
+      <path
+        fill="#000"
+        d="M43.935,25.145c0-10.318-8.364-18.683-18.683-18.683c-10.318,0-18.683,8.365-18.683,18.683h4.068c0-8.071,6.543-14.615,14.615-14.615c8.072,0,14.615,6.543,14.615,14.615H43.935z"
+        style={{ fill: '#D8D8D8' }}
+      >
+        <animateTransform
+          attributeType="xml"
+          attributeName="transform"
+          type="rotate"
+          from="0 25 25"
+          to="360 25 25"
+          dur="0.6s"
+          repeatCount="indefinite"
+        />
+      </path>
+    </S.Loader>


svg 커스텀하시는거 짱 멋있어요!

요거는 로딩 인디케이터 가져다가 썼습니다! 제가 한 것은 아니에요 ㅎㅎ

mango906 · 2020-04-16T11:11:35Z

src/containers/Search.jsx

+    try {
+      const response = await axios(
+        `https://api.github.com/search/repositories?q=+user:${username}`,
+      );
+      setUser(response.data.items);
+      setFetchState('render');
+    } catch (error) {
+      setFetchState('error');
+    }
+  }, []);
+
+  const handleSubmit = event => {
+    event.preventDefault();
+    setFetchState('loading');
+    setUser([]);
+    fetchData(value);
+    setErrorName(value);


무조건 errorName을 설정하기보단, 이렇게 수정하면 어떨까요??

Suggested change

try {

const response = await axios(

`https://api.github.com/search/repositories?q=+user:${username}`,

);

setUser(response.data.items);

setFetchState('render');

} catch (error) {

setFetchState('error');

}

}, []);

const handleSubmit = event => {

event.preventDefault();

setFetchState('loading');

setUser([]);

fetchData(value);

setErrorName(value);

try {

const response = await axios(

`https://api.github.com/search/repositories?q=+user:${username}`,

);

setUser(response.data.items);

setFetchState('render');

} catch (error) {

setFetchState('error');

setErrorName(value);

}

}, []);

const handleSubmit = event => {

event.preventDefault();

setFetchState('loading');

setUser([]);

fetchData(value);

오호 API 요청 보내는 쪽에서 에러메세지 설정하는게 맞는 것 같네요!

mango906 · 2020-04-16T11:14:16Z

src/containers/Search.jsx

+        {
+          loading: fetchState === 'loading' && <Loading />,
+          error: fetchState === 'error' && (
+            <ErrorMessage errorName={errorName} />
+          ),
+          render: fetchState === 'render' && <Result user={user} />,
+        }[fetchState]


처음보는 문법인데 {}[] 이건 무슨 문법인가요????? 🧐

추론해보면 왠지 객체를 프로퍼티로 읽는거같아요! ㅎㅎ

흠..아니네요···ㅎ

객체를 선언하고 바로 fetchState에 따라 프로퍼티를 읽는거 아닐까요 희라님 말씀대루??

맞는거 같아요 감사합니다!

넵 지수님 말씀이 맞습니다! 다만 객체의 프로퍼티를 읽는 것이다 보니 매칭이 안되는 애들의 인스턴스도 생성이 되어서 조건문으로 불필요한 인스턴스 생성을 막았습니닷~
3가지 이상의 케이스를 분기해야 하는 상황에서 구문 대신 표현식을 사용하면 더 깔끔해지는 것 같습니다!

요 부분 인상적이긴 한데,
낯설어서 그런지 코드를 읽는 데 있어서 부자연스러운 느낌이 적지않게 있네요.

lallaheeee

고생하셨습니다 ㅎㅎ

lallaheeee · 2020-04-16T14:17:41Z

src/components/Header.jsx

+const S = {
+  Container: styled.div`
+    width: 100%;
+    padding: 3rem;
+    background-color: #fafafa;
+    box-sizing: border-box;
+    margin-bottom: 3rem;
+  `,
+  Title: styled.h1`
+    font-size: 4rem;
+    margin-bottom: 1rem;
+  `,
+  SubTitle: styled.h2`
+    font-size: 1.5rem;
+    font-weight: lighter;
+    margin-bottom: 1rem;
+  `,
+};
+
+const Header = ({ value, setValue, onSubmit }) => {
+  return (
+    <S.Container>
+      <S.Title>Gitstar Ranking</S.Title>
+      <S.SubTitle>
+        Unofficial GitHub star ranking for users, organizations and
+        repositories.
+      </S.SubTitle>
+      <InputField value={value} setValue={setValue} onSubmit={onSubmit} />
+    </S.Container>


오 이렇게 styled-component를 이렇게 namespace로 사용하는거 좋은 거 같아요!

네! 요래 하면 깔끔하게 적을 수 있는데 단점은 객체의 프로퍼티로 읽다보니 없는 프로퍼티를 린트에서 잡아내지 못한다는 점입니다 ㅜ

@jus0k

확실하진 않은데, 요거 TS에서는 잡을 수 있는 것으로 알아요!
Welcome to TS world 🎉

lallaheeee · 2020-04-16T14:19:48Z

src/components/Loading.jsx

+    width: 100%;
+  `,
+  Loader: styled.svg`
+    enable-background: new 0 0 50 50;


우와 처음보는 속성인데 신기하네요!

svg는 가져다가 쓴거라 저도 잘 모르겠습니닷 ㅎㅎ후

lallaheeee · 2020-04-16T14:22:14Z

src/containers/Search.jsx

+        {
+          loading: fetchState === 'loading' && <Loading />,
+          error: fetchState === 'error' && (
+            <ErrorMessage errorName={errorName} />
+          ),
+          render: fetchState === 'render' && <Result user={user} />,
+        }[fetchState]


추론해보면 왠지 객체를 프로퍼티로 읽는거같아요! ㅎㅎ

lallaheeee · 2020-04-16T14:26:11Z

src/containers/Search.jsx

+        {
+          loading: fetchState === 'loading' && <Loading />,
+          error: fetchState === 'error' && (
+            <ErrorMessage errorName={errorName} />
+          ),
+          render: fetchState === 'render' && <Result user={user} />,
+        }[fetchState]


Suggested change

{

loading: fetchState === 'loading' && <Loading />,

error: fetchState === 'error' && (

<ErrorMessage errorName={errorName} />

),

render: fetchState === 'render' && <Result user={user} />,

}[fetchState]

{

loading: <Loading />,

error: <ErrorMessage errorName={errorName} />,

render: <Result user={user} />,

}[fetchState]

이렇게 해도 동작할까요?!

넵 동작은 하는데요~
if 문이나 switch 문은 조건에 매칭되지 않은 케이스에 대한 코드는 실행 자체가 안되지만,
객체는 render에 매칭되어도 loading, error에 대한 불필요한 인스턴스가 생성되거든요! 그래서 매칭되지 않은 값은 인스턴스 생성하지 않게하려구 컨디션을 걸어준 것입니다!

joi0104 · 2020-04-16T14:53:15Z

src/image/star.svg

+<path d="M22.4,9.5c-0.1-0.4-0.4-0.6-0.8-0.7l-6-0.9l-2.7-5.4c-0.3-0.7-1.5-0.7-1.8,0L8.4,8l-6,0.9C2.1,8.9,1.8,9.2,1.6,9.5
+	c-0.1,0.4,0,0.8,0.3,1l4.3,4.2l-1,6c-0.1,0.4,0.1,0.8,0.4,1c0.2,0.1,0.4,0.2,0.6,0.2c0.2,0,0.3,0,0.5-0.1L12,19l5.4,2.8
+	c0.3,0.2,0.7,0.1,1.1-0.1c0.3-0.2,0.5-0.6,0.4-1l-1-6l4.3-4.2C22.4,10.3,22.5,9.9,22.4,9.5z"/>


이 부분은 무슨 동작을 하는 코드인지 알 수 있을까요?? 궁금하네요 ㅎㅎ

svg 코드인데요! 저도 가져다가 쓴거라 있는대로 썼습니다 ㅎㅎ 백터 기반 이미지를 그리는 코드? 인 것 같습니다!

YukJiSoo · 2020-04-16T14:52:13Z

src/styles/GlobalStyle.js

+import reset from 'styled-reset';
+
+const GlobalStyle = createGlobalStyle`
+  ${reset}


reset css 너무 좋네여!

YukJiSoo · 2020-04-16T14:55:23Z

src/containers/Search.jsx

+        {
+          loading: fetchState === 'loading' && <Loading />,
+          error: fetchState === 'error' && (
+            <ErrorMessage errorName={errorName} />
+          ),
+          render: fetchState === 'render' && <Result user={user} />,
+        }[fetchState]


객체를 선언하고 바로 fetchState에 따라 프로퍼티를 읽는거 아닐까요 희라님 말씀대루??

YukJiSoo · 2020-04-16T15:00:15Z

src/components/Header.jsx

+  `,
+};
+
+const Header = ({ value, setValue, onSubmit }) => {


Header component의 props 이름으로 value, setValue, onSubmit보다 의미를 더 명확히 하는 네이밍은 어떤가요??

value -> username, setValue -> changeUsername, onSubmit -> searchUsername
이런 식으로 값의 주체를 명확히 해주는게 의미파악이 분명해질 것 같아요!

오 넵 좋네요! 그럼 set보다 change라고 쓰는 이유가 있으신가요?! 지수님의 네이밍 규칙이 궁금하네요 ㅎㅎ

eastroots92

크으~~ 👍

eastroots92 · 2020-04-17T14:09:41Z

package.json

+      "last 1 safari version"
+    ]
+  },
+  "devDependencies": {


package sorting 까지 디테일하게 챙기는편은 아니지만 devDependencies와 Dependencies는 붙여서 보여주는 것이 보기 더 좋을 것 같아요!

eastroots92 · 2020-04-17T14:11:29Z

src/components/ErrorMessage.jsx

+const ErrorMessage = ({ errorName }) => {
+  const errorMessage = `User ${errorName} is not found.`;
+  return <S.Container>{errorMessage}</S.Container>;
+};


요건 뭔가 요런식으로 써도 되지 않을까 하는 🙈
(개취에욬ㅋㅋ)

Suggested change

const ErrorMessage = ({ errorName }) => {

const errorMessage = `User ${errorName} is not found.`;

return <S.Container>{errorMessage}</S.Container>;

};

const ErrorMessage = ({ errorName }) => (<S.Container>User {errorName} is not found.</S.Container>);

아 이거 아마 프리티어랑 린트가 제대로 동작을 안해서 이렇게 했던 것 같네요 ㅎ후ㅜ

eastroots92 · 2020-04-17T14:13:09Z

src/components/Header.jsx

+const S = {
+  Container: styled.div`
+    width: 100%;
+    padding: 3rem;
+    background-color: #fafafa;
+    box-sizing: border-box;
+    margin-bottom: 3rem;
+  `,
+  Title: styled.h1`
+    font-size: 4rem;
+    margin-bottom: 1rem;
+  `,
+  SubTitle: styled.h2`
+    font-size: 1.5rem;
+    font-weight: lighter;
+    margin-bottom: 1rem;
+  `,
+};
+
+const Header = ({ value, setValue, onSubmit }) => {
+  return (
+    <S.Container>
+      <S.Title>Gitstar Ranking</S.Title>
+      <S.SubTitle>
+        Unofficial GitHub star ranking for users, organizations and
+        repositories.
+      </S.SubTitle>
+      <InputField value={value} setValue={setValue} onSubmit={onSubmit} />
+    </S.Container>


@jus0k

확실하진 않은데, 요거 TS에서는 잡을 수 있는 것으로 알아요!
Welcome to TS world 🎉

eastroots92 · 2020-04-17T14:16:32Z

src/containers/Search.jsx

+        {
+          loading: fetchState === 'loading' && <Loading />,
+          error: fetchState === 'error' && (
+            <ErrorMessage errorName={errorName} />
+          ),
+          render: fetchState === 'render' && <Result user={user} />,
+        }[fetchState]


요 부분 인상적이긴 한데,
낯설어서 그런지 코드를 읽는 데 있어서 부자연스러운 느낌이 적지않게 있네요.

과제 제출

ce90b66

mango906 reviewed Apr 16, 2020

View reviewed changes

lallaheeee reviewed Apr 16, 2020

View reviewed changes

joi0104 reviewed Apr 16, 2020

View reviewed changes

YukJiSoo reviewed Apr 16, 2020

View reviewed changes

eastroots92 reviewed Apr 17, 2020

View reviewed changes

github_star_9 김주성 과제 제출합니다. #10

Are you sure you want to change the base?

github_star_9 김주성 과제 제출합니다. #10

Conversation

jus0k commented Apr 16, 2020

mango906 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jus0k Apr 17, 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

jus0k Apr 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lallaheeee 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

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

jus0k Apr 17, 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jus0k Apr 17, 2020 •

edited

Loading

jus0k Apr 17, 2020 •

edited

Loading

jus0k Apr 17, 2020 •

edited

Loading