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

Design: 인증 / 설문 레이아웃 #24

Merged
merged 5 commits into from
May 29, 2022

Conversation

choisohyun
Copy link
Member

@choisohyun choisohyun commented May 29, 2022

🧑‍💻 PR 내용

  • 설문/인증 페이지 레이아웃 템플릿 만들었습니다 !
  • 전체 페이지 margin, height 을 줘서 감싸는 레이아웃도 추가했습니다

Screen Shot 2022-05-29 at 15 42 03

의견

  • 설문 페이지의 경우 페이지전략을 어떻게 가져갈지 좀더 고민이 필요할 것 같은데 어떻게 생각하시는지 궁금합니다. @Jeong-jeong @leejiho9898
    1. /survey/:surveyId로 하나의 라우터 페이지를 가지고 내용만 바뀌게 하는 방식
    2. /survey 하위에 설문마다 라우터 페이지를 정해 두고 계속 페이지 이동하게 하는 방식

저는 설문이 많아서 2번이 덜 복잡해지지 않을까 싶기도 한데 1번도 컴포넌트를 잘 해두면 괜찮을것 같아서 고민이 되네요 🤔🤔

📸 스크린샷

템플릿 안에 내용은 임의로 넣어둔것입니다!

설문(프로그래스바 있음) 인증 페이지(프로그래스바 없음)
Screen Shot 2022-05-29 at 15 33 52 Screen Shot 2022-05-29 at 15 33 42

@choisohyun choisohyun added Feat 새로운 기능 추가 Design UI 디자인 변경, css labels May 29, 2022
@choisohyun choisohyun self-assigned this May 29, 2022
Copy link
Contributor

@Jeong-jeong Jeong-jeong left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 저도 소현님처럼 라우터로 이동시키는게 더 편할 것 같긴 합니다!

Copy link
Contributor

@Jeong-jeong Jeong-jeong left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

<NavigationWrapper>
{hasProgressBar && <ProgressBar currStep={currStep} totalStep={totalStep} />}
<ButtonWrapper>
<Button size="large" variant="gray">
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Button size="large" variant="gray">
<Button size="medium" variant="gray">

</Button>
<Button
onClick={() => console.log('aa')}
size="large"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
size="large"
size="medium"

Comment on lines +48 to +49
font-family: SangjuGotgam, Pretendard Variable, -apple-system, BlinkMacSystemFont, system-ui, Roboto, 'Helvetica Neue', 'Segoe UI',
'Apple SD Gothic Neo', 'Noto Sans KR', 'Malgun Gothic', 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol', sans-serif;
Copy link
Contributor

Choose a reason for hiding this comment

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

Logo 폰트 패밀리가 기존과 다른가요? 아니면 theme이나 공통된 곳에서 사용하는 것이 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

로고가 SangjuGotgam 폰트여서 기존 폰트에서 SangjuGotgam만 추가했습니다..!
로고 폰트는 여기서만 쓰이는거 같아서 바로 썼는데 theme에 넣어두는게 나을까요??
요거 수정은 다음 작업할 때 같이 하고 머지 먼저 하겠습니다~

'Apple SD Gothic Neo', 'Noto Sans KR', 'Malgun Gothic', 'Apple Color Emoji', 'Segoe UI Emoji', 'Segoe UI Symbol', sans-serif;
font-weight: 400;
font-size: 14px;
line-height: 125%;
Copy link
Contributor

Choose a reason for hiding this comment

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

line-height는 %값을 쓰는군요! 신기합니다

Copy link
Member Author

Choose a reason for hiding this comment

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

이거는 피그마에 있는대로 가져온것인데 저도 %는 처음 써본거같네요 ㅎㅎ
https://webclub.tistory.com/631

@leejiho9898
Copy link
Member

저도 나중에 소개팅 부분까지 고려하면 많이 복잡해질 것 같아서 2번으로 하는게 더 깔끔하지 않을까 생각합니다

Copy link
Member

@leejiho9898 leejiho9898 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

return (
<>
<HeaderWrapper>
<Logo to="/">외딴썸</Logo>
Copy link
Member

Choose a reason for hiding this comment

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

로고 부분은 이미지로 하실 줄 알았는데 폰트를 바꿔서 하는 방법도 있었군요 👍

@choisohyun
Copy link
Member Author

넵!! 2번으로 가는것으로 하겠습니다! 리뷰 감사합니다~

@choisohyun choisohyun merged commit 2617af3 into chore/routes May 29, 2022
@mslee300 mslee300 added this to Done in Lonessum Jun 10, 2022
choisohyun added a commit that referenced this pull request Jun 12, 2022
 Feat: 소개팅 설문 페이지 #24 ~ #27
@choisohyun choisohyun deleted the feature/#19_surveyTemplate branch August 2, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design UI 디자인 변경, css Feat 새로운 기능 추가
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants