-
Notifications
You must be signed in to change notification settings - Fork 16
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
[정태원] 기능구현 챌린지 #6
base: main
Are you sure you want to change the base?
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.
공통 컴포넌트와 페이지에서만 사용하는 컴포넌트 분리를 잘 해주셔서 구조 볼 때도 편하고, 코드가 깔끔하게 보이는 것 같아요!
수고하셨습니당! 😁
<button | ||
type={type} | ||
onClick={onClick} | ||
className={`${customStyle ? customStyle : baseStyle} ${buttonStyle}`} |
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 클래스 이름을 조건부로 적용할 수 있는 라이브러리 중에서 clsx
가 있는데,
이게 npm trends에서 보면 classnames
과 같은 유사한 라이브러리 중에서도 상위권에 속해서 말씀드려용!
clsx
는 다양한 조건부 연산자를 지원해서 잘 사용한다면 유용하게 사용할 수도 있을 거 같아요!
제가 사용해봤을 때도 사용 안했을 때 보다 더 가독성이 좋아지는 것 같기도 해요 ㅎㅎ 😄
import clsx from 'clsx';
<button
className={clsx(customStyle ? customStyle : baseStyle, buttonStyle)}
>
// ...
</button>
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.
확실히 코드가 더 깔끔해지는 것 같네요!
좋은 정보 감사합니다ㅎㅎ
<button | ||
onClick={onClick} | ||
aria-pressed={checked} | ||
className="p-2 bg-white border-none cursor-pointer focus:outline-none" | ||
> |
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.
aria-pressed
속성을 사용하여 웹 접근성을 고려한 점이 정말 좋은 것 같아요👍
const fontSize = "text-sm"; | ||
const height = "h-6"; | ||
const chipClass = `flex p-4 items-center bg-lightGray justify-center text-black rounded-full ${fontSize} ${height}`; |
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.
Tailwind CSS 클래스를 변수화해서 사용하는 부분이 인상적이네요.
이렇게 하면 코드의 가독성이 높아지고 유지 보수도 용이할 것 같아요!!
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.
의견 남겨주셔서 감사합니다~~🙌
필수 구현 사항
https://clinicaltrialskorea.com/studies/{검색어ID}
링크로 이동console.info("calling api")
출력을 통해 콘솔창에서 API 호출 횟수 확인이 가능하도록 설정추가 구현 사항
선택 구현 사항