Skip to content

fix(Tooltip): 접근성(a11y) 개선 및 스타일 코드 개선#245

Open
minji0214 wants to merge 7 commits intodev/side-v2from
fix/tooltip-a11y
Open

fix(Tooltip): 접근성(a11y) 개선 및 스타일 코드 개선#245
minji0214 wants to merge 7 commits intodev/side-v2from
fix/tooltip-a11y

Conversation

@minji0214
Copy link
Copy Markdown

@minji0214 minji0214 commented Apr 19, 2026

Changes

  • role="tooltip"을 trigger wrapper가 아닌 실제 tooltip 엘리먼트에 배치하고, useId로 생성한 id로 aria-describedby 연결
  • click trigger일 때 aria-expanded로 열림/닫힘 상태 노출
  • Space/Enter 키 이벤트 처리 버그 수정 ('space' → 'Space')
  • scroll/resize 시 tooltip 위치 재계산 추가
  • styleVariants → vanilla-extract recipe()로 스타일 구조 통일
  • TooltipPosition, TooltipTrigger를 string literal type에서 enum-style const 객체로 변환

Visuals

as-is

2026-04-19.10.18.00.mov

to-be

2026-04-19.10.21.54.mov

Checklist

  • Have you written the functional specifications?
  • Have you written the test code?

Additional Discussion Points

현재 테스트 코드가 컴포넌트마다 한글/영어로 혼재되어 있어요. 어느 언어로 통일하면 좋을지 한번 같이 이야기해보면 좋을 것 같습니다! 🙂(논의완)

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 19, 2026

🦋 Changeset detected

Latest commit: 4154442

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@sipe-team/tooltip Patch
@sipe-team/side Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 19, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • main
  • release/v1

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 851350d5-32df-410a-8619-deb17a2dc01b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/tooltip-a11y

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
packages/tooltip/src/Tooltip.css.ts 100.00% <100.00%> (ø)
packages/tooltip/src/Tooltip.test.tsx 100.00% <100.00%> (ø)
packages/tooltip/src/Tooltip.tsx 100.00% <100.00%> (ø)
...ckages/tooltip/src/hooks/useTooltip/useTooltip.tsx 98.33% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@3o14 3o14 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 테스트 코드만 논의된 대로 영어로 통일되면 좋을 것 같습니다 👍🏻

@minji0214
Copy link
Copy Markdown
Author

고생하셨습니다! 테스트 코드만 논의된 대로 영어로 통일되면 좋을 것 같습니다 👍🏻

영어로 변경했습니다!

3o14
3o14 approved these changes Apr 20, 2026
@G-hoon
Copy link
Copy Markdown
Collaborator

G-hoon commented Apr 21, 2026

@Yeom-JinHo
진호님 이거 컴포넌트 변경 changeset 추가 안해도 되는지 검토 부탁드립니다~!

}

interface CalculateTooltipPositionParams {
export interface CalculateTooltipPositionParams {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

이거는 왜 export 하셨을까요~?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

타입 정리하다가 잘못수정된거 같아요. 제거하였습니다!

@Yeom-JinHo
Copy link
Copy Markdown
Member

@Yeom-JinHo 진호님 이거 컴포넌트 변경 changeset 추가 안해도 되는지 검토 부탁드립니다~!

@minji0214 changeset 한번 돌려주세요~

@minji0214
Copy link
Copy Markdown
Author

@Yeom-JinHo 진호님 이거 컴포넌트 변경 changeset 추가 안해도 되는지 검토 부탁드립니다~!

@minji0214 changeset 한번 돌려주세요~

반영하였습니다!

Copy link
Copy Markdown
Collaborator

@G-hoon G-hoon left a comment

Choose a reason for hiding this comment

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

@minji0214 님,
일단 Approve 해드리는데,
이거 Tooltip 컴포넌트 전반적으로 수정이 필요할 것 같아요...

  1. useTooltip.tsx -> useTooltip.ts 로 되어야 할 것 같고.
  2. handlePosition은 매 이벤트마다 DOM rect를 읽고 state를 업데이트합니다. scroll/resize는 매우 빈번하게 scroll 및 resize 가 매 렌더마다 발생하는데, requestAnimationFrame 혹은 쓰로틀링 걸어야 할 것 같아요
  3. 아래 코드에서, createPortal 된 순간 isVisible 값이 항상 true 인데, 아래 className 속성 isVisible 값이 있을 필요가 없을 것 같아요.
createPortal(
          <div
            ref={tooltipRef}
            className={clsx(styles.tooltip({ placement: placementProp }), tooltipClassName, isVisible && 'visible')}
            style={

| 'bottom-right'
| 'left'
| 'right';
export const TooltipPosition = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

더 나은 패턴이네요 좋당 ~

Copy link
Copy Markdown
Contributor

@froggy1014 froggy1014 left a comment

Choose a reason for hiding this comment

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

너무 수고많으셨습니다.

일단 SIDE 디자인시스템에 Tooltip에 대한 명확한 요구사항이 없어서 뭐가 맞다 틀리다는 없지만

WCAG 기준은 마저 다 충족하는게 좋을거같네요 !

export type TooltipPosition = (typeof TooltipPosition)[keyof typeof TooltipPosition];

export type TooltipTrigger = 'hover' | 'click';
export const TooltipTrigger = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@minji0214

기존에 trigger 방식이 hover, click 을 명시적으로 정해서 사용하는 패턴으로 구현이 되어있었네요.

SIDE 디자인 시스템에 요구사항이 명확히 없었어서 아마 임의로 작업을 했던거같습니다.

저도 의아해서 조금 찾아보니까

trigger 방식을 명시적으로 정해놓지는 않고 click과 hover는 항상 이벤트는 받고있는거 같고, 필요하면 Controlled(제어) 할 수 있도록 boolean을 넘겨주는 패턴이 좀 있는거 같아요.

일단 지금 문제점은 trigger 방식을 hover로 해놨을때, tab을 통해서 keyboard focus하게되면 툴팁이 안나오는거 같습니다. 이거는 지원해주는게 옳을거 같은데 어떻게 생각하시나요?


export type TooltipTrigger = 'hover' | 'click';
export const TooltipTrigger = {
hover: 'hover',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@minji0214

저도 다른거 찾아보다가 알게된거인데, hover했을때 나오는 Tooltip Content에 마우스가 이동했을때 Unmount되면 안된다고 해요

Screen.Recording.2026-04-23.at.00.15.25.mov

https://www.w3.org/WAI/WCAG22/Understanding/content-on-hover-or-focus.html

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants