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

로그인 페이지 정적 UI 구현 #143

Merged
merged 2 commits into from
Jul 31, 2024
Merged

Conversation

cys4585
Copy link
Contributor

@cys4585 cys4585 commented Jul 30, 2024

PR의 목적이 무엇인가요?

로그인 페이지의 정적 UI 기능 구현

이슈 ID는 무엇인가요?

설명

결과물 UI

image

디자인 시안이 러프하게 작성되어있기 때문에, 일단은 스타일링은 크게 신경쓰지 않았고, 기본 구조를 만드는 것에 집중했어요. 아래 내용은 구현 과정의 사고를 작성해봤어요. 코드 리뷰에 도움이 되길 바랍니다 😄

  1. 레이아웃
    • CompleteLayout과 구조가 비슷해서 재사용할까 했지만, 우연의 일치로 같은 것이고 추후에 Complete이든 Login이든 어느 하나가 Layout이 바뀌게 되면 찢어져야 하기 때문에 재사용하지 않는다.
    • 소파, 치코, 수야가 함께 약속한 것: 지금 당장은 layout의 재사용을 고려하지 말고, 일단은 page 하나당 layout 하나 만들자. 추후에(level4?) 리팩토링을 하면서 layout을 합쳐야하는 것들을 합치자.
    • → 결론적으로 LoginLayout을 구현했다.
  2. 컴포넌트
    • 기존의 LabeldInput과 Button을 재사용했다.
    • 그 각각의 컴포넌트를 사용하는 LoginForm 컴포넌트를 만들었다.
  3. 라우팅 및 페이지
    • App 밑에서 router로 분기될 LoginPage를 만들고, 그 LoginPage 안에서 LoginLayout과 LoginForm을 사용했다.
    • 추후에 UI 관련 요구사항이 변경될 경우 해당 요구사항에 해당되는 변경만 특정 컴포넌트에서 대응하면 될 것이라 기대한다.

질문 혹은 공유 사항 (Optional)

@cys4585 cys4585 added FE 프론트엔드 관련 이슈입니다. 🌱 기능추가 feature (새로운 기능 구현) labels Jul 30, 2024
@cys4585 cys4585 self-assigned this Jul 30, 2024
Copy link
Contributor

@jaeml06 jaeml06 left a comment

Choose a reason for hiding this comment

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

넵 코드 확인했습니다. 현재 진행 상황으로는 기존 레이아웃을 재사용하고 있고 변경사항이 필요하지 않아 확인하고 approve 드립니다. 고생했어요. 수야

export const formContainerStyle = css`
display: flex;
flex-direction: column;
gap: 20px;
Copy link
Contributor

Choose a reason for hiding this comment

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

개인적으로는 아직 gap의 단위를 rem으로 해야할지 px로 해야할지 고민이 되네요. 수야의 의견은 어떤가요?

Copy link
Contributor

Choose a reason for hiding this comment

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

저 같은 경우는 글이 있는 경우 rem에 따라 간격이 늘어나는 것이 좋아보이기도하고, 그럴 필요까지 없는 것 같기도 하네요

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 rem으로 줬어용

Copy link
Contributor

@ss0526100 ss0526100 left a comment

Choose a reason for hiding this comment

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

잘읽어보았습니다 고생하셨어요~~

export const formContainerStyle = css`
display: flex;
flex-direction: column;
gap: 20px;
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 rem으로 줬어용

return <div css={S.layoutStyle}>{children}</div>;
}

LoginLayout.Header = LoginHeader;
Copy link
Contributor

Choose a reason for hiding this comment

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

TriSectionHeader로 통합해도 괜찮을 것 같아용~

@@ -22,6 +23,10 @@ const router = createBrowserRouter([
path: ROUTES.participationComplete,
element: <ParticipationCompletePage />,
},
{
Copy link
Contributor

Choose a reason for hiding this comment

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

이걸보니 토큰이 없을 때 다른 곳에 마음대로 접근하는 경우 로그인페이지로 강제 다이렉션 되어야 할 것 같다는 생각이 들어요

@cys4585 cys4585 merged commit 83e3967 into develop-frontend Jul 31, 2024
1 check passed
@cys4585 cys4585 deleted the feature/#137 branch July 31, 2024 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 관련 이슈입니다. 🌱 기능추가 feature (새로운 기능 구현)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants