Skip to content

Conversation

@rtttr1
Copy link

@rtttr1 rtttr1 commented Sep 30, 2025

변경사항

링크

시급한 정도

🏃‍♂️ 보통 : 최대한 빠르게 리뷰 부탁드립니다.

기타 사항

  • 기존에 컬러 호버시 브라우저의 기본 툴팁으로 색상코드가 나오던 것을 이쁘게 개선했습니다.

AS-IS

2025-09-30.6.53.08.mov

TO-BE

2025-09-30.6.53.23.mov
  • 기존 Alpha 색상 스토리가 Alpha 색상이 아니었습니다.
    따라서 Alpha 색상으로 업데이트 해주고, 불투명도를 나타내기 위해 배경에 격자이미지를 넣어주었습니다.

AS-IS

스크린샷 2025-09-30 오후 6 54 58

TO-BE

스크린샷 2025-09-30 오후 6 55 05
  • yellow900 색상 코드가 달라 피그마 기준으로 변경해주었습니다.

  • Heading_40_B font가 존재하지 않아 추가해주었습니다.
    디자인 피그마에는 네이밍에 숫자가 없는데 mds에는 Heading_1 , Heading_2 의 숫자가 존재해서 우선 Heading_8_40_B로 추가했는데 추후 개선은 주용님께 맡기겠습니다.

스크린샷 2025-09-30 오후 7 00 03

@rtttr1 rtttr1 self-assigned this Sep 30, 2025
@height
Copy link

height bot commented Sep 30, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@changeset-bot
Copy link

changeset-bot bot commented Sep 30, 2025

⚠️ No Changeset found

Latest commit: bbe1d00

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

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

Copy link

@constantly-dev constantly-dev left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!! 코멘트 확인해주세요!

Choose a reason for hiding this comment

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

피그마 상으로 green/yellowalpha 색상이 있어서 아래 alpha 모여있는 곳에 추가해주셔야 할 것 같습니다!
image

Comment on lines +38 to +46
<div className='color-chips'>
<a data-title={`black\n${colors.black}`} style={{ backgroundColor: colors.black }}></a>
</div>
</div>
<div className='colors-group'>
<p>white</p>
<div className='color-chips'>
<a data-title={`white\n${colors.white}`} style={{ backgroundColor: colors.white }}></a>
</div>

Choose a reason for hiding this comment

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

chip 내부에 a태그는 무슨 역할을 하나요??
color code 복사 등의 기능이 있는 건지 궁금합니다. 특별하게 기능이 없다면 a태그가 아니어도 되지 않을까 생각해서 여쭤봅니다!

Comment on lines +104 to +105
/* GPU layer for smoother fade */
will-change: color;

Choose a reason for hiding this comment

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

will-change를 최적화를 위해 사용하신게 맞을까요??
저도 이번에 처음 본 속성이라 찾아보니 남용하면 안되는 생각보다 cost가 비싼 작업 같습니다. 특히 렌더링 과정 중 composite에 해당하는 transform이나 opacity가 아니라 color를 지정해서 결국 layer를 분리해도 repaint가 다시 일어나는 것이 아닐까 하는 생각이 듭니다.

그래서 특별한 이유가 없다면 사용을 지양하는 것이 좋아보이는데 어떻게 생각하시나요??
MDN 참고

Choose a reason for hiding this comment

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

반복되는 코드가 많은 것 같은데 Chip이나 Row등을 따로 컴포넌트로 만들고 map을 사용하도록 수정하면 어떨까요??

Copy link

@ljh0608 ljh0608 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

Comment on lines +58 to +71
<div className='color-chips'>{renderColorChips(scales, 'blue')}</div>
</div>
<div className='colors-group'>
<p>red</p>
<div className='color-chips'>{renderColorChips(scales, 'red')}</div>
</div>
<div className='colors-group'>
<p>green</p>
<div className='color-chips'>{renderColorChips(scales, 'green')}</div>
</div>
<div className='colors-group'>
<p>yellow</p>
<div className='color-chips'>{renderColorChips(scales, 'yellow')}</div>
</div>
Copy link

Choose a reason for hiding this comment

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

반복되는 부분 map활용해서 가독성과 코드량을 줄이면 더 좋을 것 같습니다!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants