github_start_9 민경빈 과제 제출합니다. #4

mango906 · 2020-04-05T09:46:44Z

No description provided.

lallaheeee

mobx 코드는 처음 봤는데 짱 신기하네요 경빈님 덕에 mobx 코드도 읽어여 ㅎㅎ
고생하셨습니다!! 👍🏻

lallaheeee · 2020-04-15T11:16:03Z

src/hooks/useInput.ts

+export default function useInput(initialForm: State) {
+  const [state, dispatch] = useReducer(reducer, initialForm);
+  const onChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+    if (e.target.validity.valid) {


input에는 validity라는 프로퍼티도 있군요..! 어떨 때 사용하나요?!

input의 프로퍼티에 pattern이라는 값이 있는데 여기에다가 정규식(?) 양식을 넣을 수 있어요. 이때 이 양식에 맞지 않으면 validty.valid 값이 false로 리턴 됩니당

하지만 지금은 필요없다고 생각해서 삭제했습니다 ㅋㅋㅋㅋ

와 짱이네요 새로운 거 알아갑니다~ 👍🏻

lallaheeee · 2020-04-15T11:27:58Z

src/containers/Main.tsx

+
+const Container = styled.div``;
+
+const Header = styled.div`


시맨틱하게 header 어떤가여 ㅎㅎ

Suggested change

const Header = styled.div`

const Header = styled.header`

근데 제가 생각하는 헤더는 약간 이런거였는데 이렇게 보니 제 변수 네이밍이 잘못되었다는 생각이 드네요. 그냥 위에있어서 Header로 지은거 같네요 ㅋㅋㅋㅋ

YukJiSoo · 2020-04-15T17:53:23Z

src/config.ts

@@ -0,0 +1,5 @@
+const config = {
+  APISERVERPATH: "https://api.github.com",


_로 구분지어주면 더 보기 좋을것 같아요!

API_SERVER_PATH

YukJiSoo · 2020-04-15T18:02:00Z

src/components/RepoItem/index.tsx

+}
+
+export const RepoItem: React.FC<Props> = (props) => {
+  const { html_url, full_name, stargazers_count } = props;


API응답을 바로 넣어주느라 snake case 를 사용하신것 같은데
다른 부분에서도 camelCase를 사용했으니 camelCase로 통일해주는게 어떨까요??

Suggested change

const { html_url, full_name, stargazers_count } = props;

const { html_url: htmlUrl, full_name: fullName, stargazers_count: starCount } = props;

이런식으로 선언하면 무방할까요??

YukJiSoo · 2020-04-15T18:14:19Z

src/containers/Main.tsx

+  const userInfo = useMemo(() => {
+    return (
+      store.username && (
+        <UserInfo>
+          <UserName>{store.username}</UserName>
+          <Count>{store.repoCount} Repositories</Count>
+          <Count>{store.totalStars} Stars</Count>
+        </UserInfo>
+      )
+    );
+  }, [store.username, store.repoCount, store.totalStars]);


component로 사용되는 변수 같아보이는데 변수명을 Pascal case로 UserInfo 어떤가요??
밑부분의 style component는 Styled prefix를 붙여주고여!
convention을 통일하는게 좋다고 생각해요!ㅎㅎ

뭔가 useMemo로 선언하는 구문은 변수 같기도하고, styled-components 이름과 자주 겹치기도 해서 camel Case로 쓰는거 같네요... 좋은거 같아요!

YukJiSoo · 2020-04-15T18:18:39Z

src/stores/store.ts

+import axios from 'axios';
+import { action, computed, flow, observable } from 'mobx';
+
+import config from '../config';
+
+const { APISERVERPATH } = config;
+
+export default class Store {
+  @observable data = [];
+
+  @observable username: string = "";
+
+  @action
+  fetchData = flow(function* (this: Store, username: string) {
+    const response = yield axios.get(`${APISERVERPATH}/users/${username}/repos`);
+
+    if (response.status === 200) {
+      this.username = username;
+      this.data = response.data;
+    }
+  });
+
+  @computed
+  get totalStars() {
+    return this.data.reduce((prev, next: any) => prev + next.stargazers_count, 0);
+  }
+
+  @computed
+  get repoCount() {
+    return this.data.length;
+  }
+}


mobx 사용한 코드를 처음봤는데 재밌을 것 같아요! 많이 배울 수 있겠어요ㅎㅎ

저도 덕분에 context로 된 코드 보고있습니다 👍

joi0104 · 2020-04-16T07:34:09Z

src/serviceWorker.ts

+
+      if (isLocalhost) {
+        // This is running on localhost. Let's check if a service worker still exists or not.
+        checkValidServiceWorker(swUrl, config);


checkValidServiceWorker 보단 validateServiceWorker가 더 간단하지 않을까요?

음 프로젝트 생성하면서 자동으로 만들어진 파일이긴한데 저 파일이 어떤 용도로 쓰이는지부터 알아보겠습니다!

jieunjieun · 2020-04-16T14:14:07Z

src/stores/store.ts

@@ -0,0 +1,34 @@
+import axios from 'axios';
+import { action, computed, flow, observable } from 'mobx';
+


오 mobx.. 나중에 mobx 관련 발표도 고려 해주시면 좋을것 같슴다..! 궁금해요 몹엑스!

Yuni-Q · 2020-04-16T15:22:36Z

package.json

+  "name": "github_star",
+  "version": "0.1.0",
+  "private": true,
+  "dependencies": {


다른 리뷰에 있던 내용이긴 한데 dependencies와 devDependencies 분리하는것도 좋아 보입니다.

Yuni-Q · 2020-04-16T15:22:59Z

package.json

+    "test": "react-scripts test",
+    "eject": "react-scripts eject"
+  },
+  "eslintConfig": {


이것도 다른 리뷰에 있던건데 설정들 다른 파일로 뺄수 있는건 빼는것도 좋아 보입니다.

Yuni-Q · 2020-04-16T15:23:51Z

public/index.html

@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html lang="en">


여기도 다른 리뷰에 있던 내용인데 ko로 바꾸는건 어떨까요?

Yuni-Q · 2020-04-16T15:26:22Z

src/containers/App.tsx

+  return (
+    <BrowserRouter>
+      <Switch>
+        <Route exact path="/" component={Main} />


라우트가 하나뿐인데 react-router-dom을 쓰신 이유가 있을까요??

처음에 Main 페이지로 store를 inject했을 때, Provider를 달고도 따로 store를 주입해야하더라구요.
근데 react-router-dom을 쓰면 별 다른 처리를 하지 않아도 되서 그렇게 처리했는데
이렇게 수정할 계획입니다

Suggested change

<Route exact path="/" component={Main} />

const injected = props as InjectedProps;

const { store } = injected;

danivelop · 2020-04-16T17:13:56Z

src/containers/Main.tsx

+  const repoItems = useMemo(() => {
+    return store.data.map((el) => <RepoItem {...el} />);
+  }, [store.data]);


RepoItem컴포넌트에 key 속성을 넣어주지 않아도 렌더링에 문제없나요??

엇 key를 깜빡했네요 감사합니다!

danivelop · 2020-04-16T17:26:22Z

src/containers/Main.tsx

+        <Input onChange={onChange} name="username" onKeyUp={handleKeyUp} />
+        <StyledButton onClick={handleSearch}>Search</StyledButton>


username을 입력하는 input과 search버튼 대신 form태그를 사용하여 묶고 button대신 input태그의 submit type을 사용해보는건 어떨까요? 이렇게 하면 위의 handleKeyUp이벤트에서 눌린 버튼이 엔터인지 검사하지 않아도 form 태그가 알아서 엔터키 이벤트를 감지할 수 있을것 같아요!

form 태그가 엔터키 이벤트를 감지하는건 처음 알았네요 새로운거 하나 배워갑니다~!

오 저도 처음 알았어요

rdd9223

몹엑스와 styled-components 신기하고 흥미로웠습니다!! 배울 점이 많았던 코드같아요 ㅎㅎ 고생하셨습니다!

rdd9223 · 2020-04-16T18:35:01Z

src/components/RepoItem/index.tsx

+  }, [htmlUrl]);
+
+  return (
+    <Wrapper onClick={handleRedirect}>


styled-components를 처음봐서 굉장히 신기하네요!!!
style 컴포넌트만 따로 모듈화 시켜서 쓰는 방법도 있나요??

네 저도 아직 익숙하진 않은데 쓰기에 따라 많은 방면에 활용될거 같습니다...!

소소한 이야기 이지만,

onClick으로 링크를 이동시키는 것 보다, a tag 등을 활용하시는 것이 SEO와 accessibility에 좋습니다.

https://developer.mozilla.org/en-US/docs/Learn/Accessibility/HTML

뭔가 페이지를 이동한다기보단, 데이터 fetching을 하는 느낌이라 함수를 이용했습니다.

eastroots92

코드가 매우 깔끔하네요 👍

eastroots92 · 2020-04-17T11:57:02Z

public/manifest.json

@@ -0,0 +1,25 @@
+{


오 CRA에서 manifest도 생성해줬나요?

그런거 같네요 제가 따로 만들진 않았으니...ㅎ

eastroots92 · 2020-04-17T12:03:57Z

src/components/RepoItem/index.tsx

+  }, [htmlUrl]);
+
+  return (
+    <Wrapper onClick={handleRedirect}>


소소한 이야기 이지만,

onClick으로 링크를 이동시키는 것 보다, a tag 등을 활용하시는 것이 SEO와 accessibility에 좋습니다.

https://developer.mozilla.org/en-US/docs/Learn/Accessibility/HTML

eastroots92 · 2020-04-17T12:05:19Z

src/components/RepoItem/index.tsx

+
+interface Props extends RepoType {}
+
+export const RepoItem: React.FC<Props> = (props) => {


Suggested change

export const RepoItem: React.FC<Props> = (props) => {

export const RepoItem: React.FC<Props> = ({ htmlUrl, fullName, starCount }: Props) => {

다음과 같이 할 수 도 있습니다.

eastroots92 · 2020-04-17T12:08:52Z

src/components/RepoItem/index.tsx

+`;
+
+const UserName = styled.div`
+  color: #495057;


주로 사용하는 컬러가 정해져 있는 듯 한데

color Theme 하나 만들어서 사용해도 좋을 것 같아요!

eastroots92 · 2020-04-17T12:12:27Z

src/components/UserInfo/index.tsx

+const Wrapper = styled.div`
+  padding: 12px 0px;
+  border-bottom: 1px solid #ced4da;
+  margin-bottom: 24px;
+`;
+
+const UserName = styled.p`
+  font-weight: 300;
+  color: #343a40;
+  font-size: 42px;
+  margin: 0;
+`;
+
+const Count = styled.span`
+  font-weight: 400;
+  color: #343a40;
+  font-size: 24px;
+  padding-right: 12px;
+
+  & + span {
+    border-left: 2px solid #343a40;
+    padding-left: 12px;
+  }
+`;


component 들을 folder로 구분 지으셨던 것 같은데, 요것도 styles.ts 등과 같이 구분 짓는 것은 어떻게 생각하세요?

그러면 관심사가 분리되어 보기 더 좋을 것 같단 생각이 들어서요!

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

src/containers/Main.tsx

+
+  const handleKeyUp = useCallback(
+    (e: React.KeyboardEvent<HTMLInputElement>) => {
+      if (e.keyCode === 13) {


정말 소소하긴 하지만, 개인적으로는 13과 같은 내용들도 상수로 빼려고 노력하는 편이에요!

13이 어떤 key인지 모르시는 분들도 있기 때문에 아래와 같이 해서 협업하시는 분들이 좀 더 읽기 편하게 하기 위해서 입니다.

enum KEY_CODE { ENTER: 13, }; if (e.keyCode === KEY_CODE.ENTER) {}

eastroots92 · 2020-04-17T12:16:58Z

src/containers/Main.tsx

+        <Input onChange={onChange} name="username" onKeyUp={handleKeyUp} />
+        <StyledButton onClick={handleSearch}>Search</StyledButton>


오 저도 처음 알았어요

eastroots92 · 2020-04-17T12:21:11Z

src/components/RepoItem/index.tsx

+
+import { RepoType } from '../../types';
+
+interface Props extends RepoType {}


RepoType이 Type으로 정의되어있는 것 같은데 extneds로 해도 정상동작하나요??

typescript 가 버전 업 될때마다 급진적이게 이것저것 추가되어 제가 놓친 것 일수도있어용..

제가 알기론 저기선 Props가 type으로 선언되어있으면 extends가 안되는걸로 알아요 (저 코드에서 타입추론이 되긴 합니다)

kand193 · 2020-04-17T15:11:08Z

src/components/Button/index.tsx

+  font-size: 14px;
+  padding: 8px 12px;
+  opacity: 0.7;
+  border: 1px solid #868e96;


css를 width|height, font, background처럼 항목별로 묶어서 표현하면 더 읽기 좋아질것 같아요!

mango906 added 19 commits April 2, 2020 01:10

initialize

450d8c0

styled-components 추가

d82fa34

feat: Header 생성

3a1f74b

feat: Input 컴포넌트 생성

c77655c

feat: Button 컴포넌트 추가

8880964

style: Input border 삭제

32ab7c8

feat: github repo 가져오는 메소드 추가

16bdce8

feat: useInput hooks 추가

3733522

style: 쓸모없는 변수 삭제

3d3d23f

feat: repository 가져오는 api

b4740d5

feat: RepoItem 컴포넌트 생성

095b383

feat: RepoItem 데이터 바인딩

de5b09e

feat: mobx 스토어 연결

ef63a20

feat: store에 데이터 바인딩

01b5606

feat: 엔터로 검색 가능하게 추가

864eca0

feat: 아이템 눌렀을 때, 해당 레포로 이동하게 수정

bc862e4

style: RepoItem 스타일 수정

7fbeca7

refactor: 안쓰는 토큰값 삭제

f152d62

fix: useInput 타입 추론

16b8a05

lallaheeee reviewed Apr 15, 2020

View reviewed changes

YukJiSoo reviewed Apr 15, 2020

View reviewed changes

joi0104 reviewed Apr 16, 2020

View reviewed changes

mango906 added 3 commits April 16, 2020 20:18

fix: 스토어 이름 상수로 설정

6a24cea

style: APISERVERPATH 상수 네이밍 변경

9c1bf28

refactor: RepoItem props를 camel case로 변경

5bd6406

jieunjieun reviewed Apr 16, 2020

View reviewed changes

Yuni-Q reviewed Apr 16, 2020

View reviewed changes

mango906 added 5 commits April 17, 2020 01:11

refactor: react-router-dom 삭제

9bb512f

refactor: 필요없는 async 삭제

1ee8c9e

refactor: UserInfo 컴포넌트 분리

0199794

refactor: 데이터 fetch 후 camelCase로 저장

0a06c38

chore: package.json dependencies 분리

15516a5

danivelop reviewed Apr 16, 2020

View reviewed changes

rdd9223 reviewed Apr 16, 2020

View reviewed changes

eastroots92 reviewed Apr 17, 2020

View reviewed changes

kand193 reviewed Apr 17, 2020

View reviewed changes

mango906 added 20 commits April 18, 2020 01:38

refactor: useInput 간소화

1d5d388

refactor: store 싱글톤으로 생성

85935d3

chore: husky 테스트

9c59b4c

chore: husky 테스트

43c3eef

chore: prettier 재정의

ed7a237

chore: husky 테스트

034c1c2

chore: husky 테스트

74f46a2

chore: husky 테스트

4389622

chore: husky 테스트

1700191

chore: husky 테스트

7fb1467

chore: husky 테스트

be3a923

chore: husky 테스트

6a1adb1

chore: husky 테스트

59d69c3

chore: husky 테스트

e060f73

chore: husky 테스트

a3f3914

chore: husky 테스트

c7c9a0e

chore: husky 테스트

5a83ddc

chore: husky 테스트

e4a0373

chore: husky 테스트

b409458

chore: husky 세팅

6aec35c

		@@ -0,0 +1,5 @@
		const config = {
		APISERVERPATH: "https://api.github.com",

	const { html_url, full_name, stargazers_count } = props;
	const { html_url: htmlUrl, full_name: fullName, stargazers_count: starCount } = props;

		@@ -0,0 +1,34 @@
		import axios from 'axios';
		import { action, computed, flow, observable } from 'mobx';

-        <Route exact path="/" component={Main} />
+   const injected = props as InjectedProps;
+   const { store } = injected;

		<Input onChange={onChange} name="username" onKeyUp={handleKeyUp} />
		<StyledButton onClick={handleSearch}>Search</StyledButton>


		interface Props extends RepoType {}

		export const RepoItem: React.FC<Props> = (props) => {


		import { RepoType } from '../../types';

		interface Props extends RepoType {}

github_start_9 민경빈 과제 제출합니다. #4

Are you sure you want to change the base?

github_start_9 민경빈 과제 제출합니다. #4

Uh oh!

Conversation

mango906 commented Apr 5, 2020

Uh oh!

lallaheeee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mango906 Apr 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdd9223 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eastroots92 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mango906 Apr 15, 2020 •

edited

Loading