refactor(tooltip): replace styles with design tokens#290
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🦋 Changeset detectedLatest commit: e42df01 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
| const tooltipArrowSize = `calc(${vars.spacing.component.md} / 2)`; | ||
| const tooltipArrowOffset = `calc(${vars.spacing.component.md} / -2)`; |
There was a problem hiding this comment.
정의되지 않은 토큰이면 하드코딩하는게 좋을꺼 같아요~
spacing이라 size에 의존이 생기는게 어색합니다!
| boxShadow: '0 4px 8px rgba(0, 0, 0, 0.2)', | ||
| zIndex: 1000, | ||
| boxShadow: tooltipShadow, | ||
| zIndex: zIndex.dropdown, |
There was a problem hiding this comment.
https://github.com/sipe-team/side/blob/main/packages/tokens/src/effects/zIndex.ts
를 보니까,
zIndex.tooltip 이 있는데. 이를 쓰면 어떨까요?
| border: '1px solid #ccc', | ||
| borderRadius: '4px', | ||
| borderRadius: vars.radius.component.md, | ||
| backgroundColor: '#f9f9f9', |
There was a problem hiding this comment.
이거 색,
"#ccc" 랑 "f9f9f9" 는 따로 토큰화 하지 않고 냅두는 건가요?
|
|
||
| const tooltipBackgroundColor = `var(--tooltip-bg-color, ${themeColor['1st'].background})`; | ||
| const tooltipArrowSize = `calc(${vars.spacing.component.md} / 2)`; | ||
| const tooltipArrowOffset = `calc(${vars.spacing.component.md} / -2)`; |
There was a problem hiding this comment.
const tooltipArrowSize = calc(${vars.spacing.component.md} / 2);
const tooltipArrowOffset = calc(${vars.spacing.component.md} / -2);
이거를,
const tooltipArrowSize = `calc(${vars.spacing.component.md} / 2)`;
const tooltipArrowOffset = `calc(${tooltipArrowSize} * -1)`;
처럼 바꾸는 건 어떨가요...?
Changes
Visuals
Checklist
Additional Discussion Points