Refactor/#400 비로그인 상태로 댓글 작성 시도 시 로그인 권유 모달이 표시되도록 변경 #401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
변경 과정에서 Avatar, Input 컴포넌트에 대한 중복된 삼항연산자를 제거하였음.
Creative-Lee
added
[ 🌞 FE ]
Creative-Lee
commented
Sep 11, 2023
| ) : ( | ||
| <> | ||
| <Avatar src={defaultAvatar} alt="익명 프로필" /> | ||
| <Input type="text" onFocus={recommendLogin} placeholder="댓글 추가..." maxLength={0} /> |
There was a problem hiding this comment.
💬 onFocus 이벤트로 모달을 띄우기 때문에 input에 값을 입력할 수 없지만,
maxLength를 0으로 명시해서 입력할 수 없는 input이라는 의미를 주고싶었는데 여러분들은 어떻게 생각하시는지요?
위쪽, 정상 input에 maxLength 200과 대조 되어 의미가 강조될 것 같아 작성했습니다.
There was a problem hiding this comment.
혼자 조금 더 생각해 봤는데 없어도 의미 전달 잘 되는 듯 해서 제거했습니다!
ukkodeveloper
approved these changes
Sep 13, 2023
Comment on lines
48
to
51
| const recommendLogin: React.FocusEventHandler<HTMLInputElement> = (e) => { | ||
| e.currentTarget.blur(); | ||
| openLoginModal(); | ||
| }; |
There was a problem hiding this comment.
👍 모달을 열때 input을 blur 시키는 디테일 좋네요
Comment on lines
31
to
33
| <LoginButton type="button" onClick={goLoginPage}> | ||
| 로그인하러 가기 | ||
| </LoginButton> |
There was a problem hiding this comment.
💬 이부분은 Link로 개선되어도 될 것 같네요
굳이 navigate 함수가 필요한 로직은 아닌 것 같아서 링크로만 변경되어도 될 것 같아요
There was a problem hiding this comment.
좋은 의견 감사합니다~ 반영했습니다~
cruelladevil
approved these changes
Sep 13, 2023
input을 모달 열린 상태일 때 비활성화 하도록 하여 핸들러 함수를 제거함
Creative-Lee
force-pushed
the
refactor/#400
branch
from
d7af869 to
6c82e84
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝작업 내용
💬리뷰 참고사항
코드에 간단한 설명 코멘트 적어둘게요~!
참고해 주세요~
본 PR은 #392 PR 먼저 merge 후 충돌 해결하여 merge할 예정입니다.
충돌 예상 코드는 LoginModal 내부 네비게이션 코드입니다~
#️⃣연관된 이슈
close #400