Feat/#11/social login button #20
Conversation
워크플로우 추가를 위한 메인 병합
src/types/loginType/index.tsx
Outdated
| export type loginType = { | ||
| type: "google" | "kakao"; | ||
| }; | ||
|
|
||
| export type loginBtnType = { | ||
| type: "google" | "kakao"; | ||
| handler: () => void; | ||
| }; |
There was a problem hiding this comment.
IMO
소셜 로그인 타입은 프로바이더의 추가 또는 삭제 되는 경우와
다른 화면에서 로그인한 경로 확인이 필요한 경우 등
동일한 기준으로 다양한 사용처를 갖고 있다고 생각됩니다
따라서, 소셜로그인의 경로 ( 지원중인 프로바이더 목록 ) 을 enum 또는 상수로 관리하는 것이 좋아 보입니다
There was a problem hiding this comment.
Enum도 좋지만 Union 타입으로 선언하는 것도 좋아보입니다.
Enum의 경우 js객체로 변환되기 때문에 전체적인 코드 사이즈가 증가할 수 있다는 이유로 Union 타입을 선호하는 편입니다.
현재는 객체로 설정 되어 있지만 key 값이 한 개라 수정했습니다.
Type의 명명은 파스칼로 통일하는 것이 좋아보입니다.
export type SocialLoginType = "google" | "kakao";
// SocialLoginButton.tsx
interface Props {
type: SocialLoginType,
handler: () => void;
}
There was a problem hiding this comment.
흠 저는 잘 몰라서 그러는데, 혹시 enum 처리한다는 것이 객체를 만든다는 것과 같은 얘기인가요 ?? 저는 객체로 프로바이더들을 정의하면 좋을 것 같다는 생각입니다!
각 프로바이더를 예를 들어 'APPLE': '애플'과 같은 프로퍼티로 객체를 정의하면, 타입도 keyof 연산자를 써면 알아서 유니온 처리해주니까 관리하기 용이할 것 같아요. (용현님이 말씀하신 게 이런 방향이 맞는지는 모르겠지만..)
또 지금은 버튼에 나타낼 텍스트에 삼항연산자를 사용해주셨는데 두개보다 더 많아질 경우에는 사용하기 힘들어질 수 있을 것 같아서요!
src/component/SocialLoginButton.tsx
Outdated
| const SocialLoginButton = ({ type, handler }: loginBtnType) => { | ||
| return ( | ||
| <button | ||
| css={css` | ||
| width: 100%; | ||
| height: 4.8rem; | ||
| border-radius: 0.8rem; | ||
| background-color: ${backgroundColors[type]}; | ||
| text-align: center; | ||
| position: relative; | ||
| border: ${type === "google" ? "0.01rem solid rgba(0, 0, 0, 0.08)" : "none"}; | ||
| `} | ||
| onClick={() => { | ||
| handler(); | ||
| }} | ||
| > |
There was a problem hiding this comment.
IMO
후술한 소셜 로그인 경로를 관리할 경우, 각 경로 별 버튼을 정의하여 사용할 수 있을 것 같아요
src/component/SocialLoginButton.tsx
Outdated
| google: "#FFFFFF", | ||
| }; | ||
|
|
||
| const SocialLoginButton = ({ type, handler }: loginBtnType) => { |
There was a problem hiding this comment.
IMO
반환값 ( 랜더링 되는 컴포넌트 ) 기준 최상위 엘레멘트를 중심으로 props를 전달하는 경우, 해당 DOM요소의 기본 속성값을 갖는 타입을 상속하여 사용가능합니다 .
e.g) ComponentPropsWithoutRef<T = "target DOM Element">, HTMLAttribute<HTMLDomElement>
There was a problem hiding this comment.
svg형태의 스프라이트 이미지를 사용하는 방식은 되게 신기하네요 🫨 너무너무 잘 봤습니다!
컴포넌트를 구현하실 때 궁금한 점이 있어요! 지금은 구글이랑 카카오만 구현이 되어있는데, 추후에 앱스토어에 올리게 되었을때도 가정을 하면 자체 로그인이 없는 상태에서 애플 로그인이 들어갈 경우도 생기는데 현재 코드는 카카오와 구글에만 종속되어있는 형태 같아서 이 부분에 대해서는 추후 수정하실 생각이신지 궁금합니다! 나중에 추가된다고 하면 지금보다 수정사항이 조금 더 발생할 수도 있을 것 같아서요!
src/types/loginType/index.tsx
Outdated
| export type loginType = { | ||
| type: "google" | "kakao"; | ||
| }; | ||
|
|
||
| export type loginBtnType = { | ||
| type: "google" | "kakao"; | ||
| handler: () => void; | ||
| }; |
There was a problem hiding this comment.
흠 저는 잘 몰라서 그러는데, 혹시 enum 처리한다는 것이 객체를 만든다는 것과 같은 얘기인가요 ?? 저는 객체로 프로바이더들을 정의하면 좋을 것 같다는 생각입니다!
각 프로바이더를 예를 들어 'APPLE': '애플'과 같은 프로퍼티로 객체를 정의하면, 타입도 keyof 연산자를 써면 알아서 유니온 처리해주니까 관리하기 용이할 것 같아요. (용현님이 말씀하신 게 이런 방향이 맞는지는 모르겠지만..)
또 지금은 버튼에 나타낼 텍스트에 삼항연산자를 사용해주셨는데 두개보다 더 많아질 경우에는 사용하기 힘들어질 수 있을 것 같아서요!
| > | ||
| <LoginSpriteSvg type={type} /> | ||
| </div> | ||
| {type === "kakao" ? "카카오로" : "구글"} 로그인 |
There was a problem hiding this comment.
요기! 두개 이상이 되는 경우에 힘들어질 수 있을 것 같아요!
| onClick={() => { | ||
| handler(); | ||
| }} |
There was a problem hiding this comment.
기존 onClick 이벤트를 handler로 정의하신 이유가 궁금합니다!
There was a problem hiding this comment.
로그인 버튼에 onClick으로 로그인 로직을 넣는 것보단 handler라는 이름에 대입 하는게 직관적이라고 생각해서 위와 같이 정의하게 되었습니다!!
🏄🏼♂️ Summary (요약)
🫨 Describe your Change (변경사항)
type: kakao | google => 로그인 버튼의 종류를 입력합니다.handler: 클릭했을 때, 실행할 함수를 입력합니다.🧐 Issue number and link (참고)
📚 Reference (참조)
이미지 사용할 때 제가 자주 쓰는 최적화 기법인데, 궁금하신 분들은 아래 참조 보셔서 확인해보시면 좋을 것 같아요!!
SVG Sprite 기법 적용 사례