github_star_9 육지수 과제 제출합니다. by YukJiSoo · Pull Request #7 · mash-up-kr-web/github_star_9

YukJiSoo · 2020-04-05T13:19:55Z

No description provided.

for using async/await.

view focus on only presentation logic.

- page layer 추가 - globalStyle을 App.tsx로 이동(의미상 자연스럽게 느껴져서)

jieunjieun · 2020-04-14T02:14:56Z

+  &:hover {
+    background-color: #dcdcdc;
+  }
+`;


질문이 한가지 있습니다!
css 작성하신 것 보면 한 줄이 띄워져있는 것도 있고 붙어 있는 줄도 있는 것 같은데, 이런 형태는 처음 봐서 신기해서용!
혹시 어떤 기준으로 줄을 나누신건지 알 수 있을까요?

매번 css를 작성할 때 마다 달라지긴하는데 제가 알아보기 쉬우려고 그룹을 나눠서 개행을 해요!
예를 들어서 display: flex 관련된 그룹, padding/margin 관련된 그룹, width/height 이런식으로 나눠서 작성하면 보기에 편하더라구요!

혼자서 할 때는 잘 지키지는 않지만...

https://moo-you.tistory.com/21
옛날에 봤던 css 선언 순서와 관련된 글인데 이 글을 보면서 시작?된 것 같아요.

오 👍
지수님 요 순서 다 외우셨나요?? ㅋㅋㅋㅋ

회사에서 맨날 놓쳐서 푸시전에 솔팅 툴 돌리는 1인..

아녀 못 외우고 저만의 규칙? 으로 매번 달라져여ㅋㅋㅋㅋㅋㅋㅋㅋㅋ

mango906 · 2020-04-14T12:23:35Z

+  .background {
+    position: absolute;
+
+    z-index: -1;
+    top: 0;
+    left: 0;
+
+    background-color: #fcffbe;
+    width: 100%;
+    height: 100%;
+
+    transform: skew(-20deg);
+
+    animation-name: changeColor;
+    animation-duration: 15s;
+    animation-iteration-count: infinite;
+  }


styled-components에 이렇게 클래스를 선언할 수 있는지 몰랐네요.. 덕분에 좋은거 하나 알게된거 같아요 감사합니다!

lallaheeee

LGTM ⚡️ 고생하셨습니다

lallaheeee · 2020-04-14T11:43:20Z

+  title: string;
+}
+
+const BeautifulTitle: React.FC<BeautifulTitle> = ({ title }) => {


정말 Beautiful 합니다 ~! 🤩

앜ㅋㅋㅋㅋ 희라님ㅋㅋㅋ 핵웃곀ㅋㅋ

근데 저 섬세함에 감탄하고 갑니다.

lallaheeee · 2020-04-14T11:45:33Z

+  };
+
+  const handleInputKeyEvent = (e: React.KeyboardEvent<HTMLInputElement>) => {
+    if (e.keyCode !== 13) return;


Suggested change

if (e.keyCode !== 13) return;

const ENTER = 'Enter'

if (e.key !== ENTER) return;

어떠나유 ㅎㅎ

@lallaheeee 저 저번에 이렇게 써본거같은데 이럴 경우에 모바일 키보드에서 전송(?) 버튼 누르면 Enter만으로는 작동 안하는걸로 알아요!

@lallaheeee 물론 지금은 웹이긴 하지만요...

@mango906 오 새로운 사실 알아갑니다 ㅎㅎ

그렇다면 어떠나유

Suggested change

if (e.keyCode !== 13) return;

const ENTER = 'Enter'

if (e.keyCode !== 13 || e.key !== ENTER ) return;

@lallaheeee e.key !== ENTER를 추가한데 특별한 이유가 있으신가요??

@mango906
13이라는 숫자가 enter라는 걸 알기 힘드니까
상수로 값의 의미를 명확히 기술하여 가독성을 높이면 좋겠다는 생각이 들었어요 ㅎㅎ

const ENTER = 13 if( e.keyCode !== ENTER )

이ㅇㅓ도 좋을것같아여 ㅎㅎ

@lallaheeee 좋아요!

찾아보니 경빈님이 말씀하신 모바일 키패드 문제도 그렇구 브라우저나 모바일 별로 key code에 대한 이슈가 있는 것 같았어요.
몇몇 사람들은 key code 처리에 대한 polyfill 함수를 만들어서 사용하는 것 같아서 그런 함수를 만드는것도 좋은 것 같아요!

keyCode is deprecated
stack overflow related to keyCode

오 이런 디테일 매우 좋아요 ㅋㅋㅋㅋ
Enter가 있다는 것은 처음알았네요 👍

lallaheeee · 2020-04-14T11:47:57Z

+        <li key={name}>
+          <Styled.RepositoryItem href={link}>
+            <span className="repository-name">{name}</span>
+            {description && <span className="repository-description">{description}</span>}


레포 description도 보여주는 건가요? 섬세하시네유..👍🏻

lallaheeee · 2020-04-14T12:12:33Z

+  return <UserInfoContext.Provider value={{ userInfo, searchUsefInfo }}>{children}</UserInfoContext.Provider>;
+};
+
+export default UserInfoProvider;


Suggested change

export default UserInfoProvider;

export default function useUserInfo {

return useContext(UserInfoContext);

}

굉장히 사소한건데, 이렇게 사용하면 편하더라고요 ~

lallaheeee · 2020-04-14T12:17:18Z

+
+  const historyPushForSearchUserInfo = useCallback(
+    (username: string) => {
+      history.push(`?username=${username}`);


검색 기록을 히스토리로 관리해주는 거 좋네요 배워갑니다ㅎㅎ

lallaheeee · 2020-04-14T12:21:09Z

+    async (username: string) => {
+      chagePageStatus(PageStatus.Loading);
+
+      checkHasBeenSearched();


API 요청을 중복 요청을 방지를 위해 확인하는 것 같은 네이밍처럼 느껴지는데 checkScrolled 는 어때유??

첫 검색 시에 moveTop state를 true로 변경시켜주고
moveTop state가 false에서 true로 변경되면 타이틀이 상단으로 움직이는 animation이 실행되요.
moveTop state를 보고 변경해주는 함수가 checkHasBeenSearched이고
첫 검색이후에는 아무런 작업을 하지 않습니다.

희라님이 API 요청을 중복 요청을 방지를 위해 확인하는 함수라고 생각하셨으니 저 함수의 네이밍이 잘못된 것 같네요!
좀 더 괜찮은 이름을 생각해보는게 좋겠어요!

lallaheeee · 2020-04-14T12:27:25Z

+const request = <T>(path: string, option: Option) => {
+  return fetch(`${GITHUB_API_BASE_URL}${path}`, option).then((res) => res.json() as Promise<T>);
+};
+


분리 하신 걸를 보니 다른 프로젝트에서도 재사용하실 것 같은데
apiPaths 처럼 baseURL을 분리해도 좋을 것 같아유 ㅎㅎ

Suggested change

const request = <T>(path: string, option: Option) => {

return fetch(`${GITHUB_API_BASE_URL}${path}`, option).then((res) => res.json() as Promise<T>);

};

const request = (baseURL: string) => <T>(path: string, option: Option) => {

return fetch(`${baseURL}${path}`, option).then((res) => res.json() as Promise<T>);

};

오 좋아여!

lallaheeee · 2020-04-14T12:28:56Z

+  const option = {
+    method: HttpMethod.Get,
+  };


request 함수의 default 로 설정하면 어떨까요?

지금은 api 함수가 하나밖에 없지만 확장성을 고려했을때 Get method 타입의 option이 default 옵션은 아니라고 생각했어요!
cors나 header 관련 option이 추가되어야 한다면 그 값이 base가 되는 형식으로 작성되어야 하지 않을까 생각합니다!

lallaheeee · 2020-04-14T12:30:31Z

+      'module-resolver',
+      {
+        alias: {
+          '^~/(.+)': './src/\\1',


지수님 hoxy 정규식의 고수..?

아니요~ 검색해서 나온거 따라한거~

lallaheeee · 2020-04-14T12:31:13Z

+const Button: React.FC<ButtonProps> = (props) => {
+  const { children, type = 'button', ...rest } = props;
+
+  // eslint-disable-next-line react/button-has-type


6줄이 있는데, ESlint 설정을 바꾸는 것보다 주석으로 처리하는 걸 선호하는 이유가 있을까요?!

전체적인 lint설정은 유지하면서 특정 경우에 한해서만 lint를 disable해주고 싶어서 그랬습니다ㅎㅎ

NuclearPunch · 2020-04-15T11:12:05Z

+};
+
+const SearchResult: React.FC<SearchResultProps> = ({ userInfo }) => {
+  if (!userInfo) return null;


null을 꼭 뱉어야하는 상황이 아니라면 생략해도 좋을 것 같습니다!

userInfo가 빈값으로 오는 경우가 있을거라고 생각해서 이 라인을 작성해주었는데
생각해보니 user가 존재하지 않는 경우는 404응답이 오고 error 처리를 해주기 때문에 생략하는게 맞는 것 같아요!
감사합니다!! 👍

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

+      <Styled.UserMetadata>
+        <div className="user-name">{username}</div>
+        <div className="user-repo-stars">
+          {repositories.length} Repositories / {starCount} stars


1개인 경우에 단어를 바꿔주는건 어떨까요?

오 감사해요 섬세함!!

kand193 · 2020-04-15T13:24:21Z

@@ -0,0 +1,31 @@
+import React, { createContext, useState } from 'react';
+
+export enum PageStatus {


joi0104 · 2020-04-16T15:02:23Z

+  width: 100%;
+
+  position: absolute;
+  top: 30vh;


vh 단위를 처음보는데.. 혹시 설명해주실 수 있나요? % 단위와 다른가요?

뷰포트의 높이값입니다!

브라우저 높이가 만약에 1080px이면 30vh는 1080 * 0.3 px입니다ㅏ

Yuni-Q · 2020-04-16T16:58:15Z

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


ko로 수정하면 어떨까요?

Yuni-Q · 2020-04-16T16:59:07Z

+  return (
+    <BrowserRouter>
+      <Switch>
+        <Route path="/search" component={SearchPage} />


루트(/)로 접근했을땐 어떻게 되는걸까요?

루트를 고려못했네요! 감사합니다!!

루트로 접근했을 떄 /search 로 리다이렉트 시키거나,
루트로 path를 변경해주거나 방법은 여러가지일 것 같은데
현재 상황에서 여러 페이지가 존재하는게 아니니 기존의 searct path를 루트로 변경해주는게 좋을 것 같아요!

Yuni-Q · 2020-04-16T17:01:27Z

+import React from 'react';
+import Loader from 'react-loader-spinner';
+
+const LoadingSpinner: React.FC<{ className: string }> = ({ className }) => {


Props를 따로 빼면 가독성이 조금 더 좋아질거 같습니다 !

Yuni-Q · 2020-04-16T17:03:39Z

+const ButtonMemo = React.memo(Button);
+
+const SearchKeywordBox: React.FC<SearchKeywordBoxProps> = ({ search, initialKeyword }) => {
+  const [keyword, setKeyword] = useState<string>(initialKeyword);


const SearchKeywordBox: React.FC = ({ search, initialKeyword = '' }) => {
이런 식으로 선언해도 useState에 제네릭을 주어야 하나요?

윤희님이 말씀하신대로 하면 타입추론이 되어서 제네릭 안넘겨줘도 될 될거에용

danivelop · 2020-04-16T18:22:12Z

+
+      checkHasBeenSearched();
+
+      await delay(1000);


delay는 어떤것 때문에 주셨는지 궁금합니다!

api 요청할 때 로딩 spinner를 보여주고 싶어서 추가했어요!
다른 방법이 있을 것 같은데 모르겠어서 무식하게 setTimeout을 주었답니다...허허

eastroots92

와 지수님 진짜 많은 것을 배우고 갑니다. 👍

eastroots92 · 2020-04-17T13:06:51Z

@@ -0,0 +1 @@
+defaults


eastroots92 · 2020-04-17T13:11:01Z

+  &:hover {
+    background-color: #dcdcdc;
+  }
+`;


오 👍
지수님 요 순서 다 외우셨나요?? ㅋㅋㅋㅋ

회사에서 맨날 놓쳐서 푸시전에 솔팅 툴 돌리는 1인..

eastroots92 · 2020-04-17T13:13:02Z

+  title: string;
+}
+
+const BeautifulTitle: React.FC<BeautifulTitle> = ({ title }) => {


앜ㅋㅋㅋㅋ 희라님ㅋㅋㅋ 핵웃곀ㅋㅋ

근데 저 섬세함에 감탄하고 갑니다.

eastroots92 · 2020-04-17T13:14:10Z

+const ButtonMemo = React.memo(Button);
+
+const SearchKeywordBox: React.FC<SearchKeywordBoxProps> = ({ search, initialKeyword }) => {
+  const [keyword, setKeyword] = useState<string>(initialKeyword);


윤희님이 말씀하신대로 하면 타입추론이 되어서 제네릭 안넘겨줘도 될 될거에용

eastroots92 · 2020-04-17T13:15:07Z

+  };
+
+  const handleInputKeyEvent = (e: React.KeyboardEvent<HTMLInputElement>) => {
+    if (e.keyCode !== 13) return;


오 이런 디테일 매우 좋아요 ㅋㅋㅋㅋ
Enter가 있다는 것은 처음알았네요 👍

mash-up-kr-web#7 (comment)

because of type inference mash-up-kr-web#7 (comment)

YukJiSoo added 30 commits March 31, 2020 19:29

dev: init project setting

1c4d76d

chore(npm): install styled-components

789758c

feat(style): add global style

5fa3a3e

feat(markup): add SearchPage

ae7fb2b

feat(component): add Button component

844db1a

feat(component): add Input component

167c32f

feat(style): add search page style

16fd361

feat(component): add SearchKeywordBox component

c4996ce

feat(component): add BeautifulTitle component

50da533

chore(babel): add @babel/plugin-transform-runtime

20fce08

for using async/await.

feat: add api module

585fe0c

chore(eslint): add rule

7896e7a

feat(model&parser): add UserInfo model & parser

6087880

chore(npm): install react-icons

2e8d538

feat(component): add SearchResult component

19008fa

feat(search): apply search api to SearchPage

966652c

feat(SearchResult): add repository description

bb3427e

feat(animation): move top when search first start

be2cd1b

feat(component): add api response display component - loading, error

0a7ae4a

refactor: seperate search page component - body

23d4af4

refactor: move parser logic to api layer from view

52c89fd

view focus on only presentation logic.

refactor: change Loading component name for clarity

29e38aa

refactor: seperate js and style file

32887e3

feat: add repository link

5efdc1a

docs: update README.md

973f17d

feat: use query string to show search username

eb75dbb

refactor: seperate PageStatus context

d1d1d4d

refactor: seperate UserInfo context

99a429b

refactor: seperate history and page start location logic

1b65a30

refactor: change direcotory structure

4bfebfc

- page layer 추가 - globalStyle을 App.tsx로 이동(의미상 자연스럽게 느껴져서)

YukJiSoo added 2 commits April 5, 2020 23:03

chore: delete DS_Store

dbbc6b1

fix(context): add initialUserInfo props to UserInfoProvider

6b35b81

jieunjieun reviewed Apr 14, 2020

View reviewed changes

mango906 reviewed Apr 14, 2020

View reviewed changes

lallaheeee approved these changes Apr 14, 2020

View reviewed changes

NuclearPunch reviewed Apr 15, 2020

View reviewed changes

kand193 reviewed Apr 15, 2020

View reviewed changes

joi0104 reviewed Apr 16, 2020

View reviewed changes

Yuni-Q reviewed Apr 16, 2020

View reviewed changes

danivelop reviewed Apr 16, 2020

View reviewed changes

eastroots92 reviewed Apr 17, 2020

View reviewed changes

YukJiSoo added 9 commits April 18, 2020 01:06

refactor: replace import path to alias path

37fa6cb

refactor: check enter key pressed with util function

0fcf881

mash-up-kr-web#7 (comment)

refactor: modify function name to explicitly indicate meaning

8d94f0e

mash-up-kr-web#7 (comment)

refactor: set userInfo props mandatory

7b7516b

mash-up-kr-web#7 (comment)

refactor: apply singular, plural type to repo, star count phrase

6ed92f2

mash-up-kr-web#7 (comment)

refactor: change html language attribute for korea

036c804

refactor: chagnge path name

2122dff

mash-up-kr-web#7 (comment)

refactor: add props interface for readability

d4ce4ec

mash-up-kr-web#7 (comment)

refactor: remove useState generic type

725403d

because of type inference mash-up-kr-web#7 (comment)

-    if (e.keyCode !== 13) return;
+    const ENTER = 'Enter'
+    if (e.key !== ENTER) return;

-export default UserInfoProvider;
+export default  function useUserInfo {
+    return useContext(UserInfoContext);
+}

		@@ -0,0 +1,31 @@
		import React, { createContext, useState } from 'react';

		export enum PageStatus {

		@@ -0,0 +1 @@
		defaults

Uh oh!

Conversation

YukJiSoo commented Apr 5, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YukJiSoo Apr 14, 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!

YukJiSoo Apr 17, 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!

lallaheeee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eastroots92 Apr 17, 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!

lallaheeee 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!

lallaheeee Apr 14, 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!

lallaheeee Apr 14, 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

YukJiSoo Apr 14, 2020 •

edited

Loading

YukJiSoo Apr 17, 2020 •

edited

Loading

eastroots92 Apr 17, 2020 •

edited

Loading

lallaheeee Apr 15, 2020 •

edited

Loading

lallaheeee Apr 14, 2020 •

edited

Loading

lallaheeee Apr 14, 2020 •

edited

Loading