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: 플로팅 버튼 구현 #52

Merged
merged 14 commits into from
Mar 20, 2024
Merged

feat: 플로팅 버튼 구현 #52

merged 14 commits into from
Mar 20, 2024

Conversation

hae-on
Copy link
Contributor

@hae-on hae-on commented Mar 19, 2024

Issue

✨ 구현한 기능

  • 기존 스크롤 버튼의 디자인을 수정하였습니다.
  • 글쓰기 버튼을 생성했습니다.
스크린샷 2024-03-19 오후 1 53 34 스크린샷 2024-03-19 오후 1 46 24 스크린샷 2024-03-19 오후 1 45 21

📢 논의하고 싶은 내용

WriteButton의 경우 디자이너분께 댓글 남겨놓긴 했는데
꿀조합에서만 쓰이는건지 아니면 다른 화면에도 쓰이는건지 모르겠어요.
그래서 일단은 다 쓰인다고 생각하고 이름 이렇게 지었는데, 만약에 꿀조합에서만 쓰이면 이름 수정해볼게요!

그리고 WriteButton의 스타일링이 겁나 지저분해졌어요.
flexCenter라는 스타일 변수 만들어서 노력해보았지만...여전히 더럽습니다.
이거 더 줄이거나 덜어낼 스타일 알고 계신 분은 댓글 남겨주세요오....

x 버튼 클릭하면 말풍선 닫히는거 상태 이름 괜찮나요? Open으로 하는게 덜 헷갈리나요?

const [bubbleClose, setBubbleClose] = useState(false);

말풍선 보이는게 !bubbleClose && !member 이 상태라서 봤을 때 이해가 안갈까 싶어 물어봅니다!

스토리북에서 로그인 된 상태와 안 된 상태 나누어서 스토리 짤 수 없나요?
이거 찾는다고 오래 걸렸습니다 ㅠ
결국 방법 못 찾아서 수동으로 true, false 주면서 스타일링 했어요 휴,,,
그래서 스토리북 배포 주소 들어가도 지금 우리가 무조건 로그인 상태로 되어 있어서
노란색 버튼밖에 볼 수 없습니당,,,

🎸 기타

  • 특이 사항이 있으면 작성합니다.

⏰ 일정

  • 추정 시간 : 1시간
  • 걸린 시간 : 3시간

Copy link

🔗 배포된 storybook 주소: https://65f015a4aed45406385006ee-urmhdxtkis.chromatic.com/

Copy link
Contributor

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

해온 수고했어요~
스토리북은 그러게요... 목핸들러를 수정하면 또 에러 터질거 같은데 어렵네요
스타일은 괜찮습니다 굿굿
코멘트 확인해주세요

아 그리고 lint fix 한번 해주세요~

Comment on lines 4 to 7
position: 'fixed',
right: 20,
width: 40,
height: 40,
Copy link
Contributor

Choose a reason for hiding this comment

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

fixed는 뷰포트에 따라서
sticky를 사용하는건 어떤가요?
화면폭에 상관없이 부모요소(main)에 따르게 됩니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이거 sticky로 바꿔봤는데, right 속성이 아예 작동 안하더라구요? left는 잘 됩니다!
fixed로 할 땐 잘 되고, sticky로 할 땐 right만 적용 안하는 이유...를...아시나요....
상위 부모 속성 하나씩 다 빼봤는데 똑같습니다 ㅠ

Copy link
Contributor Author

@hae-on hae-on Mar 19, 2024

Choose a reason for hiding this comment

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

left 퍼센트로 넣으면 오른쪽에 붙긴하는데,,, 우짜죠?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

놀라운 사실 발견. sticky에서 right는 float:right를 넣어야 작동한다!

https://stackoverflow.com/questions/72126594/aligning-position-sticky-at-the-bottom-right-is-not-working

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2024-03-20.1.28.09.mov

아놔 얘 왜 일정 높이 이상 스크롤 내리면 같이 딸려올라가서 사라짐??

Copy link
Contributor

Choose a reason for hiding this comment

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

부모요소에서 overflow 속성 있으면 적용 되지 않는다고 하네요. 이 부분 한번 확인해보세요

https://stackoverflow.com/questions/72126594/aligning-position-sticky-at-the-bottom-right-is-not-working

대박..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

overflow 때문 맞는거 같아요.
ProductDetailPage에서는 스크롤 버튼의 부모가 이 페이지인데 이 페이지 자체에 overflow 걸려서 그렇더라구요?

이게 레이아웃 PR pull 땡기니까 다른 페이지에서 오류는 사라졌는데
두 버튼의 사용처인 꿀조합 페이지는 지금 로컬에서 그냥 마우스 스크롤 내리면 일정 구간에서 안 움직이는 렉이 있어요.

그래서 저 플로팅 버튼들 일단은 그대로 두고, 나중에 페이지 완성되면 추후 위치 수정할게요! 일단 sticky로 고쳤습니다~~

Comment on lines 12 to 14
position: 'fixed',
right: 20,
gap: 6,
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도 같은 의미로.!

Comment on lines +3 to +7
const flexCenter = style({
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
});
Copy link
Contributor

Choose a reason for hiding this comment

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

묶어주는거 좋은데요?

},
]);

export const speechBubble = style([
Copy link
Contributor

Choose a reason for hiding this comment

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

bubble만 써도 괜찮겠는데요?

Copy link
Member

Choose a reason for hiding this comment

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

잔소린 버블버블버블~

<>
{/* 클릭 시 이동 (dialog 사용할 지, 페이지 이동할 지 미정) */}
<div className={container}>
{!bubbleClose && !member && (
Copy link
Contributor

Choose a reason for hiding this comment

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

조건이 많다면 위에 따로 변수로 만들어도 좋아보이네요
const isShow = !bubbleClose && !member

Comment on lines 51 to 53
borderStyle: 'solid',
borderWidth: '6px 0 6px 10px',
borderColor: 'transparent transparent transparent #FFFFFF',
Copy link
Contributor

Choose a reason for hiding this comment

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

오..이렇게도 쓸 수 있군요
옆에 말풍선 삼각형인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

예압 저렇게 삼각형을 만들 수 있더라구요? 화면 확대하면서 보면 일정 구간에서 실금이 보입니다...


const WriteButton = () => {
const { data: member } = useMemberQuery();
const [bubbleClose, setBubbleClose] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

저는 open 한표
불리언이니까 is- ..?

Copy link
Member

Choose a reason for hiding this comment

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

isBubbleOpen?

Copy link
Member

@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.

고생했어요 해옹~~~

@@ -245,6 +245,15 @@
<symbol id="trashcan" viewBox="0 0 24 24">
<path d="M6 19c0 1.1.9 2 2 2h8c1.1 0 2-.9 2-2V7H6v12zM19 4h-3.5l-1-1h-5l-1 1H5v2h14V4z" />
</symbol>
<symbol id="arrowUp" viewBox="0 0 11 18">
<path
fill="none"
Copy link
Member

Choose a reason for hiding this comment

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

근데 이거 fill none하면 스토리북에서 보이나요??

Copy link
Contributor Author

@hae-on hae-on Mar 19, 2024

Choose a reason for hiding this comment

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

아뉴...지금 몇개는 스토리북에서 투명 아이콘임니당....
근데 fill none을 해야했던 이유는

스크린샷 2024-03-20 오전 12 39 02

요기 보이시나요? 원래 화살표 모양인데 이상한 삼각형이 같이 나옵니다...
스토리북 아이콘에서만 보면 stroke가 적용이 안돼서 fill의 저 삼각형 대가리만 나옵니다,,,하하

Copy link
Contributor

Choose a reason for hiding this comment

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

이거 SVGIcon에서 fill 기본값으로 색상 적용되어 있는데, 사용하는 곳에서 props로 fill='none' 주면 됩니다

Copy link
Contributor

Choose a reason for hiding this comment

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

sprite에 있는 아이콘에 fill, stroke 속성 다 지우면 되어요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러면 스토리북 SVGIcon 컴포넌트에서는 이상하게 뜨는게 정상이 맞는거죠???
사용처에서 fill none으로 주니까 스크롤 버튼 컴포넌트는 스토리북에서 정상적으로 나옵니다!

Copy link
Member

Choose a reason for hiding this comment

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

sprite에서 지우면 어케되나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

지금 sprite에서 지운 상태에요!

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

import ScrollButton from './ScrollButton';
import { useRef } from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

뭔가 이거 린트 안된거 같아여


const WriteButton = () => {
const { data: member } = useMemberQuery();
const [bubbleClose, setBubbleClose] = useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

isBubbleOpen?

</div>
)}
<button className={button} disabled={!member}>
<SvgIcon variant="pencil2" fill="none" stroke="white" width={17} height={17} />
Copy link
Member

Choose a reason for hiding this comment

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

이거 fill optional이라서 안적어도 되지 않나용?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스크린샷 2024-03-20 오전 12 31 42

안적으면 요렇게 됩니당!

},
]);

export const speechBubble = style([
Copy link
Member

Choose a reason for hiding this comment

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

잔소린 버블버블버블~

@hae-on
Copy link
Contributor Author

hae-on commented Mar 20, 2024

@xodms0309 @Leejin-Yang 여러분 리뷰 다 고쳤어용 어푸르브 주세요

Copy link
Contributor

@Leejin-Yang Leejin-Yang left a comment

Choose a reason for hiding this comment

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

하나만 확인 부탁..!
그리고 지금 크로마틱 배포 터지는거 classnames 의존성 없는데 배포하려 해서 나는거라 feat/v2 머지 한번 해주세요
LGTM~

</button>
</div>
)}
<button className={button} disabled={false}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<button className={button} disabled={false}>
<button type='button' className={button} disabled={false}>

타입 추가..! 근데 disabled false 맞나요?

Copy link
Contributor

Choose a reason for hiding this comment

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

button에 다 추가해주세요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아...스토리북에서 테스트 한다고 하다가 고치는거 깜빡했다...감삼다

@hae-on
Copy link
Contributor Author

hae-on commented Mar 20, 2024

저거 안그래도 pull 땡겨서 머지했는데도 똑같더라구요.
저 yarn도 했는데 또 x 뜨네요 ㅠ
브랜치 병합 커밋 이후부터 x 뜨는데...?

다시 feat/v2가서 pull 땡겨도 Already up to date 뜨고,
작업 브랜치가서 땡겨도 똑같습니다 ㅠㅠㅠㅠㅠㅠ

@Leejin-Yang
Copy link
Contributor

@hae-on 엥 머지 충돌나는거 없음 괜찮겠죠 수고했어요

@hae-on
Copy link
Contributor Author

hae-on commented Mar 20, 2024

아오 yarn 다시 깔아도 똑같네요. 일단 머지하고 문제 생기면 해결할게요!

@hae-on hae-on merged commit db1c534 into feat/v2 Mar 20, 2024
1 check failed
@hae-on hae-on deleted the feat/issue-48 branch March 20, 2024 15:10
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.

플로팅 아이콘 구현 및 개선
3 participants