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

6회차 과제 - 구준혁 #5

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

gugitgugit
Copy link

@gugitgugit gugitgugit commented May 8, 2024

1. 구조 설계

App
  Header
  Video
    Menu_li
    VideoBox
  Short
    ShortBox

2. 실행 결과 화면

결과 화면

3. 아쉬웠던 점

  • 구조를 좀 더 깔끔하게 나눌 수 있었을 것 같다.

4. 새롭게 알게된 점

  • React에서 Fontawesome 사용법
  • styled-components에서 hover 사용법
  • map으로 텍스트를 받을 때 줄바꿈 입력법

@immms immms mentioned this pull request May 8, 2024
Copy link

@kyw2790 kyw2790 left a comment

Choose a reason for hiding this comment

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

코드가 전반적으로 너무깔끔하고 잘짜여있어서 코드 리뷰를 하면서 더 배워가는 것같습니다!! 코드 짜느라 수고하셨습니당!!

import ShortBox from './ShortBox';

const Short = () => {
const shortList = [
Copy link

Choose a reason for hiding this comment

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

배열을 통해서 id 필드 추가하니 다양한 값을 넣을 수 있고 코드도 깔끔하게 보이네요!!

</Box>
);
};
const Box = styled.div`
Copy link

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.

styled-components 를 사용해서 구현해보았습니다! className을 통한 다른 css파일에서의 접근보다는 더 직관적인것 같아 괜찮은것 같습니다!

Copy link
Author

@gugitgugit gugitgugit left a comment

Choose a reason for hiding this comment

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

좋은 리뷰 감사합니다!

@gugitgugit
Copy link
Author

@YoonKeumJae 코드 리뷰 완료했습니다!

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