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

[FE] feat: 꿀조합 전체 목록 페이지 구현(마크업) #403

Merged
merged 17 commits into from
Aug 13, 2023

Conversation

Leejin-Yang
Copy link
Collaborator

@Leejin-Yang Leejin-Yang commented Aug 11, 2023

Issue

✨ 구현한 기능

  • API 연결 전까지 구현했습니다.
  • 꿀조합 전체 목록 페이지 구현
  • 스크롤 버튼 스타일 수정

📢 논의하고 싶은 내용

  • 꿀조합 작성하기 버튼의 위치가 애매해요... 어디에 둬야할지 추천해주세요. 저의 최선입니다.!

🎸 기타

스크린샷 2023-08-11 오후 7 26 51

⏰ 일정

  • 추정 시간 : 4시간
  • 걸린 시간 : 6시간

@github-actions
Copy link

github-actions bot commented Aug 11, 2023

Unit Test Results

2 tests   2 ✔️  7s ⏱️
1 suites  0 💤
1 files    0

Results for commit b53caeb.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hae-on hae-on left a comment

Choose a reason for hiding this comment

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

굿~ 수고했습니다! 스크롤 버튼 위치는 저도 모르겠네염 일단 저렇게 두고 다른 사이트 레퍼런스 찾으면 바꾸는 것도 나쁘지 않아보입니다 근데 컴포넌트 옆 padding이 원래 저 정도밖에 없었나요?? 왤캐 좁아 보이지,,,?

return (
<RecipeItemContainer>
<ImageWrapper>
<RecipeImage src={image} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

alt 추가해주세요~

<RecipeItemContainer>
<ImageWrapper>
<RecipeImage src={image} />
<ProfileImage src={author.profileImage} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도~

<Text color={theme.textColors.sub}>
{author.nickname} 님 | {getFormattedDate(createdAt)}
</Text>
<Text size="xl" weight="bold">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Text 컴포넌트가 여러개 있어서 제목을 찾기 어렵네요 이런데에 Heading Tag를 써도 될까요? 흠 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

상품 이름을 h3로 수정했습니다

{title}
</Text>
{/*TODO: 임시 데이터, API 연동 후 수정*/}
<Text color={theme.textColors.info}>#불닭볶음면 #옥수수콘 #치즈...</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 ... 처리 해주십쇼~~ 원래 디자인보다 글자 길이 더 길게 가도 될 듯??

Copy link
Collaborator

Choose a reason for hiding this comment

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

목록 조회에 사용된 상품도 받아와야겠네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

백엔드에 이야기했습니다
이름 임의로 작성해서 필드 만들고 작성해볼게요!

</Title>
<LinkWrapper>
<Link as={RouterLink} to="/search">
<SvgIcon variant="search" width={24} height={24} />
Copy link
Collaborator

Choose a reason for hiding this comment

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

svgicon default width height가 24입니다!

Copy link
Collaborator

@xodms0309 xodms0309 left a comment

Choose a reason for hiding this comment

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

수고하셨어요~~ 코멘트 확인해주세요

@@ -18,6 +18,7 @@ const AuthLayoutContainer = styled.div`
`;

const MainWrapper = styled.main`
position: relative;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 다 position:relative 준 이유가 스크롤 버튼 때문인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

검색 버튼 두려고 적용했습니다

Copy link
Collaborator

Choose a reason for hiding this comment

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

검색버튼은 Title 컴포넌트에 들어가는게 아닌가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헤딩을 박스안에 두지 않고 따로 해보고 싶었습니다. (타이틀 목적에 맞게 텍스트만 있게 하도록)
타이틀 안으로 옮길까요?? 타미는 어느것이 좋아 보이나요?

페이지 자식으로 absolute가 존재하면 기존에는 전체 화면 기준으로 위치를 잡더라구요.!
혹시 몰라서 main 기준으로 위치를 잡기 위해 모든 레이아웃에 적용했습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 postion:relative로 전체를 감싸는게 맞나 싶어서 여쭤봤습니다. 일단 알겠습니다~

{title}
</Text>
{/*TODO: 임시 데이터, API 연동 후 수정*/}
<Text color={theme.textColors.info}>#불닭볶음면 #옥수수콘 #치즈...</Text>
Copy link
Collaborator

Choose a reason for hiding this comment

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

목록 조회에 사용된 상품도 받아와야겠네요

Comment on lines 86 to 88
display: flex;
align-items: center;
gap: 4px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

display 속성이 더 위입니다~

Comment on lines 63 to 64
bottom: -20px;
right: 16px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

top right bottom left

Comment on lines 73 to 76
position: relative;
display: flex;
flex-direction: column;
justify-content: space-between;
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 순서~

export default RecipeList;

const RecipeListContainer = styled.ul`
& > li + li {
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 뭐에요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ul 안에 li 두번째 부터 스타일 적용합니다

🍯 꿀조합
</Title>
<LinkWrapper>
<Link as={RouterLink} to="/search">
Copy link
Collaborator

Choose a reason for hiding this comment

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

상수화~~

Comment on lines 49 to 50
color={!member ? 'gray3' : 'primary'}
textColor={!member ? 'white' : 'default'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

근데 궁금한데 member ? 'primary' : 'gray3' 이렇게 쓰지 않은 이유가 뭔가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정했습니다!

font-size: 24px;
`;

const LinkWrapper = styled.div`
Copy link
Collaborator

Choose a reason for hiding this comment

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

이거 styled(Link) 로 하면 안되나요?

@Leejin-Yang
Copy link
Collaborator Author

@hae-on @xodms0309

리뷰 반영했습니다!
확인해주세요 😊

Copy link
Collaborator

@hae-on hae-on left a comment

Choose a reason for hiding this comment

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

수고했슴당 👍

@Leejin-Yang Leejin-Yang merged commit 34d9c90 into develop Aug 13, 2023
1 check passed
@Leejin-Yang Leejin-Yang deleted the feat/issue-393 branch August 13, 2023 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FE] 꿀조합 전체 조회 페이지 구현
3 participants