-
Notifications
You must be signed in to change notification settings - Fork 57
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
[김윤미] week13 #403
The head ref may contain hidden characters: "part3-\uAE40\uC724\uBBF8-week13"
[김윤미] week13 #403
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
css를 초기화할 때 layer를 사용하면 혹시나 생길 수 있는 css 적용 우선순위를 해결하는 데 도움이 됩니다.
읽어볼 만한 글을 아래에 첨부합니다.
https://medium.com/appwrite-io/css-layers-for-css-resets-f60f270aa1cd
https://developer.mozilla.org/en-US/docs/Web/CSS/@layer
layer가 필요한 경우를 겪어보셨는지는 모르겠지만 혹시 styled-components와 styled-reset를 함께 사용했을 때 일부 css가 작성한 css보다 styled-reset이 우선 적용되는 문제가 있습니다.
layer는 어디에 저장해두셨다가 해당되는 이슈를 겪으셨을 때 이런 게 있구나 하고 찾아서 적용해 보시길 바라겠습니다.
@@ -0,0 +1,31 @@ | |||
import { ReactNode } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReactNode와 MouseEvent는 react로부터 제공되는 타입들이기 때문에 아래와 같이 작성되어야 합니다.
import { ReactNode, MouseEvent } from 'react';
interface ButtonProps { | ||
children: ReactNode; | ||
className?: string; | ||
variant: 'primary'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
primary 밖에 없는 버튼이라면 variant prop을 사용하지 않는 게 좋았을 것 같습니다.
항상 정해진 값으로 지정되어 있다면 props로 값을 내려줄 필요는 없습니다.
해당 variant는 삭제해 주는 게 좋겠습니다.
children, | ||
className = '', | ||
variant = 'primary', | ||
onClick, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onClick과 children은 구조 분해하지 않으면 props와 같이 button 요소에 넘어가게 됩니다.
불필요한 구조 분해를 생략하는 게 어떨까요?
import { MouseEvent } from 'react'; | ||
import styles from '@/components/Button.module.scss'; | ||
import classNames from 'classnames/bind'; | ||
interface ButtonProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ButtonProps는 최종적으로 아래와 같이 수정되면 좋을것 같습니다.
type ButtonSize = 'large' | 'small';
interface ButtonProps extends React.ComponentProps<'button'> {
size: ButtonSize;
}
위와 같이 수정한 이유는 아래와 같습니다.
- 추후 Button에 넘어오는 size를 외부에서 상태로 관리할 경우 해당 버튼 타입을 가져다 사용하기 위함입니다.
React.ComponentProps<'button'>
를 확장하도록 함으로써 ButtonProps가 기본 HTML button 요소의 모든 프로퍼티를 상속받도록 하였습니다.
className={buttonLinkStyles['button-primary']} | ||
<Button | ||
className={styles.button} | ||
variant={'primary'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prop으로 넘기는 값이 문자열인 경우 괄호를 생략해서 variant="primary"
와 같이 작성하는 게 좋습니다.
그 이유는 아래와 같습니다.
- JSX에서 props에 괄호를 사용하는 것은 표현식을 평가하기 위함입니다.
변수나 함수와 같은 값을 넘길 때 사용됩니다. - 문자열과 같은 리터럴 값은 표현식으로 표현하지 않고 직접 문자열로 넘겨주게 됩니다.
eslint에서 다음의 룰을 사용하여 괄호 안에 담긴 문자열을 괄호가 없도록 수정할 수 있습니다.
해당 룰을 사용하여 props에 괄호를 사용하는 부분의 일관성을 유지할 수 있게 됩니다.
react/jsx-curly-brace-presence
제가 지난주에 classname를 추천해 드렸었나 보군요 잘 사용하고 계신 것 같아 다행입니다 ㅎㅎ |
요구사항
기본
12주차
13주차
심화
주요 변경사항
스크린샷
멘토에게