[FE] 공용 토글 버튼 컴포넌트 구현 #158

Largopie · 2024-08-05T06:38:18Z

작업 내용

https://paper-mass-5ff.notion.site/47545046f8414544aa193d164d9ed578?pvs=4

작업 내용 및 특이사항 노션에 기록해두었습니다 :)

특이 사항

stylelint 관련 에러 발생(노션에 "구현 중 발견한 Stylelint 에러" 참고)

리뷰 요구사항 (선택)

isClicked 라는 네이밍이 살짝 어색하게 느껴지는데 다른 의견 있으면 자유롭게 부탁드립니다! 🙇‍♂️

github-actions · 2024-08-05T13:33:20Z

Test Results

5 tests 5 ✅ 3s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit 51d19f4.

♻️ This comment has been updated with latest results.

Yoonkyoungme

낙타~~ 빠르게 구현해 주셨군요!! 고생 많으셨습니다🤸‍♀️

Yoonkyoungme · 2024-08-05T16:25:20Z

frontend/src/components/_common/ToggleButton/ToggleButton.styles.ts

+
+export const s_input = css`
+  position: absolute;
+  left: -1000%;


[Q]
left: -1000%는 무엇을 위한 설정인가요? 🫨

아하 해당 스타일 속성을 없애면 아래처럼 보이는군요 ...!

[P3]
주석으로 간략하게 설명을 남겨두면 좋을 것 같네요!

맞아요~! 해당 토글 버튼은 label에서 작용하도록 구현했어요!
빙봉 말대로 주석으로 간략한 설명을 남겨두면 더욱 좋겠네요! 의견 감사해요~! 🙌

Yoonkyoungme · 2024-08-05T16:27:32Z

frontend/src/components/_common/ToggleButton/index.tsx

+
+interface ToggleButtonProps {
+  id: string;
+  isClicked: boolean;


[P3]
isClicked 대신 isToggled 어떤가요?

좋습니다! 이걸로 하면 더욱 직관적이겠네요!

Yoonkyoungme · 2024-08-05T16:31:22Z

frontend/src/components/_common/ToggleButton/index.tsx

[P3]
Button 관련된 컴포넌트들은
components/_comoon/Button 디렉터리 하위에 위치시키는 것은 어떨까요?

ex)
components/_common
ㄴ Button
ㄴ BaseButton
ㄴ ToogleButton
ㄴ NavigateButton
ㄴ AuthButton
ㄴ HomeButton

현재 공통 컴포넌트로 구현한 버튼이 수정사항이 생겨서 기존 Button의 이름을 그대로 사용하게 되었는데요..!
그래서 Button 하위에 두기엔 이미 존재하는 Button 컴포넌트가 있어서 애매한 느낌이 있는 것 같습니다.

이름 자체는 매우 비슷하지만, 내부 구현되어 있는 로직이 다르기도 해서 파생상태가 아니다 보니 폴더구조를 어떻게 잡아야할지 현재는 감이 오지 않네요!!

현재는 여러 버튼이 존재하기보다는 기존의 Button으로 디자인이 충분히 가능할 것이라는 생각도 들어서 더이상 Button 관련된 컴포넌트가 생길까 싶기도 한 생각이 듭니다!

기존의 Button 컴포넌트를 유지하면서 다른 좋은 의견이 혹시 있으실까요~? 😭

현재 구현한 Button(=index.tsx)을 BaseButton으로 네이밍을 변경하고

ToggleButton을 Button 디렉터리 내로 이동시키는건 어떨까요?

지금 구조는 Button 속성을 띈 컴포넌트들이 흩어져있다는 느낌이 들어요🥲

@Largopie

Yoonkyoungme · 2024-08-05T16:33:26Z

++ 사소하지만 Button 스토리북에서 CustomText 버튼의 텍스트가 길어서 버튼을 벗어나네요 😭

hwinkr

토글 버튼 만든다고 고생했씁니다 낙타!!!

간단한 코멘트 달았는데, RC를 날릴까 하다가 사소한 것이라서 반영해도, 안해도 크게 문제가 되지 않을 것이라 판단해 코멘트로 요청합니다 ㅎㅎ

hwinkr · 2024-08-06T00:16:54Z

frontend/src/components/_common/Header/Header.styles.ts

-  align-items: center;
  gap: 1.2rem;
+  align-items: center;
+


[Comment]

린트가 적용이 안되다가, typography 관련 문제가 해결되어 다시 적용된 것일까요~?

그것은 다른 컨텍스트인데, 명령어로 npm run lint:css입력했더니 요게 적용 안되있었는지 적용되었습니다! 추가로 적용했어요! 🙌

hwinkr · 2024-08-06T00:30:18Z

frontend/src/components/_common/ToggleButton/index.tsx

+    <div css={s_container}>
+      {children}
+      <label htmlFor={id} onClick={onClick} css={s_buttonContainer(isClicked)}>
+        <span css={s_switch}>선택</span>


[Q]

선택이라는 텍스트의 스타일이 display : none인데, 보이지 않게 하는 요소에 텍스트를 추가한 이유가 있을까요~? 웹 접근성을 위해서라면, aria-label등 html 어트리뷰트를 활용하면 좋을 것 같아요!

먼저 좋은 질문 감사합니다 해리 👍

저번에 하루와 aria-label과 일반 label에 대해서 대화를 나눈 기억이 있었는데, aria-label은 일부 스크린 리더에는 지원이 안된다고 하셨던 기억이 나서 관성적으로 사용을 안했던 것 같네요 😅. 직접 확인해보고자 찾아봤는데 아직 관련 레퍼런스는 찾지 못했습니다.

그래서 구글 스크린리더 익스텐션을 설치해서 테스트해봤는데 정상적으로 잘 동작하네요! aria-label로 이용하면 좋을 것 같습니다!

깊게 연관된 것은 아니지만 관련 자료 함께 남겨드려요!
https://bootcamp.uxdesign.cc/when-to-use-aria-label-or-screen-reader-only-text-cd778627b43b

탐구 정신 미쳤다 송재석...

hwinkr · 2024-08-06T00:31:34Z

frontend/src/components/_common/ToggleButton/index.tsx

+      <label htmlFor={id} onClick={onClick} css={s_buttonContainer(isClicked)}>
+        <span css={s_switch}>선택</span>
+      </label>
+      <input type="checkbox" id={id} checked={isClicked} css={s_input} />


[Comment]

토글 버튼은 input 태그를 활용해서 만들 수 있겠군요... 한 번도 만들어보지 않은 컴포넌트라서 인풋 태그를 사용해서 만든다는 것을 처음 알았네요...! 웹 접근성을 위해서 해당 태그를 활용한다고 보면 될까요?? (궁금궁금)

원래는 저도 button 태그로 이용해보려 생각을 했습니다!
그런데 checkbox로 이용하게 되면 어차피 checked 프로퍼티는 항상 true, false값을 가지기 때문에 해당 체크박스로 구현하는 것이 더욱 좋을거라고 판단했습니다 :)

특별하게 웹 접근성을 고려한 것은 아니고 취향차이인 것 같아요 :)

hwinkr · 2024-08-06T00:32:38Z

frontend/src/components/_common/ToggleButton/ToggleButton.stories.tsx

+export const False: Story = {
+  args: {
+    id: 'toggle-button',
+    isClicked: false,
+    onClick: () => {},
+  },
+};
+
+export const True: Story = {
+  args: {
+    id: 'toggle-true-button',
+    isClicked: true,
+    onClick: () => {},
+  },
+};


[P3]

False, True로는 어떤 UI가 그려질지 예상하기 힘든 것 같습니다...!

그래서 스토리 이름을 Toggled, UnToggled로 변경해보는 것은 어떨까요~? :)

너무 좋습니다! 제안 감사합니다~! ☺️

hwinkr · 2024-08-06T00:33:03Z

frontend/src/components/_common/ToggleButton/ToggleButton.stories.tsx

+  title: 'Components/ToggleButton',
+  component: ToggleButton,
+  tags: ['autodocs'],
+  decorators: [


[Comment]

데코레이터 활용 좋습니다!!

Largopie

수정사항 반영해서 답글 남겼습니다 :) 확인 후 재검토 부탁드려요~ 🐫

Largopie · 2024-08-06T06:38:23Z

frontend/src/components/_common/Header/Header.styles.ts

-  align-items: center;
  gap: 1.2rem;
+  align-items: center;
+


그것은 다른 컨텍스트인데, 명령어로 npm run lint:css입력했더니 요게 적용 안되있었는지 적용되었습니다! 추가로 적용했어요! 🙌

Largopie · 2024-08-06T06:41:18Z

frontend/src/components/_common/ToggleButton/ToggleButton.stories.tsx

+export const False: Story = {
+  args: {
+    id: 'toggle-button',
+    isClicked: false,
+    onClick: () => {},
+  },
+};
+
+export const True: Story = {
+  args: {
+    id: 'toggle-true-button',
+    isClicked: true,
+    onClick: () => {},
+  },
+};


너무 좋습니다! 제안 감사합니다~! ☺️

Largopie · 2024-08-06T06:45:37Z

frontend/src/components/_common/ToggleButton/ToggleButton.styles.ts

+
+export const s_input = css`
+  position: absolute;
+  left: -1000%;


맞아요~! 해당 토글 버튼은 label에서 작용하도록 구현했어요!
빙봉 말대로 주석으로 간략한 설명을 남겨두면 더욱 좋겠네요! 의견 감사해요~! 🙌

Largopie · 2024-08-06T06:48:02Z

frontend/src/components/_common/ToggleButton/index.tsx

+
+interface ToggleButtonProps {
+  id: string;
+  isClicked: boolean;


좋습니다! 이걸로 하면 더욱 직관적이겠네요!

Largopie · 2024-08-06T07:41:09Z

frontend/src/components/_common/ToggleButton/index.tsx

+    <div css={s_container}>
+      {children}
+      <label htmlFor={id} onClick={onClick} css={s_buttonContainer(isClicked)}>
+        <span css={s_switch}>선택</span>


먼저 좋은 질문 감사합니다 해리 👍

저번에 하루와 aria-label과 일반 label에 대해서 대화를 나눈 기억이 있었는데, aria-label은 일부 스크린 리더에는 지원이 안된다고 하셨던 기억이 나서 관성적으로 사용을 안했던 것 같네요 😅. 직접 확인해보고자 찾아봤는데 아직 관련 레퍼런스는 찾지 못했습니다.

그래서 구글 스크린리더 익스텐션을 설치해서 테스트해봤는데 정상적으로 잘 동작하네요! aria-label로 이용하면 좋을 것 같습니다!

깊게 연관된 것은 아니지만 관련 자료 함께 남겨드려요!
https://bootcamp.uxdesign.cc/when-to-use-aria-label-or-screen-reader-only-text-cd778627b43b

Largopie · 2024-08-06T07:49:00Z

frontend/src/components/_common/ToggleButton/index.tsx

+      <label htmlFor={id} onClick={onClick} css={s_buttonContainer(isClicked)}>
+        <span css={s_switch}>선택</span>
+      </label>
+      <input type="checkbox" id={id} checked={isClicked} css={s_input} />


원래는 저도 button 태그로 이용해보려 생각을 했습니다!
그런데 checkbox로 이용하게 되면 어차피 checked 프로퍼티는 항상 true, false값을 가지기 때문에 해당 체크박스로 구현하는 것이 더욱 좋을거라고 판단했습니다 :)

특별하게 웹 접근성을 고려한 것은 아니고 취향차이인 것 같아요 :)

Largopie · 2024-08-06T07:54:57Z

frontend/src/components/_common/ToggleButton/index.tsx

현재 공통 컴포넌트로 구현한 버튼이 수정사항이 생겨서 기존 Button의 이름을 그대로 사용하게 되었는데요..!
그래서 Button 하위에 두기엔 이미 존재하는 Button 컴포넌트가 있어서 애매한 느낌이 있는 것 같습니다.

이름 자체는 매우 비슷하지만, 내부 구현되어 있는 로직이 다르기도 해서 파생상태가 아니다 보니 폴더구조를 어떻게 잡아야할지 현재는 감이 오지 않네요!!

현재는 여러 버튼이 존재하기보다는 기존의 Button으로 디자인이 충분히 가능할 것이라는 생각도 들어서 더이상 Button 관련된 컴포넌트가 생길까 싶기도 한 생각이 듭니다!

기존의 Button 컴포넌트를 유지하면서 다른 좋은 의견이 혹시 있으실까요~? 😭

hwinkr

리뷰 반영 고생하셨습닏~~

hwinkr · 2024-08-06T09:22:16Z

frontend/src/components/_common/ToggleButton/ToggleButton.stories.tsx

@@ -30,25 +30,25 @@ export default meta;

 type Story = StoryObj<typeof meta>;

-export const False: Story = {
+export const UnToggled: Story = {


👍👍👍

Yoonkyoungme · 2024-08-06T13:49:15Z

frontend/src/components/_common/ToggleButton/index.tsx

현재 구현한 Button(=index.tsx)을 BaseButton으로 네이밍을 변경하고

ToggleButton을 Button 디렉터리 내로 이동시키는건 어떨까요?

지금 구조는 Button 속성을 띈 컴포넌트들이 흩어져있다는 느낌이 들어요🥲

@Largopie

Largopie added 🐈 프론트엔드 프론트엔드 관련 이슈에요 :) 🚀 기능 기능을 개발해요 :) 🎨 디자인 디자인을 해요 :) labels Aug 5, 2024

Largopie added this to the 3차 데모데이 milestone Aug 5, 2024

Largopie self-assigned this Aug 5, 2024

Largopie requested review from hwinkr and Yoonkyoungme as code owners August 5, 2024 06:38

Yoonkyoungme requested changes Aug 5, 2024

View reviewed changes

hwinkr reviewed Aug 6, 2024

View reviewed changes

Largopie commented Aug 6, 2024

View reviewed changes

Yoonkyoungme mentioned this pull request Aug 6, 2024

[FE] 특정 테이블 셀에 마우스를 올리거나, 클릭하면 해당 셀을 선택한 약속 참여자들을 확인할 수 있는 툴팁 컴포넌트 구현 #156

Merged

hwinkr approved these changes Aug 6, 2024

View reviewed changes

Yoonkyoungme approved these changes Aug 6, 2024

View reviewed changes

Largopie added 8 commits August 7, 2024 10:02

style: 헤더 스타일 파일 css 프로퍼티 정렬

d637af8

feat: 토글 버튼 구현

02057b5

feat: 토글 버튼 스토리북 작성

48f7c42

refactor: 스토리북 버튼을 나타내는 이름 수정

4d644ef

refactor: isClicked -> isToggled로 상태값 네이밍 수정

d70bb1e

refactor: label 태그 텍스트 aria-label로 변경

3498e2e

comment: style 파일 주석 추가

299a3ae

refactor: Buttons 하위 폴더로 동작하도록 구조 수정

51d19f4

Largopie force-pushed the feat/155-toggle-button branch from e2dd173 to 51d19f4 Compare August 7, 2024 01:03

Largopie merged commit dfc17d7 into develop Aug 7, 2024
5 checks passed

Largopie deleted the feat/155-toggle-button branch August 7, 2024 04:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FE] 공용 토글 버튼 컴포넌트 구현 #158

[FE] 공용 토글 버튼 컴포넌트 구현 #158

Largopie commented Aug 5, 2024

github-actions bot commented Aug 5, 2024 •

edited

Loading

Yoonkyoungme left a comment

Yoonkyoungme Aug 5, 2024 •

edited

Loading

Largopie Aug 6, 2024

Yoonkyoungme Aug 5, 2024

Largopie Aug 6, 2024

Yoonkyoungme Aug 5, 2024 •

edited

Loading

Largopie Aug 6, 2024

Yoonkyoungme Aug 6, 2024

Yoonkyoungme commented Aug 5, 2024

hwinkr left a comment •

edited

Loading

hwinkr Aug 6, 2024

Largopie Aug 6, 2024

hwinkr Aug 6, 2024

Largopie Aug 6, 2024

hwinkr Aug 6, 2024

hwinkr Aug 6, 2024

Largopie Aug 6, 2024

hwinkr Aug 6, 2024

Largopie Aug 6, 2024

hwinkr Aug 6, 2024

Largopie left a comment

Largopie Aug 6, 2024

Largopie Aug 6, 2024

Largopie Aug 6, 2024

Largopie Aug 6, 2024

Largopie Aug 6, 2024

Largopie Aug 6, 2024

Largopie Aug 6, 2024

hwinkr left a comment

hwinkr Aug 6, 2024

Yoonkyoungme Aug 6, 2024

[FE] 공용 토글 버튼 컴포넌트 구현 #158

[FE] 공용 토글 버튼 컴포넌트 구현 #158

Conversation

Largopie commented Aug 5, 2024

관련 이슈

작업 내용

특이 사항

리뷰 요구사항 (선택)

github-actions bot commented Aug 5, 2024 • edited Loading

Test Results

Yoonkyoungme left a comment

Choose a reason for hiding this comment

Yoonkyoungme Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoonkyoungme Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yoonkyoungme commented Aug 5, 2024

hwinkr left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Largopie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hwinkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 5, 2024 •

edited

Loading

Yoonkyoungme Aug 5, 2024 •

edited

Loading

Yoonkyoungme Aug 5, 2024 •

edited

Loading

hwinkr left a comment •

edited

Loading