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

[Feat/#47] Main 페이지 캐러셀 구현 #51

Merged
merged 25 commits into from
Jan 11, 2024
Merged

Conversation

namdaeun
Copy link
Member

@namdaeun namdaeun commented Jan 10, 2024

✨ 해당 이슈 번호 ✨

closed #47

todo

  • 캐러셀 이미지 넣기
  • 경우의 수에 따라 이미지 크기 조절
  • 마지막 슬라이드 더 궁금한 부분
  • 버튼과 상단 타이틀 추가

📌 내가 알게 된 부분

📌 질문할 부분

📌스크린샷(선택)

스크린샷 2024-01-11 오후 5 54 00
2024-01-11.8.38.34.mov
2024-01-11.8.32.38.mov

@namdaeun namdaeun added ✨ Feature 새로운 기능 추가 (새로운 구현) 다은 labels Jan 10, 2024
@namdaeun namdaeun self-assigned this Jan 10, 2024
Copy link
Contributor

@se0jinYoon se0jinYoon left a comment

Choose a reason for hiding this comment

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

LGTM ~
잘했다 ~

Comment on lines +52 to +58
isImage && isLast
? '47.8rem'
: isImage && !isLast
? '59.8rem'
: !isImage && isLast
? '68.2rem'
: '85.8rem'};
Copy link
Contributor

Choose a reason for hiding this comment

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

p5) 헉 너무 야무지게 잘했네잉 ~

${({ theme }) => theme.fonts.body3};
`;

const Image = styled.img<{ isLast: boolean }>`
Copy link
Contributor

Choose a reason for hiding this comment

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

p5 )어이구 타스도 잘 쓰네 ..

Copy link
Member

@ljh0608 ljh0608 left a comment

Choose a reason for hiding this comment

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

LGTM


export default Carousel;

const CarouselContentWithSummaryWrapper = styled.div``;
Copy link
Member

Choose a reason for hiding this comment

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

[ P3 ] 이렇게 돔을 하나더 추가할 필요 없이
<></> fragment 사용하면 좋아요 이건 dom에 추가안돼서 지금처럼 스타일 없는 div를 추가할 필요 없습니다!
https://react.dev/reference/react/Fragment

Copy link
Member Author

@namdaeun namdaeun Jan 13, 2024

Choose a reason for hiding this comment

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

넵 참고하겠습니다!!!

{subtext}
</SubText>
</TextContainer>
{image && <Image src={image} isLast={isLast} alt="group-content-image" />}
Copy link
Member

Choose a reason for hiding this comment

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

[P5] alt 적용해서 시맨틱하게 코드짜는거 배워갑니다~

@github-actions github-actions bot added the size/m size/m label Jan 11, 2024
@ljh0608 ljh0608 merged commit 3b358ea into develop Jan 11, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 새로운 기능 추가 (새로운 구현) size/m size/m 다은
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ Feat ] Carousel 이미지 들어간 경우도 따지기
3 participants