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

[#126] [FEATURE] E#10-S#7 비밀번호 재설정 뷰 #128

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

Juhee-Hwang
Copy link
Member

@Juhee-Hwang Juhee-Hwang commented Apr 4, 2022

✨ 구현 기능 명세

비밀번호 재설정 뷰 퍼블리싱을 하였습니다!

🎁 PR Point

  • 다른 기능들은 따로 넣진 않았고 퍼블리싱 작업 먼저 했어요!
  • 페이징이 잘 되었는지? (해당 폴더 위치가 맞는 위치인지..)
  • 비효율적인 구조는 아닐지?

🕰 소요시간

  • 5 hour + 2 hour

😭 어려웠던 점

  • 오랜만에 해서 이것저것 적응하는데 조금 시간이 걸린 것 같습니다..
  • 기능을 붙이는 것을 염두에 두고 작업을 해야하는데.. 그게 어려운 것 같아요ㅠ
  • 인증번호 입력 받는 부분.. 떨어져있는 입력칸을 한번에 입력할 수 있게 하고 싶은데 어떻게 해야할지 모르겠어요...
  • 라우팅 제대로 한거 맞는지? (파일 이름 저렇게 해도 되나요..? 하핳..)

@Juhee-Hwang Juhee-Hwang added 주희 주희가 맡았어요. 🔫 리뷰중 코드 리뷰가 진행중이에요. labels Apr 4, 2022
@Juhee-Hwang Juhee-Hwang self-assigned this Apr 4, 2022
@Juhee-Hwang Juhee-Hwang added ✨ feature 새로운 기능을 개발해요. 🥉 3의 우선순위를 가져요. labels Apr 4, 2022
Copy link
Member

@KimKwon KimKwon left a comment

Choose a reason for hiding this comment

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

각 텍스트마다 의미의 중요도가 다른데 전부 h3 태그로 작성된 것 같아요.
중요도에 따라 헤딩 레벨을 변경해보는 것은 어떨까요?

단순히 줄 바꿈을 위해서였다면 다른 block 태그 p나 div를 사용하셔도 될 것 같구요! ! !

`;

const StyledInput = styled.input`
width: 39.1rem;
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 하나하나 너비를 지정하면 나중에 수정하기 힘들거에요 . . .
이 컴포넌트 자체 너비가 39.1rem이니까
StyledRoot에 39.1rem 주시고 하위 태그들에게 width: 100%; 주는 방식은 어떨까요?

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
Member

@KimKwon KimKwon May 17, 2022

Choose a reason for hiding this comment

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

applyMediaQuery 함수 사용하시면 될 것 같아용!

${applyMediaQuery('mobile')} {
    width: 25rem;
}

이런 식으로 하셔도 되고
고정 값이 아닌 %로 주셔도됩니다!

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
Member

@sohee-K sohee-K left a comment

Choose a reason for hiding this comment

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

h3 태그는 용도에 맞게 변경하면 좋을 것 같아용! 고생했습니다~

Comment on lines 6 to 17
<StyledRoot>
<StyledTitleWrapper>
<h3>비밀번호를 잊어버리셨나요?</h3>
</StyledTitleWrapper>
<StyledExplain>
<h3>소담에 가입했던 이메일을 입력해주세요</h3>
<h3>비밀번호 재설정에 필요한 인증번호를 전송해드립니다</h3>
</StyledExplain>
<StyledInput placeholder="[email protected]" />
<StyledWarningMessage>존재하지 않는 이메일입니다</StyledWarningMessage>
<StyledButton>인증번호 발송</StyledButton>
</StyledRoot>
Copy link
Member

Choose a reason for hiding this comment

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

나중에 입력받은 값을 전송해야 하니까 입력받는 영역을 form 태그로 감싸면 좋을 것 같아요.
StyledButton의 type을 submit으로 설정하면 엔터 또는 버튼 클릭시 onSubmit 이벤트가 발생할 거예요!
근데 이렇게 되면 form의 자식 요소를 모두 Styled로 지정하는 게 아니라, css selector를 사용하는 게 더 깔끔할 것 같아요.

<StyledForm>
      <input placeholder="[email protected]" />
      <p>존재하지 않는 이메일입니다</p>
      <button type="submit">인증번호 발송</button>
</StyledForm>

Copy link
Member Author

Choose a reason for hiding this comment

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

맞다맞다 시맨틱태그.. 감사합니다!!!
근데 혹시 이렇게 했을 때 자식 요소를 css selector로 하는 것이 더 깔끔한 이유가 있나요!?

Copy link
Member

Choose a reason for hiding this comment

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

Styled 태그를 일일이 지정해주면 코드가 길어지기 때문이져.. ㅎㅎ 그리고 만약 중복되는 스타일이 있다면 하나의 Styled 안에서 selector를 사용하는 게 더 깔끔해요

padding-left: 1.6rem;
color: ${theme.colors.black2};

& > :placeholder {
Copy link
Member

Choose a reason for hiding this comment

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

placeholder는 &::placeholder 이렇게 설정합니당 (콜론 2개)

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅎㅎ... 감사해요...아직 갈길이 멀었네요 저는.. 희희

}
`;

const StyledWarningMessage = styled.h3`
Copy link
Member

Choose a reason for hiding this comment

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

여기도 h3으로 되어있는데 워닝 메세지는 h 태그 대신에 p 태그를 사용하는 게 좋을 것 같아요!

Copy link
Member Author

@Juhee-Hwang Juhee-Hwang May 17, 2022

Choose a reason for hiding this comment

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

태그를 적재적소에 사용하자! 감사합니다!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feature 새로운 기능을 개발해요. 🥉 3의 우선순위를 가져요. 주희 주희가 맡았어요. 🔫 리뷰중 코드 리뷰가 진행중이에요.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] E#10-S#7 비밀번호 재설정 뷰
3 participants