-
Notifications
You must be signed in to change notification settings - Fork 83
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
[view] 유저 정보 매칭 실패시 유저 아이콘 클릭 비활성화 #745
Conversation
@@ -5,24 +5,33 @@ import type { AuthorInfo } from "types"; | |||
import { GITHUB_URL } from "../../../constants/constants"; | |||
|
|||
const Author = ({ name, src }: AuthorInfo) => { | |||
const isUser = src.includes(GITHUB_URL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조금 더 구체적으로 isGitHubUser
와 같이 네이밍 해도 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋은 의견 감사합니다!! 추후 반영하도록 하겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
다음 PR도 기대됩니다 👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! 🔥💪
<Avatar | ||
alt={name} | ||
src={src} | ||
sx={{ width: 30, height: 30, minWidth: 30, minHeight: 30, cursor: "pointer" }} | ||
/> | ||
</a> | ||
) : ( | ||
<Avatar | ||
alt={name} | ||
src={src} | ||
sx={{ width: 30, height: 30, minWidth: 30, minHeight: 30, cursor: "pointer" }} | ||
sx={{ width: 30, height: 30, minWidth: 30, minHeight: 30 }} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avatar의 스타일 코드들이 중복으로 사용되고 있는데
const avatarStyle = { width: 30, height: 30, minWidth: 30, minHeight: 30, cursor: isUser ? "pointer" : "default" };
이런 식으로 변수에 담아서 써도 괜찮을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
의견 감사합니다!! 좋은 방법인 것 같아요👍 추후 수정하겠습니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGGGGTM입니다!!!!
Related issue
#571
Result
2024-10-02.5.44.38.mov
2024-10-02.5.42.45.mov
Work list
Discussion