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

Feat/firebase authentication #7

Merged
merged 14 commits into from
Mar 9, 2024
Merged

Feat/firebase authentication #7

merged 14 commits into from
Mar 9, 2024

Conversation

SayisMe
Copy link
Member

@SayisMe SayisMe commented Mar 4, 2024

No description provided.

Copy link
Member

@vinivin153 vinivin153 left a comment

Choose a reason for hiding this comment

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

👍🏼

Comment on lines 4 to 12
const firebaseConfig = {
apiKey: process.env.NEXT_PUBLIC_FIREBASE_API_KEY,
authDomain: process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN,
projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID,
storageBucket: process.env.NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET,
messagingSenderId: process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
appId: process.env.NEXT_PUBLIC_FIREBASE_APP_ID,
measurementId: process.env.NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID,
};
Copy link
Member

Choose a reason for hiding this comment

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

찬섭님이 작성한 src/constants/firebase.ts 이용해도 좋을 것 같습니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

동의합니다! 동일한 폴더/파일에 객체로 관리되면 더욱 좋을 것같아요

Copy link
Member Author

Choose a reason for hiding this comment

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

찬섭님께서 작성해주신 동일 파일로 수정완료하였습니다 ✅

Copy link
Contributor

@yeongjunekoh yeongjunekoh left a comment

Choose a reason for hiding this comment

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

전반적으로 작업하신 사항들 모두 확인하였습니다!

카카오와 소셜 로그인 연동 부분은 아직 안되어있는 것으로 확인했습니다:D

요것은 폴더 구조와 관련된 것인데, component안에 LoginForm과 같은 폴더를 만든 뒤에 세부 사항에서 index.tsx처럼 관리하는 것이 어떨지요?

components
| - LoginForm
|   | - index.tsx
|   | - index.module.css
| - SignupForm
|   | - index.tsx
|   | - index.module.css

해당 부분은 저희 모두 공통적으로 맞춰야 하는 부분이기에 논의에 한 번 올리도록하겠습니다!

+) PR도 문서화를 신경쓰면 어떨까라는 생각입니다!
+) 해당 CSS 모듈에서 tailwind 프레임워크를 사용하게 css 변경 요청드립니다!

Comment on lines 4 to 12
const firebaseConfig = {
apiKey: process.env.NEXT_PUBLIC_FIREBASE_API_KEY,
authDomain: process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN,
projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID,
storageBucket: process.env.NEXT_PUBLIC_FIREBASE_STORAGE_BUCKET,
messagingSenderId: process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
appId: process.env.NEXT_PUBLIC_FIREBASE_APP_ID,
measurementId: process.env.NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

동의합니다! 동일한 폴더/파일에 객체로 관리되면 더욱 좋을 것같아요

@SayisMe
Copy link
Member Author

SayisMe commented Mar 9, 2024

  • firebaseConfig를 통합시켰습니다.
  • tailwind를 이용하여 CSS를 수정하였습니다.
  • jest test code (user) 를 일부 수정하였습니다.
  • firebase authentication을 적용하였습니다.
  • 로그인시 firestore에 유저 정보가 uid를 기준으로 저장되도록 하였습니다.

@yeongjunekoh @vinivin153

Code Review 꼼꼼히 해주심에 감사드립니다 👍🏻

@SayisMe SayisMe merged commit 6723742 into main Mar 9, 2024
4 checks passed
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.

3 participants