-
Notifications
You must be signed in to change notification settings - Fork 2
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] ToolTip 컴포넌트 구현 #285
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생많으셨슴다 !! 언어에 따른 툴팁 이슈만 해결해주심 될 것 같아요 LGTM 🚀
export const arrowStyle = css({ | ||
position: 'absolute', | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
툴팁 내용이 많아지는 경우 svg크기가 줄어들더라고요 !
flexShrink: 0
으로 해결할 수 있을 것 같은데 한 번 수정해보시면 좋을 것 같아요
margin?: number; | ||
} | ||
|
||
const ToolTip = ({ position = 'right', message, margin = 0, children }: ToolTipProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...props
추가해 줍시다 ~!
export const containerStyle = css({ | ||
position: 'relative', | ||
border: '1px solid', | ||
'&:hover': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
필수는 아니고 제 욕심 ..? 인데
transition 넣으면 더 부드럽게 뜰 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 아마 디자이너분들과 얘기해봐야겠네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨고, 수정하실거 수정하시면 다시한번 보겠습니답 !
export const containerStyle = css({ | ||
position: 'relative', | ||
border: '1px solid', | ||
'&:hover': { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 아마 디자이너분들과 얘기해봐야겠네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아티클을 보니 고민의 흔적이 보입니다
까다로워 보이는 구조인데 넘 수고 많으셨어요 !! 👍
…iki/TIKI_CLIENT into feature/tiki/#276-tooltip
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다 ! 뀨 홍 !
|
||
const ToolTip = ({ position = 'right', message, margin = 0, children, ...props }: ToolTipProps) => { | ||
return ( | ||
<div css={containerStyle}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria role
을 통해 부가적인 설명해주어도 좋을 것 같아요 ! role=tooltip
도 존재하니까 이거 사용하면 될 것 같습니다. 그리고 children
요소에게 cloneElement
하여서 aria-describeby: message
부여하고 툴팁의 div
요소에는 id=message
추가해주면 아주 좋을 것 같아요 !
요고 참고해보시면 좋을 것 같습니다.
export interface ToolTipProps extends HTMLAttributes<HTMLSpanElement> { | ||
position?: 'top' | 'right' | 'bottom' | 'left'; | ||
message: string; | ||
margin?: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마진보다는 gap
??이 더 와닿는 것 같은데 다른 분들 생각은 어떠신지
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동의합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이슈 해결하느라 고생 많으셨습니다 !
|
||
return ( | ||
<div | ||
id={message} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria-describeby
를 id
대신 넣어주어야할것 같아요 !! id
는 tooltip
인 span
에다가 넣어주어야할 것 같습니다 ㅎㅎ
tabIndex
로 컨테이너 요소가 포커스될 때 tooltip
나오게 하는거 너무 좋아요 ! 다만 현재 tooltip
에 컨테이너 요소와 함께 outline
이 생겨서, 컨테이너 요소만 아웃라인 생기게 해주시면 좋을 것 같슴다 !
해당 이슈 번호
closed #276
체크리스트
📌 내가 알게 된 부분
즉, left 이동을 할때의 기준은 부모의 왼쪽 테두리이고, bottom 이동을 할때는 부모의 아래 테두리가 기준이 된다.
즉, left: 100% 는 왼쪽 border를 기준으로 부모의 width 만큼 이동한거다
💎 PR Point
접근성 챙겨주는 리펙토링 했습니다!
tab 키로 tooltip 접근, escape 키로 tooltip 없앨 수 있습니다. 스토리북에서 체험해보세요!
해당 내용 아티클에서 자세하게 확인 가능합니다.
간단한 아티클로 대체하겠습니다.
🍀사용법
호버시 ToolTip을 띄어줄 컴포넌트를 ToolTip 공컴으로 감싸주고 적절한 prop을 넘겨주면 된다.
position
: ToolTip을 띄워줄 방향 설정 left, right, bottom, top 네방향만 우선 구현, 추후 방향 추가시 나머지 방향 추가 예정message
: ToolTip에 보여줄 추가 정보margin
: 화살표 끝을 기준으로 기준 컴포넌트에서 간격을 띄워주고 싶을 때 사용. 기준은 rem트러블 슈팅
위의 사진 한장으로 모든 트러블 슈팅이 드러난다.
span
태그사이에 간격이 생긴다.1번 문제는
width: ‘max-content’
속성으로 쉽게 해결했다.2번 문제가 오랫동안 나를 괴롭혔는데 최종적으로 해결방법은 1px 을 빼주는 것이었다.
서브픽셀 랜더링의 원인은 브라우저가 소수점을 계산하지 못해서 일어나는 현상이다. 즉, 오차가 1px 보다 클 수가 없다. 따라서 화살표 svg의 위치를 잡을 때
transform
속성을 사용하여 원래 위치해야 하는 곳에서 1px을 덜 움직여 주었다.해당 방법을 사용하니 대부분의 경우에 문제가 해결되었지만 사파리에서 화면을 극도로 축소하면 다시 간격이 벌어지는 것을 확인했다. 추후에 리펙토링을 한번 해야할 것 같다.
📌스크린샷 (선택)
storybook 확인해주세요~