Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ORT-2] feat: add KakaoLoginButton #2

Merged
merged 10 commits into from
Aug 30, 2024
Merged

Conversation

crew852
Copy link
Contributor

@crew852 crew852 commented Aug 16, 2024

맡은 태스크를 맞게 수행하는 코드를 작성했는지 잘 모르겠네요...
일단 로그인 버튼 -> 로그인 페이지 -> 리다이렉트 -> 백엔드에 code값과 state값 전달이 되게 만들어 봤습니다.
많은 도움 부탁드립니다...

Copy link

Copy link

cloudflare-workers-and-pages bot commented Aug 16, 2024

Deploying ort-storybook with  Cloudflare Pages  Cloudflare Pages

Latest commit: dd22322
Status: ✅  Deploy successful!
Preview URL: https://1cb203aa.ort-storybook.pages.dev
Branch Preview URL: https://ort-2-add-kakaologinbutton.ort-storybook.pages.dev

View logs

Copy link

cloudflare-workers-and-pages bot commented Aug 16, 2024

Deploying ort-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: dd22322
Status: ✅  Deploy successful!
Preview URL: https://3d70f2c7.ort-web.pages.dev
Branch Preview URL: https://ort-2-add-kakaologinbutton.ort-web.pages.dev

View logs

Comment on lines +4 to +6
const clientId = 'f5aa2f20e42d783654b8e8c01bfc6312';
//redirectUri는 등록된 redirectUri중에 임의로 사용했습니다.
const redirectUri = 'http://localhost:5173/oauth/kakao';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우선은 상관없지만, 제가 Env 관련 작업을 하고 나면 환경변수로 변경하면 좋을 듯합니다!

@@ -0,0 +1,71 @@
//HTTP 요청을 보내기 위해 axios라이브러리를 추가했습니다.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 API 요청 코드베이스를 작성 중인데, axios를 굳이 사용하지 않아도 작업이 가능할 것 같습니다.
제 작업이 끝나고 나면 그 코드베이스 기반으로 제가 수정해 놓겠습니다!

app/routes/oauth.kakao.tsx.tsx Outdated Show resolved Hide resolved
app/routes/oauth.kakao.tsx.tsx Outdated Show resolved Hide resolved
app/routes/oauth.kakao.tsx.tsx Outdated Show resolved Hide resolved
app/components/KakaoLoginButton/KakaoLoginButton.tsx Outdated Show resolved Hide resolved
@aube-dev aube-dev requested a review from Godbell August 16, 2024 18:51
Copy link
Member

@aube-dev aube-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

app/routes/oauth.kakao.tsx.tsx Outdated Show resolved Hide resolved
@crew852 crew852 requested a review from aube-dev August 30, 2024 07:24
app/utils/getUUID.ts Outdated Show resolved Hide resolved
return getUserUUID;
};

export default getUUID;
Copy link
Member

@aube-dev aube-dev Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

한 파일에 하나의 함수만 넣는 방식인 것 같은데, 그러기엔 코드가 너무 짧아 보여서 그냥 app/utils/random.ts 정도로만 네이밍하고, default export가 아닌 named export를 하는 게 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그게 맞는 것 같긴 한데, 이 외에 이 함수와 같이 들어갈 만한 다른 함수가 생각나지 않아서 이렇게 작성했습니다.
그러나 말씀해주신 대로 이름 붙여 놓으면 나중에 뭔가 추가되어 수정을 하게 되어도 괜찮겠네요!
이것도 반영해 수정해 둘게요~

package.json Outdated
"isbot": "4.4.0",
"react": "18.3.1",
"react-dom": "18.3.1"
"react-dom": "18.3.1",
"uuid": "^10.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^을 붙이면 일관된 버전을 적용하지 않아서 예상치 못한 breaking change를 디버깅하기 어려울 수 있어요.

Suggested change
"uuid": "^10.0.0"
"uuid": "10.0.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오...이건 그냥 몰랐습니다
정말 위대합니다, 선생!

package.json Outdated
@@ -39,6 +41,7 @@
"@storybook/test": "8.2.5",
"@types/react": "18.3.3",
"@types/react-dom": "18.3.0",
"@types/uuid": "^10.0.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"@types/uuid": "^10.0.0",
"@types/uuid": "10.0.0",

Comment on lines 9 to 19
try {
const response = await axios.post('(백엔드api url)', {
code,
state,
});
return json({ message: 'Success', data: response.data });
} catch (error) {
const axiosError = error as AxiosError;
console.error('Error sending code to backend:', axiosError);
return json({ message: 'Error', error: axiosError.message });
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드 부분 혹시 지워주실 수 있을까요? 제가 이어서 작업하겠습니다!

Copy link
Contributor Author

@crew852 crew852 Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지우면 커밋이 불가해서 대충 console.log 찍는 try문으로 대체해 놨습니다.
코드의 해당 부분 지우고 이어서 해주시면 될 것 같습니당~

  // 이 아래의 try문 지우고 이어서 하면 됩니다
  try {
    console.log(code, state);
  } catch {
    console.log(json);
  }
};

package.json Outdated
@@ -21,9 +21,11 @@
"@remix-run/react": "2.10.2",
"@remix-run/serve": "2.10.2",
"@vanilla-extract/css": "1.15.3",
"axios": "^1.7.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 라이브러리 지워주실 수 있을까요? 제가 API 부분 작업하려고 합니다!

Copy link
Member

@aube-dev aube-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@aube-dev aube-dev merged commit a5cd638 into main Aug 30, 2024
3 checks passed
@aube-dev aube-dev deleted the ORT-2_add-KakaoLoginButton branch August 30, 2024 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants