Skip to content

Conversation

@WonJuneKim
Copy link
Contributor

@WonJuneKim WonJuneKim commented Nov 22, 2025

💡 작업 내용

  • radix를 사용해 tooltip 커스텀
  • Icon 컴포넌트 ref 주입(임시 조치)
  • Label 컴포넌트 스타일링 일부 변경(임시 조치)

💡 자세한 설명

✅ Context 구조

  • 기본적인 인터페이스 선언 및 props들은 Radix UI 에서 제공하는 기능을 보존했으며, 커스텀을 한 부분에 대해서만 명시적으로 선언하였습니다.

  • ToolTip.Provider(Radix 자체 Provider) 에서는 전역적인 동작을 관리하도록 의도하였습니다. 시스템을 사용하는 공간안에서 일관된 동작을 보장하기 위한 delayDuration 정도가 명시적으로 선언되어있습니다.

  • TooltipContext.Provider는 내부 Context 를 제공하며 선언한 Tooltip.Root 내부 Tooltip.Content에만 영향을 줍니다. Root 단위에서 side, sideOffset, collisionPadding 등을 제어할 수 있도록 커스텀하였습니다.

  • 다시 말해, Content의 포지셔닝을 Root에서 설정하도록 커스텀한 것 입니다.

  • **collisionPadding 의 경우 디자인 에셋에서는 jds.scheme.semantic.margin.md 를 사용해야합니다. 다만 Storybook에서의 한정된 공간 및 토큰 명을 parseInt 등으로 number 타입을 반환하는 과정을 거쳐야하기 때문에 매직넘버 0 을 사용했습니다.

const TooltipContent = ({ children, ...restProps }: TooltipContentProps) => {
  const { side, sideOffset, collisionPadding } = useTooltipContext();

  return (
    <TooltipPrimitive.Portal>
      <StyledTooltipContent
        side={side}
        sideOffset={sideOffset}
        collisionPadding={collisionPadding}
        {...restProps}
      >
        {children}
      </StyledTooltipContent>
    </TooltipPrimitive.Portal>
  );
};

에서 Portal를 별도 명시할 필요 없이 TooltipContent 사용 시 자체적으로 처리 될 수 있도록 변경하였습니다.

✅ 기타 변경점

  • Label.style.ts 에서 display 속성을 제거했으며 선언한 메타 태그의 기본 제공 값을 사용하도록 변경하였습니다.

  • Icon 의 경우 span 태그로 Wrapping 시 ref를 별도로 선언하지 않아 asChild로 ref forwarding 시 DOM까지 ref가 전달되지 않는 문제에 대한 임시 조치입니다.

📗 참고 자료 (선택)

📢 리뷰 요구 사항 (선택)

  • radix 사용이 처음이라 best practice 방식과 맞지 않는 부분들이 있을 거 같은데 피드백 부탁드립니다!

  • Tooltip.Trigger의 경우 asChild 관련 권장하는 방식은 기본값 (false)를 사용하는 것인데, Trigger는 기본적으로 제공하는 Trigger의 HTMLButtonElement 를 렌더링하더라구요! 범용적으로 생각하면 asChild를 false 처리 후 해당 요소를 사용하는 방식 or 현 구현 처럼 forwardRef로 래핑된 interactive 한 요소들에 대해서만 사용을 제약하는 방식 중 후자를 택했습니다. (resetStyle이 주입되지 않았을 때 기본 제공되는 Button이 의도된 시스템과 매우 다르기 때문)

  • 보통 디자인 시스템에서는 restStyle, GlobalStyle 의 핵심 부분 정도는 관리를 해주는 편인데, 현재 tokens/globalStyles 에서 해당 부분이 없기 때문에 오히려 고민되는 부분이네요. (리셋 스타일과 이원화)

✅ 셀프 체크리스트

  • 머지할 브랜치 확인했나요?
  • 이슈는 close 했나요?
  • Reviewers, Labels, Projects를 등록했나요?
  • 기능이 잘 동작하나요?
  • 불필요한 코드는 제거했나요?

closes #263

@WonJuneKim WonJuneKim self-assigned this Nov 22, 2025
@WonJuneKim WonJuneKim added ✨feature 구현, 개선 사항 관련 부분 👩🏻‍💻frontend 프론트엔드 작업 labels Nov 22, 2025
@vercel
Copy link

vercel bot commented Nov 22, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ject-official-web-site-client Ready Ready Preview Comment Nov 25, 2025 11:27pm

@WonJuneKim WonJuneKim changed the title feat: 디자인 시스템 - 툴팁 컴포넌트 구현 feat: 디자인 시스템 - Tooltip(툴팁) 컴포넌트 구현 Nov 22, 2025
@whdgur5717
Copy link
Contributor

whdgur5717 commented Nov 23, 2025

resetstyle은 추가될 거라, 그부분 때문에 툴팁 컴포넌트에 영향을 미치는건 고려하지 않아도 됩니다

Copy link
Contributor

@whdgur5717 whdgur5717 left a comment

Choose a reason for hiding this comment

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

스타일 관련한 선언이나 실제 값 적용은 현재는 크게 중요하지 않은것같아서 생략하겠습니다


export const Icon = ({ name, size = 'md', color = 'currentColor', ...props }: IconProps) => {
const IconComponent = iconMap[name];
export const Icon = forwardRef<HTMLSpanElement, IconProps>(
Copy link
Contributor

@whdgur5717 whdgur5717 Nov 23, 2025

Choose a reason for hiding this comment

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

HTMLSpanElement대신, ElementRef<typeofStyledIconWrapper>(정확한 문법은아닙니다) 로 활용하는게 좋을것같습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

지금처럼 Wrapper로 감싸는 경우 특히 해당 요소를 직접 참조하는 것이 더 적절하겠네요! 5e1fd34 에서 변경했습니다~

import type { Tooltip } from 'radix-ui';
import type { ReactNode } from 'react';

export type TooltipSide = 'top' | 'right' | 'bottom' | 'left';
Copy link
Contributor

Choose a reason for hiding this comment

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

이것도 TooltipContentProps의 side의 타입과 동일한 것 같은데 따로 선언하신 이유가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음에는 설정한 context 내부에서는 undefined 값을 허용하지 않기 때문에 명시적으로 작성해놓았는데, 그렇다하더라도 NonNullable(radix의 기본 제공 props) 으로 사용하거나, 기존 props 형태를 그대로 사용하는게 유지보수하기 더 용이하겠네요.

e008dd1에서 변경하였습니다!

Comment on lines 12 to 16
interface TooltipContextValue {
side: TooltipSide;
sideOffset: number;
collisionPadding: number;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

이 속성들이 Root 컴포넌트에서 주입받아서- useContext를 통해 전달되어야 하는 이유가 있나요?
지금 코드를 봐도 크게 Root나 Trigger에서 사용되고 있지는 않는것 같은데,
그냥 간단하게, 원래 radix-ui TooltipContent의 활용법 그대로 활용해도 괜찮을것 같습니다

Copy link
Contributor

@rkdcodus rkdcodus Nov 23, 2025

Choose a reason for hiding this comment

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

Tooltip.Content에서도 해당 props들을 직접 전달할 수 있지만 Root에서 관리하도록 API를 재구성한 것으로 보이네요!!
radix를 기반으로 하는 컴포넌트들의 API 패턴들을 전반적으로 Root에서 관리하는 패턴으로 갈 것인지 의견을 나눠보는게 좋을 것 같아요 다른 분들은 어떻게 생각하시나요?

다른 컴포넌트 포함 전반적으로 Root에서 관리하는 방식이면 일관성 면에서는 이런 방식도 좋다고 생각하는데 일부 핵심 속성을 Root에서 관리하면 구조적으로 간단해져서 좋다고 생각하는데 나머지 props는 또 Content에 직접 전달하고 있어서 이부분이 컴포 사용자 입장에서 조금 헷갈릴 수도 있을 것 같긴하네요!

Copy link
Contributor

@whdgur5717 whdgur5717 Nov 23, 2025

Choose a reason for hiding this comment

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

합성 컴포넌트라는걸 사용하는 이유가, srp 관점에서 각 파트별로 책임을 담당하면서 복잡도를 줄이는 목적이 크다고 보는데
어디에 렌더링될지를 결정하는건 Content에서 담당하는 역할인데, 저렇게 Root에 넣게 되면 합성 컴포넌트를 쓸 이유가 크게 없는것 같습니다

  • 지금은 tooltip.Root에서 다른 속성들을 많이 사용하지 않게끔 구현하신 것 같은데 대부분의 Root 컴포넌트에서는 제어/비제어라던가 이벤트 콜백함수등을 정의하는 경우가 많아서 오히려 더 복잡할 것 같습니다

  • 오히려 저렇게 사용할거라면 합성컴포넌트 방식보다 차라리 Content에 해당하는 걸 props로 받는게 더 좋아보입니다

<Tooltip content={<Button>}/>

Copy link
Contributor

Choose a reason for hiding this comment

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

지금 구현되어 있는 Select 컴포넌트의 경우 Context로 받은 값을 Label, Checkbox, Radio 같은 여러 종류의 자식 컴포넌트에서 사용할 수 있도록 설계되어 있는 것 같은데, 이런 경우라면 Root에서 Context로 관리하는 것이 합리적이라고 생각합니다.
그런데 Tooltip의 경우 현재 Context로 받은 값을 Content 한 곳에서만 사용하고 있고, 향후에도 추가될 자식 컴포넌트가 없을 것 같다면, 꼭 Context를 사용해야 할까? 하는 생각이 들긴 합니다.
기존에 구현한 다른 컴포넌트들과의 일관성을 우선시한다면 현재 구조도 괜찮겠지만, Radix UI를 사용하기로 했다면 해당 props를 사용하는 곳에 직접 전달하는 것이 더 명확하지 않을까 생각합니다!

Copy link
Contributor Author

@WonJuneKim WonJuneKim Nov 25, 2025

Choose a reason for hiding this comment

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

동구님 말씀대로 Context로 받은 값을 내부 자식들에게 각각 뿌려주거나, 여러개의 자식 컴포넌트들이 와서 일괄적인 관리가 필요한 경우에는 Root에서 관리하는 것에 이점이 있지만 ToolTip의 경우 사실상 1대1 대응관계라 (물론 특정 트리거에 대해서 여러개의 Content가 올수도 있습니다...만 이러한 매우 예외적인건 제외) 오히려 Context가 컴포넌트의 복잡도를 올린 것 같네요.

또한 Radix 내부에서 이미 해당 구조를 사용하고 있기 때문에 Context 자체를 추가로 씌우는 구조는 변경하도록 하겠습니다~

Copy link
Contributor Author

@WonJuneKim WonJuneKim Nov 25, 2025

Choose a reason for hiding this comment

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

@whdgur5717 @rkdcodus @kimdonggu42
피그마 요구 사항 중

  • 상부에 여유 공간이 없을 경우 우측부, 우측부에도 여유 공간이 없을 경우 하부에 표시됩니다. 우선순위 순으로 상 → 우 → 하 → 좌 입니다.


해당 부분을 구현을 하기 위해서는 radix가 기본 제공하는 avoidCollisionsprop에는 지정된 방향성만 존재하기 때문에 Trigger의 ref를 Content가 사용해야하는 상황이 발생합니다. 따라서 위치 감지 로직을 구현하여 사용할 경우 다시 자체 Context가 필요할 것으로 생각되나, 일단 해당 부분은 Todo로 남겨놓겠습니다!

Copy link
Contributor

@rkdcodus rkdcodus left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! 😊😊 radix 사용하니까 구조 이해가 잘 되는 것 같아요!
피그마 디자인 요구사항 보니까 side 우선순위에 따라 자동으로 툴팁이 나타나는 옵션이 필요한 것 같더라구요!

docs: {
description: {
story:
'툴팁은 네 가지 방향(top, right, bottom, left)으로 표시할 수 있습니다. 공간이 부족하면 자동으로 다른 방향으로 전환됩니다.',
Copy link
Contributor

Choose a reason for hiding this comment

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

피그마에서 정의된 내용 보니까 주변 여유 공간에 따라 방향 우선순위(상 -> 우 -> 하 -> 좌)가 있더라구요!
side 방향을 내부에서 자동으로 결정하도록 하는 auto placement 같은 옵션을 따로 두거나 해서 uncontrolled 방식이 추가되면 좋을 것 같습니다!

sideOffset = 8,
collisionPadding = 0,
delayDuration = 0,
...radixProps
Copy link
Contributor

Choose a reason for hiding this comment

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

radixProps라고 네이밍 지으니까 좋네요!!! 👍

//NOTE : 모바일 디바이스 절대 최소치 기준(토큰 breakpoint 명으로 개선되어도 됨)
maxWidth: pxToRem(320),
overflowWrap: 'break-word',
zIndex: 9999,
Copy link
Contributor

Choose a reason for hiding this comment

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

이렇게 앞으로 나와야하는 요소에 대해 zIndex 값을 어떻게 사용해야될지 기준이 정해지면 좋을 것 같아요! zIndex 토큰을 만들어도 좋을 것 같네요!! (저는 값을 몇으로 줘야하나 고민이 되어서 저는 그냥 50으로 작성했었습니닷..)

Copy link
Contributor Author

@WonJuneKim WonJuneKim Nov 25, 2025

Choose a reason for hiding this comment

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

케이스별로 쌓여야하는 레이어들도 있기 때문에 0, 1, 2, 3, 이후 9999 등도 좋을 거 같네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 동영님께 ui 상으로 권장되는 오버레이 또는 z-index의 임계점을 여쭤봐도 좋을 거 같네요~ 답변 오면 공유드리겠습니다!

Comment on lines +17 to +23
'&[data-state="delayed-open"]': {
animation: `tooltipFadeIn ${theme.environment.semantic.duration[200]} ${theme.environment.semantic.motion.fluent}`,
},

'&[data-state="instant-open"]': {
animation: `tooltipFadeIn ${theme.environment.semantic.duration[200]} ${theme.environment.semantic.motion.fluent}`,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

두 data-state에 해당하는 animation 코드가 동일해서 합쳐서 쓰는게 나은가 싶었는데 추후에 서로 다른 스타일을 줄 가능성도 있으니까 지금처럼 분리하는 것도 합리적이네요! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요 부분은 이후 툴팁에 대한 명세가 추가될 수 있기 때문에(확장성) 별도로 명시해 두었습니다!

Copy link
Contributor

@kimdonggu42 kimdonggu42 left a comment

Choose a reason for hiding this comment

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

Radix UI를 Emotion의 styled로 래핑해서 사용하는 거였군요! 배워갑니다! 👍

styled(TooltipPrimitive.Content)(({ theme }) => ({
  backgroundColor: theme.color.semantic.fill.boldest,
}));

…y/JECT-Official-WebSite-Client into feat/263-tooltip-design-system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨feature 구현, 개선 사항 관련 부분 👩🏻‍💻frontend 프론트엔드 작업

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

feat: ToolTip (툴팁) - 디자인 시스템을 구현합니다.

5 participants