Skip to content
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/Select & Option 컴포넌트 작업 #31

Merged
merged 15 commits into from
May 9, 2024
Merged

Conversation

rai-95
Copy link
Contributor

@rai-95 rai-95 commented May 6, 2024

📖 관련 문서

#15
#23

✍🏻 변경사항:

스크린샷 2024-05-05 오전 1 00 40

스크린샷 2024-05-07 오전 12 29 12

  • Select Text Input 추가
  • tag 추가 및 삭제
  • warning_triangle 아이콘 stroke가 적용되지 않는 이슈로 fill 변경했습니다!
    • fill="current" -> fill="currentColor"
  • option 컴포넌트 추가

🔍 확인할 목록

[ ] 스토리북 select 확인
[ ] 스토리북 option 확인
[ ] tag 입력 및 삭제 확인

+) optionList도 같이 작업할 예정입니다!
interface 중복 정리 필요
TextField와 중복되는 부분 확인 필요

@rai-95 rai-95 requested a review from hoongding May 6, 2024 06:12
@rai-95 rai-95 self-assigned this May 6, 2024
@rai-95 rai-95 added ✨ feat 구현, 개선 사항에 관련된 내용입니다. 🎨 style UI 관련 이슈입니다. labels May 6, 2024
Copy link

github-actions bot commented May 6, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-vnuwuoxylm.chromatic.com/

@rai-95 rai-95 changed the title Feat/Select 컴포넌트 작업 Feat/Select & Option 컴포넌트 작업 May 6, 2024
Copy link
Contributor

@hoongding hoongding left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~ 성은님! 리뷰 확인 부탁드립니다~

return <div className={cn(selectInputWrapperVariants({ disabled, isInvalid }))}>{children}</div>;
};

export default SelectInputWrapper;
Copy link
Contributor

Choose a reason for hiding this comment

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

제일 아래로 빼는건 어떨까용~?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵! 좋은거 같아요!

value={value}
className={cn(selectInputVariants({ disabled }))}
placeholder={placeholder}
onInput={onChange}
Copy link
Contributor

Choose a reason for hiding this comment

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

React에서는 onInput과 onChange의 차이가 없다고 하더라구요!
input의 event는 onInput으로 할까요 onChange라고 할까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오호! React에서는 onChange을 일반적으로 많이 쓰는거 같아요! onChange가 좋은거 같습니다

{isInvalid && (
<Icon
name='warningTriangle_f'
className='w-16 h-16 -icon-critical text-icon-critical'
Copy link
Contributor

Choose a reason for hiding this comment

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

이쪽 -icon-critical은 오타인가용?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗.. 작업하다가 삭제를 안했나보네요 수정하겠습니다ㅠ

@@ -1,3 +1,3 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" fill="none">
<path d="M7.14295 1.56366L6.84305 1.044V1.044L7.14295 1.56366ZM8.85713 1.56366L8.55722 2.08333V2.08333L8.85713 1.56366ZM9.49375 2.19538L10.0118 1.89265L10.0101 1.88982L9.49375 2.19538ZM14.6667 11.5376L14.0667 11.5392L14.6667 11.5376ZM14.4327 10.647L14.9555 10.3525L14.9507 10.3443L14.4327 10.647ZM13.8057 13.086L13.4958 12.5722L13.8057 13.086ZM12.9325 13.3333L12.9325 13.9334L12.9393 13.9333L12.9325 13.3333ZM2.19442 13.086L1.88453 13.5998H1.88453L2.19442 13.086ZM3.0612 13.3333L3.05444 13.9333H3.0612L3.0612 13.3333ZM1.56262 12.4294L2.087 12.1378H2.087L1.56262 12.4294ZM1.33338 11.5376L1.93338 11.5392L1.33338 11.5376ZM1.56749 10.6471L1.04939 10.3443L1.04473 10.3526L1.56749 10.6471ZM6.50632 2.19538L5.98995 1.88981L5.98829 1.89266L6.50632 2.19538ZM8.60004 5.33334C8.60004 5.00197 8.33141 4.73334 8.00004 4.73334C7.66867 4.73334 7.40004 5.00197 7.40004 5.33334H8.60004ZM7.40004 8.66668C7.40004 8.99805 7.66867 9.26668 8.00004 9.26668C8.33141 9.26668 8.60004 8.99805 8.60004 8.66668H7.40004ZM8.60004 10.6667C8.60004 10.3353 8.33141 10.0667 8.00004 10.0667C7.66867 10.0667 7.40004 10.3353 7.40004 10.6667H8.60004ZM7.40004 10.7333C7.40004 11.0647 7.66867 11.3333 8.00004 11.3333C8.33141 11.3333 8.60004 11.0647 8.60004 10.7333H7.40004ZM8.00004 0.733343C7.59382 0.733343 7.19529 0.84071 6.84305 1.044L7.44286 2.08333C7.6139 1.98463 7.80574 1.93334 8.00004 1.93334V0.733343ZM9.15704 1.044C8.80479 0.84071 8.40626 0.733343 8.00004 0.733343V1.93334C8.19434 1.93334 8.38619 1.98463 8.55722 2.08333L9.15704 1.044ZM10.0101 1.88982C9.80252 1.53901 9.50917 1.24722 9.15704 1.044L8.55722 2.08333C8.72836 2.1821 8.87353 2.32545 8.97739 2.50094L10.0101 1.88982ZM15.2667 11.5359C15.2656 11.1214 15.1587 10.7134 14.9554 10.3526L13.9099 10.9415C14.0117 11.1223 14.0661 11.3284 14.0667 11.5392L15.2667 11.5359ZM14.9618 12.721C15.1631 12.3591 15.2678 11.9506 15.2667 11.5359L14.0667 11.5392C14.0673 11.7499 14.014 11.9564 13.9131 12.1378L14.9618 12.721ZM14.1156 13.5998C14.4693 13.3864 14.7606 13.083 14.9618 12.721L13.9131 12.1378C13.8122 12.3191 13.6679 12.4684 13.4958 12.5722L14.1156 13.5998ZM12.9456 13.9332C13.3583 13.9286 13.7617 13.8132 14.1156 13.5998L13.4958 12.5722C13.3238 12.676 13.1295 12.7311 12.9321 12.7333L12.9456 13.9332ZM12.9393 13.9333L12.9457 13.9332L12.9321 12.7333L12.9257 12.7334L12.9393 13.9333ZM1.88453 13.5998C2.23833 13.8132 2.64171 13.9286 3.05444 13.9333L3.06796 12.7333C2.87062 12.7311 2.67633 12.676 2.50431 12.5722L1.88453 13.5998ZM1.03824 12.721C1.23953 13.083 1.5308 13.3864 1.88453 13.5998L2.50431 12.5722C2.33221 12.4684 2.18784 12.3191 2.087 12.1378L1.03824 12.721ZM0.733383 11.5359C0.73225 11.9506 0.837007 12.3591 1.03824 12.721L2.087 12.1378C1.98612 11.9564 1.9328 11.7499 1.93338 11.5392L0.733383 11.5359ZM1.04473 10.3526C0.841504 10.7134 0.734516 11.1213 0.733383 11.5359L1.93338 11.5392C1.93395 11.3285 1.98839 11.1224 2.09025 10.9416L1.04473 10.3526ZM6.84305 1.044C6.49092 1.24721 6.19756 1.539 5.98996 1.88982L7.02269 2.50094C7.12653 2.32545 7.27171 2.18211 7.44286 2.08333L6.84305 1.044ZM12.9389 12.7333L3.0612 12.7333L3.0612 13.9333L12.9389 13.9333L12.9389 12.7333ZM12.9325 12.7333L3.0612 12.7333L3.0612 13.9333L12.9325 13.9333L12.9325 12.7333ZM8.97572 2.49811L13.9146 10.9498L14.9507 10.3443L10.0118 1.89266L8.97572 2.49811ZM2.08553 10.9498L7.02436 2.4981L5.98829 1.89266L1.04946 10.3444L2.08553 10.9498ZM7.40004 5.33334V8.66668H8.60004V5.33334H7.40004ZM7.40004 10.6667V10.7333H8.60004V10.6667H7.40004Z" fill="current"/>
<path d="M7.14295 1.56366L6.84305 1.044V1.044L7.14295 1.56366ZM8.85713 1.56366L8.55722 2.08333V2.08333L8.85713 1.56366ZM9.49375 2.19538L10.0118 1.89265L10.0101 1.88982L9.49375 2.19538ZM14.6667 11.5376L14.0667 11.5392L14.6667 11.5376ZM14.4327 10.647L14.9555 10.3525L14.9507 10.3443L14.4327 10.647ZM13.8057 13.086L13.4958 12.5722L13.8057 13.086ZM12.9325 13.3333L12.9325 13.9334L12.9393 13.9333L12.9325 13.3333ZM2.19442 13.086L1.88453 13.5998H1.88453L2.19442 13.086ZM3.0612 13.3333L3.05444 13.9333H3.0612L3.0612 13.3333ZM1.56262 12.4294L2.087 12.1378H2.087L1.56262 12.4294ZM1.33338 11.5376L1.93338 11.5392L1.33338 11.5376ZM1.56749 10.6471L1.04939 10.3443L1.04473 10.3526L1.56749 10.6471ZM6.50632 2.19538L5.98995 1.88981L5.98829 1.89266L6.50632 2.19538ZM8.60004 5.33334C8.60004 5.00197 8.33141 4.73334 8.00004 4.73334C7.66867 4.73334 7.40004 5.00197 7.40004 5.33334H8.60004ZM7.40004 8.66668C7.40004 8.99805 7.66867 9.26668 8.00004 9.26668C8.33141 9.26668 8.60004 8.99805 8.60004 8.66668H7.40004ZM8.60004 10.6667C8.60004 10.3353 8.33141 10.0667 8.00004 10.0667C7.66867 10.0667 7.40004 10.3353 7.40004 10.6667H8.60004ZM7.40004 10.7333C7.40004 11.0647 7.66867 11.3333 8.00004 11.3333C8.33141 11.3333 8.60004 11.0647 8.60004 10.7333H7.40004ZM8.00004 0.733343C7.59382 0.733343 7.19529 0.84071 6.84305 1.044L7.44286 2.08333C7.6139 1.98463 7.80574 1.93334 8.00004 1.93334V0.733343ZM9.15704 1.044C8.80479 0.84071 8.40626 0.733343 8.00004 0.733343V1.93334C8.19434 1.93334 8.38619 1.98463 8.55722 2.08333L9.15704 1.044ZM10.0101 1.88982C9.80252 1.53901 9.50917 1.24722 9.15704 1.044L8.55722 2.08333C8.72836 2.1821 8.87353 2.32545 8.97739 2.50094L10.0101 1.88982ZM15.2667 11.5359C15.2656 11.1214 15.1587 10.7134 14.9554 10.3526L13.9099 10.9415C14.0117 11.1223 14.0661 11.3284 14.0667 11.5392L15.2667 11.5359ZM14.9618 12.721C15.1631 12.3591 15.2678 11.9506 15.2667 11.5359L14.0667 11.5392C14.0673 11.7499 14.014 11.9564 13.9131 12.1378L14.9618 12.721ZM14.1156 13.5998C14.4693 13.3864 14.7606 13.083 14.9618 12.721L13.9131 12.1378C13.8122 12.3191 13.6679 12.4684 13.4958 12.5722L14.1156 13.5998ZM12.9456 13.9332C13.3583 13.9286 13.7617 13.8132 14.1156 13.5998L13.4958 12.5722C13.3238 12.676 13.1295 12.7311 12.9321 12.7333L12.9456 13.9332ZM12.9393 13.9333L12.9457 13.9332L12.9321 12.7333L12.9257 12.7334L12.9393 13.9333ZM1.88453 13.5998C2.23833 13.8132 2.64171 13.9286 3.05444 13.9333L3.06796 12.7333C2.87062 12.7311 2.67633 12.676 2.50431 12.5722L1.88453 13.5998ZM1.03824 12.721C1.23953 13.083 1.5308 13.3864 1.88453 13.5998L2.50431 12.5722C2.33221 12.4684 2.18784 12.3191 2.087 12.1378L1.03824 12.721ZM0.733383 11.5359C0.73225 11.9506 0.837007 12.3591 1.03824 12.721L2.087 12.1378C1.98612 11.9564 1.9328 11.7499 1.93338 11.5392L0.733383 11.5359ZM1.04473 10.3526C0.841504 10.7134 0.734516 11.1213 0.733383 11.5359L1.93338 11.5392C1.93395 11.3285 1.98839 11.1224 2.09025 10.9416L1.04473 10.3526ZM6.84305 1.044C6.49092 1.24721 6.19756 1.539 5.98996 1.88982L7.02269 2.50094C7.12653 2.32545 7.27171 2.18211 7.44286 2.08333L6.84305 1.044ZM12.9389 12.7333L3.0612 12.7333L3.0612 13.9333L12.9389 13.9333L12.9389 12.7333ZM12.9325 12.7333L3.0612 12.7333L3.0612 13.9333L12.9325 13.9333L12.9325 12.7333ZM8.97572 2.49811L13.9146 10.9498L14.9507 10.3443L10.0118 1.89266L8.97572 2.49811ZM2.08553 10.9498L7.02436 2.4981L5.98829 1.89266L1.04946 10.3444L2.08553 10.9498ZM7.40004 5.33334V8.66668H8.60004V5.33334H7.40004ZM7.40004 10.6667V10.7333H8.60004V10.6667H7.40004Z" fill="currentColor"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 current에서 currentColor로 변경하신 이유 여쭤봐도될까용?

Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 fill이 안먹히면 fill-색깔 하면 current로 설정해도 되더라구용!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fill-색깔이 어떤걸까요!? fill-color?

current에 stroke했을때 색이 이상하게 입혀져서 변경했습니다!

스크린샷 2024-05-07 오전 1 48 49

current보다 currentColor로 작업을 많이 했었는데 부모의 컬러값으로 변경이 가능해서 공통 컴포넌트에서는 좋다고 하더라구요!

혹시 평소에도 current로 작업을 많이 하셨을까요!

Copy link
Contributor

Choose a reason for hiding this comment

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

저희 아이콘이 fill값으로 색깔을 지정하거나 stroke값으로 색깔을 지정하거나 두개가 있더라구요!
그래서
fill이 current면 맨 뒤에 f를, stroke가 current면 맨 뒤에 s를 붙여준다.
svg 태그에 붙어있는 width, height는 삭제하고, viewbox는 남긴다.

사용법

  1. fill이 current 인 경우
    <Icon name='warningTriangle_f' className = 'w-16 h-16 fill-critical' />

  2. stroke가 current 인 경우
    <Icon name='chevronDown_s' className = 'w-16 h-16 stroke-critical' />

이런식으로 사용방법을 한번 만들어봤습니다!! Icon>lib>index.ts 에서 확인 가능합니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

부모의 컬러값으로 변경이 가능하다면 currentColor로 해도 되겠군용!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그러네요! Icon작업하시고 말씀해주신거 같은데 까먹었었네요ㅠㅠ

공통 컴포넌트라서 stroke와 fill로 나눠지는것보다 currentColor로 해서 text 컬러 하나로 관리하는게 좋을거 같은데 어떻게 생각하시나요!?

Copy link
Contributor

Choose a reason for hiding this comment

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

너무 좋습니다!! 이 부분 변경해야겠네용

Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 성은님 아이콘 스토리북 풀리퀘에서 작업 해주실 수 있을까요? Icon 쪽 변경 사항 한번에 처리해도 좋을것 같아서요!

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

Choose a reason for hiding this comment

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

감사합니다 성은님!!

Copy link

github-actions bot commented May 6, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-ezvsmohksa.chromatic.com/


const selectInputWrapperVariants = cva(
[
'w-full max-h-48 h-full flex items-center justify-between gap-8 px-16 py-12 border rounded-lg bg-surface border-border hover:border-border-hover',
Copy link
Contributor

Choose a reason for hiding this comment

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

@rai-95
추가로 성은님! Input foucs 되었을 때 style이 빠진 것 같습니다! 확인 부탁드립니당~
InputWrapper에서는 focus-within으로 focus event 잡을 수 있더라구요! 참고하시면 좋을 듯 합니다!

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

github-actions bot commented May 7, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-rwylslpcxu.chromatic.com/

Copy link

github-actions bot commented May 7, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-hqkynbuwdc.chromatic.com/

Copy link
Contributor

@hoongding hoongding left a comment

Choose a reason for hiding this comment

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

@rai-95
성은님! inValid 상태일 때, focus 시, default state일 때처럼 녹색으로 focus 색이 나옵니다!!

Copy link

github-actions bot commented May 9, 2024

🚀 Storybook 자동 배포 URL: https://6637bd73cba78215aa6e39fa-wimhugevsg.chromatic.com/

@hoongding hoongding merged commit d7044e4 into main May 9, 2024
3 checks passed
@bbung95 bbung95 self-assigned this Jun 12, 2024
@hoongding hoongding deleted the feat/common-select branch July 13, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ feat 구현, 개선 사항에 관련된 내용입니다. 🎨 style UI 관련 이슈입니다.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants