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

[고급 레이아웃 컴포넌트] 윤생(이윤성) 미션 제출합니다. #69

Merged
merged 14 commits into from
Oct 3, 2023

Conversation

2yunseong
Copy link

@2yunseong 2yunseong commented Sep 26, 2023

🎯 2단계. 고급 레이아웃 컴포넌트

안녕하세요 가람! 2주차도 잘 부탁드립니다!
스토리북 링크

공통

  • 의미있는 커밋 메시지 작성
  • PR에 관련 없는 변경 사항이 포함 유무 확인

컴포넌트 요구사항

  • 선택한 컴포넌트의 기능 구현
  • npm 배포
  • README.md에 코드 저자로서 특히 고민한 부분과 의도 설명
  • 테스트 코드를 작성하였나요? (선택 사항)

📌 구현한 내용 설명

기본 요구사항에 맞게 구현해보았어요~!

📸 스크린샷 (선택 사항)

(필요하다면 구현한 내용을 보여주는 스크린샷 첨부.)

🔗 참고 링크 (선택 사항)

(유용했던 참고 자료나 레퍼런스가 있다면 링크 첨부)

@2yunseong 2yunseong self-assigned this Sep 26, 2023
@2yunseong 2yunseong requested a review from guridaek September 26, 2023 05:20
Copy link

@guridaek guridaek left a comment

Choose a reason for hiding this comment

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

안녕하세요 윤생!
전반적으로 깔끔하게 구현해주신 것 같아요.
리드미도 어렵지 않게 작성해주셔서 쉽게 사용했습니다. 👍
코드 단계에서 피드백 드릴 건 특별히 없어보이네요 ㅎㅎ
아래의 몇가지 사항만 확인해주시면 될 것 같습니다.

+ 업뎃된 스토리북 링크도 추가해주세요!


탭이 많아지면 화면이 깨지는 것 같아요.
탭 제목의 줄을 바꿔주거나, overflow속성을 추가하면 어떨까요!?

image

콘솔창에 에러가 표시되는 부분도 확인해주시면 좋을 것 같아요!

image

README.md Outdated
Comment on lines 144 to 146
`tabs: string[]` : 요소를 선택해 콘텐츠를 이동할 수 있는 Tab Header에 표시될 문자열 배열 값입니다.

`children: ReactElement` : 선택된 tabs 요소에 따라 띄울 콘텐츠를 보여줍니다. 보여주는 값은 선택된 tabs의 인덱스에 의존합니다.

Choose a reason for hiding this comment

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

사용해보니 bordercolor도 필수인 것 같아서, 속성에 추가해주면 좋을 것 같아요!

@@ -0,0 +1 @@
* text=auto

Choose a reason for hiding this comment

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

요건 어떤 설정인가요??

Copy link
Author

Choose a reason for hiding this comment

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

제가 주말에 윈도우에서 작업을 한번 했는데, CRLF로 저장이 되어서 그걸 막으려고 지정해준 옵션입니다!


`tabs: string[]` : 요소를 선택해 콘텐츠를 이동할 수 있는 Tab Header에 표시될 문자열 배열 값입니다.

`children: ReactElement` : 선택된 tabs 요소에 따라 띄울 콘텐츠를 보여줍니다. 보여주는 값은 선택된 tabs의 인덱스에 의존합니다.

Choose a reason for hiding this comment

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

children이 props에선 ReactElement[]이니깐, 여기도 배열로 알려주는 게 좋을 것 같아요!

@@ -0,0 +1,18 @@
import type { Meta, StoryObj } from '@storybook/react';

Choose a reason for hiding this comment

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

작성하신 스토리를 보고싶은데,
어디서 확인할 수 있을까요?
링크가 없는 것 같아서..

Copy link
Author

@2yunseong 2yunseong Sep 30, 2023

Choose a reason for hiding this comment

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

링크 PR 메세지에 다시 남겨두었습니다~! 늦게 남겨두어서 죄송해요 🥹

@2yunseong
Copy link
Author

가람 안녕하세요! 즐거운 명절 보내시고 계신가요!

코멘트와, overflow, 그리고 오류 나는 것 까지 수정해보았습니다!

다시 리뷰 요청드립니다~! 남은 연휴 잘 보내세요!

@2yunseong 2yunseong requested a review from guridaek September 30, 2023 16:28
Copy link

@guridaek guridaek left a comment

Choose a reason for hiding this comment

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

수정하신 부분 및 스토리북 페이지 확인했습니다.
머지할게요 !!
이번 미션 수고하셨습니다 😁

@guridaek guridaek merged commit c60f6a6 into woowacourse:2yunseong Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants