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.

Sign up for GitHub

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

Jump to bottom

[Style/#20] 즐겨찾기 컴포넌트 구현 #21

Merged
merged 19 commits into from
Nov 26, 2024
Merged

[Style/#20] 즐겨찾기 컴포넌트 구현 #21

merged 19 commits into from
Nov 26, 2024

Conversation

gwagjiug
Copy link
Member

@gwagjiug gwagjiug commented Nov 22, 2024 •
edited
Loading

📌 관련 이슈번호

🎟️ PR 유형

어떤 변경 사항이 있나요?

  • 새 기능 추가
  • 버그 수정
  • CSS 등 사용자 UI 디자인 변경
  • 리팩토링

✅ Key Changes

즐겨찾기 하트 컴포넌트 퍼블리싱 및 기능구현

  1. 작업 내용

📢 To Reviewers

  • 필터 버튼 cursor : pointer 속성 추가했습니다.
  • 즐겨찾기 버튼 애니메이션 기능 추가했습니다.
  • 일단 애니메이션 기능 구현 위주로 코드를 작성했고, 추후에 api 연동할 때에는 변동이 생길 것 같습니다

📸 스크린샷

20241125_185856.mp4

🔗 참고 자료

@gwagjiug gwagjiug self-assigned this Nov 22, 2024
@gwagjiug
Merge branch 'develop' of https://github.com/SOPT-all/35-COLLABORATIO…
dfe51ae 
…N-WEB-MCDONALD into style/#20/LikeButton
@gwagjiug
feat: 즐겨찾기 btn 추가 및 이벤트 설정
dd5327d
@gwagjiug
fix: build 에러 해결
473c919
@gwagjiug
Merge branch 'develop' of https://github.com/SOPT-all/35-COLLABORATIO…
aabc0c1 
…N-WEB-MCDONALD into style/#20/LikeButton
@gwagjiug
merge 후 충돌 해결
40d9200
@gwagjiug
Merge branch 'develop' of https://github.com/SOPT-all/35-COLLABORATIO…
8c74091 
…N-WEB-MCDONALD into style/#20/LikeButton
@gwagjiug
feat : 필터버튼 커서 포인터 추가
c3261d6
@gwagjiug
feat :즐겨찾기 버튼 애니메이션 추가
3024032
@gwagjiug
init : 즐겨 찾기 페이지 초기세팅
74c2413
@gwagjiug
style: 배너 퍼블리싱
bed68dd
@gwagjiug
style: products 섹션 업데이트
504077a
@gwagjiug
style: burger post 뷰 추가
f2b526d
@gwagjiug
fix: build 에러 해결
836e988
youtheyeon
youtheyeon approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@youtheyeon youtheyeon left a comment

Choose a reason for hiding this comment

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

좋습니다~!
그런데 버거 포스트 컴포넌트에 width: 100% 속성이 반영 안 되어 있는 것 같아서 추가해주시면 좋을 것 같습니다:)
그리고 즐겨찾기 버튼 눌렀을 때 효과를 setTimeout으로 구현하셨던데 css에 active 속성 사용하면 버튼을 누르는 순간부터 떼는 시점까지의 스타일을 적용시킬 수 있습니다! 그냥 이런 방식도 있다~ 하고 넘어가주세요. 고생하셨습니다!

src/components/BurgerPost/BugerPost.style.ts Outdated
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
Member Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋ ㅜㅜ 둘다 반영완료했습니다. actice 속성은 뭔가 디자이너 선생님이 디테일하게 조정을 원하실까봐 했는데 그런 방법도 있다니 다음에 반영해보겠습니다

imddoy
imddoy approved these changes Nov 25, 2024
View reviewed changes
Copy link
Collaborator

@imddoy imddoy left a comment

Choose a reason for hiding this comment

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

굿굿!!

src/components/BurgerPost/BurgerPost.tsx Outdated
@@ -17,6 +18,7 @@ const BurgerPost = ({ titleEn, titleKo }: BugerPostProps) => (
<div css={postStyleContainer}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기에 ThemeProvider를 넣어주신 이유가 무엇인가요??
App에 해둬서 지워두셔도 될 것 같습니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

wow 반영하겠습니다~ 처음에 시험해본다고 안지웠네요 꼼꼼이 리드

src/components/BurgerPost/BurgerPost.tsx Outdated
Comment on lines 2 to 6
import {
postStyleContainer,
titleSection,
imageSection,
} from './BugerPost.style';
Copy link
Collaborator

Choose a reason for hiding this comment

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

스타일이 많다면 전체를 import해서 접근하는타일이 많다면 전체를 import해서 접근하는 것도 좋을 것 같습니다 ㅎㅎ

Copy link
Member Author

Choose a reason for hiding this comment

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

이 부분 수정했습니다 ㅎㅎ

public/svgs/state=pressed.svg Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트가 원 색깔과, 하트 색깔말고는 다 같은데 하나의 컴포넌트로 2개의 컬러를 props로 관리하는것이 코드도 적어지고, 렌더링 측면에서도 좋을 것 같은데 어떻게 생각하시나요?

src/assets/svgs/likeButton/BtnHeart.tsx Outdated
Comment on lines 12 to 14
x={0.5}
fill="#fff"
fillOpacity={0.8}
Copy link
Collaborator

Choose a reason for hiding this comment

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

이 값이랑 아래에 path에 있는 fill을 props로 관리하면 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

반영완료입니다~

src/components/BurgerPost/BugerPost.style.ts Outdated
Comment on lines 13 to 15
background-color: ${theme.colors.gray000};
`;
export const imageSection = css`
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
Member Author

Choose a reason for hiding this comment

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

수정완료!!

src/components/BurgerPost/BugerPost.style.ts
@@ -13,6 +13,7 @@ export const postStyleContainer = (theme: Theme) => css`
background-color: ${theme.colors.gray000};
`;
export const imageSection = css`
position: relative;
display: flex;
height: 11rem;
justify-content: center;
Copy link
Collaborator

Choose a reason for hiding this comment

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

이전 코드라 코리가 안달리네요...ㅜㅜ
flex-shrink를 주신 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

피그마 상으로 0으로 되어있어서 넣긴했는데 어차피 뷰가 고정이라 필요없는 속성 같아용 다른 곳도 수정할게요~

@gwagjiug gwagjiug merged commit 0dbc1bb into develop Nov 26, 2024
@gwagjiug gwagjiug deleted the style/#20/LikeButton branch November 26, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youtheyeon youtheyeon youtheyeon approved these changes

@imddoy imddoy imddoy approved these changes

@bongtta bongtta Awaiting requested review from bongtta

Assignees

@gwagjiug gwagjiug

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ Style ] 즐겨찾기 버튼 컴포넌트 퍼블리싱
3 participants
@gwagjiug @imddoy @youtheyeon