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

[ jin-Pro ] LoginPage / RegisterPage / Button.tsx / Input.tsx #28

Merged
merged 15 commits into from
Nov 2, 2021

Conversation

jin-Pro
Copy link
Member

@jin-Pro jin-Pro commented Nov 1, 2021

  • LoginPage 구현
  • RegisterPage 구현
  • Button.tsx [ Atom ] 구현
  • Input.tsx [ Atom ] 구현

고민

1번

inline으로 css 를 넣어주기 위해
emotion/css 를 사용하여

const containerStyle = css``;

를 많이 사용하였습니다.

그러다보니 코드가 너무 두꺼워지는 것 같아서 불편합니다.

혹시 Style을 따로 담아두는 파일을 생성하여 BEM 구조 처럼 변수명을 설정해서 관리해주는게 좋을까요??

2번

기본적으로 Atomic 디자인은

Atom >> Molecules >> Organism >> Template >> Page
로 구성이 되어있는데

RegisterPage와 LoginPage같은 경우

Atom 들로 나열되어있는데 이것들을 재사용성이 없어도 가독성을 위해서 Molecules >> Organism >> Template 단계를 거치는게 나을까요??

굳이 3 단계를 거치지 않더라도 하나의 단계를 추가해주어서 가독성을 위해 간결하게 정리하는게 나을까요??

@jin-Pro
Copy link
Member Author

jin-Pro commented Nov 1, 2021

1번 질문에 대해서
inline style에 대해서 굳이 변수로 설정을 해야하는가 ??
아래 이미지 처럼 하는거에 대한 생각들은 어떠신가요?

스크린샷 2021-11-02 오전 2 08 12

@taehong0-0
Copy link
Member

1번 질문에 대해서 inline style에 대해서 굳이 변수로 설정을 해야하는가 ?? 아래 이미지 처럼 하는거에 대한 생각들은 어떠신가요?

스크린샷 2021-11-02 오전 2 08 12

저도 해당하는 방법을 찾아봤었는데 사진과 같이 간단한 코드에서는 오히려 알아보기도 쉽고 좋은데 여러줄이 되거나 하면 어떤 구조로 되어있는 지 알기 어려워 보여서 저는 변수를 통해 설정해주는 것이 좋다고 생각했습니다.

@taehong0-0
Copy link
Member

1번 질문에 대하여 BEM구조에 대해 찾아보았는데 저희가 사용하는 아토믹 디자인과 같이 사용하면 좋을 것 같다는 생각이 들었습니다.
저는 파일을 구분하는 것도 해당하는 파일이 어떤 역할을 하는지 명확하게 구분할 수 있다고 생각하여 좋은 것 같습니다.

2번에 대한 질문이 정확하게 이해가 되지 않습니다.

@Noelsky-code
Copy link
Member

1번 질문에 대해서 inline style에 대해서 굳이 변수로 설정을 해야하는가 ?? 아래 이미지 처럼 하는거에 대한 생각들은 어떠신가요?

스크린샷 2021-11-02 오전 2 08 12

=> 이렇게 짧다면 코드는 짧아지겠지만 가독성이 좋은지는 잘 모르겠습니당

@Noelsky-code
Copy link
Member

Noelsky-code commented Nov 2, 2021

  1. bem 구조를 이용하고 한곳에 css를 몰아준다면 css 재활용이 많이 일어날 수 있을 거 같지만 우리 프로젝트에서는 그렇게 재활용이 일어날 거 같지 않습니다.
    일단 개발을 하고 5주차 또는 6주차에 리팩토링할 수 있는 시간에 시도해보면 어떨 까 싶습니당. 재사용되는 class 정도만 global 정도에 놓으면 좋을 거 같아요

  2. 제 생각엔 3단계를 거치지 않고 각 단계를 거치지 않아도 충분히 가독성이 있어보입니다. 필요에 따라 정의하면 될 거 같다고 생각듭니다
    오히려 필요없는 중간단계를 거친다면 가독성은 더 떨어진다고 생각합니다

@ddaynew365
Copy link
Collaborator

1번 질문에 대해서 저도 2줄 이상의 css 코드는 변수로 행해주는 것이 좋다고 생각합니다.
2번 질문에 대해서 atomic을 template에 바로 달아준다는 의미면은 무의미한 단계를 지나기보다는 그게 더 보기 좋을 것 같습니다.


function App() {
return (
<>
<Global styles={reset} />
<LogInPage />
<Switch>
Copy link
Member

Choose a reason for hiding this comment

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

Global 안에 각 페이지가 있는게 더 자연스럽지 않나요?
상관없는 건가요>


{!code && (
<div css={SocialLoginButtonContainerStyle}>
<Button type="Long" color="#000000">
Copy link
Member

Choose a reason for hiding this comment

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

이 부분 다른 파일 (Molecules) 또는 Organism으로 분리하는게 좋을 거 같음


<div css={ButtonContainerStyle}>
<Link to="/register">
<Button type="Small">회원가입</Button>
Copy link
Member

Choose a reason for hiding this comment

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

이 부분도 분리하는게 어때보임??

<div css={IdContainerStyle}>
<Input placeholder="ID" autoComplete="off" />
<Button type="Small"> 중복 체크 </Button>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

[label - input] 부분이 많이 중복되는거 같은데 분리해서 재활용하는 것도 좋아보임

@jin-Pro jin-Pro merged commit 12741c1 into dev Nov 2, 2021
@jin-Pro jin-Pro deleted the Feat/RegisterPage branch August 8, 2022 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment