사이드바 구현 #76

hseoy · 2022-01-01T14:13:20Z

개요

사이드바를 구현했습니다!!

이슈 번호

closes 사이드바 구현 #67

변경사항

IconTabBar 컴포넌트 추가
Sidebar 컴포넌트 추가
SideLayout 컴포넌트 추가
FeedbackMessagesPage 페이지 컴포넌트 추가
IconTabBar & Sidebar 컴포넌트에 대한 스토리북 스토리 추가
- chromatic에서 UI 테스트(추가된 스토리)에 대해 Accept/Deny를 할 수 있는 데 확인하시고 Accept해주시면 넘 좋을 것 같습니다 ㅎ Build 41입니당

특이사항

IconTabBar를 구현할 때 IconTabBar.TabItem 과 같이 사용하도록 구현했는 데 괜찮은지! -> 그림님이 저번 PR comment로 남겨주신 글을 참고했습니다!
- IconTabBar를 구현할 때 추후에 모바일 환경에서 사이드바가 아니라 하단의 탭으로 사용하는 것을 고려해서 top, right, left, bottom 여러 방향으로 바꿀 수 있도록 구현했습니다. 스토리북 참고!
저번에 PR 리뷰 중 Emotion과 ClassName을 같이 사용하면 어떨까하는 생각을 했는 데 이번에 한번 class name을 쫌 적극적으로 사용해봤습니다.
스토리북 생각보다 쉽고 간단하더라고요! 그냥 기본적인 것만 추가해놔도 prop의 값들을 수정할 수 있으니까 PR 리뷰어의 시간을 많이 줄일 수 있지 않을까 싶습니다. 앞으로 열심히... ㅎ
사이드바 하단의 서비스 리스트 드랍다운은 PR 범위가 너무 커지는 감이 있어서 제외하고 구현했습니다. 추후에 채팅 구현하면서 구현하도록 하겠습니다!!
/service-edit 혹은 /feedback URL의 페이지들이 채팅 관련된 페이지들인데 현재 로그인하지 않아도 접근할 수 있습니다. 추후에 채팅 구현하면서 Private로 로그인해야만 접근할 수 있도록 바꾸면 될 것 같고 현재는 테스트 하시기 편하시라고 냅뒀습니다.
스토리북 링크

pumpkiinbell

수고하셨습니다!!!

pumpkiinbell · 2022-01-04T00:43:50Z

src/components/common/IconTabBar/index.tsx

+IconTabBar.TabIcon = TabIcon;
+IconTabBar.TabItem = TabItem;


확실히 Compound Components 를 사용하니 훨씬 수정하기 용이하게 느껴집니다!!
다만 이전에 말씀하신 것처럼 한 모듈에 모든 컴포넌트가 있으니 가독성이 떨어지긴 하네요 ㅠㅠ
TabIcon, TabItem 을 같은 폴더 내의 모듈로 만들고, import 해서 사용하는건 어떨까요...?

Suggested change

IconTabBar.TabIcon = TabIcon;

IconTabBar.TabItem = TabItem;

import TabIcon from "./TabIcon";

import TabItem from "./TabItem";

// ...

IconTabBar.TabIcon = TabIcon;

IconTabBar.TabItem = TabItem;

좋습니당

pumpkiinbell · 2022-01-04T00:47:12Z

src/components/common/IconTabBar/index.tsx

+            css={tabIconStyle}
+            className={`tab-icon ${selected && selectable ? 'selected' : ''}`}
+          >
+            <img src={src} alt={children} />


alt 를 alt 같은 prop 이 아닌 children prop 으로 받는 이유가 궁금합니다
약간 직관적이지 않다고 느껴져서요 😅

넹 제 생각도 children으로 안 받고 이미지 alt를 이 이미지 역할 정도로만 정의해줘도 될 것 같아요!

TabIcon이라는 네이밍에서 이것은 아이콘만을 보여주는 컴포넌트라는 것을 드러냈다면 굳이 alt와 같이 이미지 태그스러운 속성을 사용할 필요가 있을까 했습니다. 그래서 children으로 받아 일종의 숨김을 사용한 것이었는 데 두 분의 의견이 직관적이지 않다로 일치하신 것 같아 alt 속성을 사용한 형태로 수정수정,,,,

pumpkiinbell · 2022-01-04T00:50:48Z

src/components/common/IconTabBar/style.ts

+    }
+  }
+
+  &.right .curved-block-wrap {


개인적으로 right, bottom className 을 봤을 때 오른쪽, 하단에 TabBar 가 있을 거라고 예상했는데,
아래 그림처럼 각각 상단, 왼쪽에 위치하고 내부적인 UI 요소 위치만 변경되더라고요!!
혹시 의도하신 걸까요 ??

이와 별개로 특징이 드러나는 스타일 요소들을 함수로 만들어줘서 가독성이 좋게 느껴지네요!! 👍🏻

각각 상단, 왼쪽에 위치하고 내부적인 UI 요소 위치만 변경되더라고요!!
혹시 의도하신 걸까요 ??

TabBar의 배치 자체는 어차피 상위 컴포넌트에서 신경을 써야 하는 문제이기 때문에 선택했을 때 파란색 선택 배경의 방향만 이동을 하도록 해두었습니다.

pumpkiinbell · 2022-01-04T00:54:58Z

src/components/layout/SideLayout/index.tsx

+          <div className="body">{renderSidebar && renderSidebar()}</div>
+          <div className="footer">


body, footer 가 semantic 한 요소들이란 통념이 있어서 그런지 className 으로 다소 어색하게 느껴지네요...!! 혹시 해당 네이밍으로 하신 이유를 말씀해주실 수 있을까요??

요 body, footer 부분을 Sidebar 컴포넌트 안이 아닌 SideLayout에 정의한 이유가 있을까요?
Sidebar 안에서는 head, body가 있고 그 body 안에 children을 넣던데, 그게 또 SideLayout에서 정의되어서 body, footer를 가지니 뭔가 구조가 확실히 들어오지 않는 것 같아서요!!
renderSidebar 라는 함수도 그 역할이 와닿지 않는 것 같아요!

요 body, footer 부분을 Sidebar 컴포넌트 안이 아닌 SideLayout에 정의한 이유가 있을까요?

SideBar의 children은 사이드바의 오른쪽 파란색 부분에 렌더링 되는 부분인데 요걸 body, footer 요런 식으로 구분을 하게 되면 Body와 Footer에 각각 다른 내용을 렌더링해야 할 때 어떻게 하지...? 라는 의문이 들었습니다. 서비스 드랍다운 리스트를 사이드바 컴포넌트로 넣어서 children에는 renderSidebar()만 렌더링할 수도 있겠지만 단순히 사이드바에 서비스 리스트라는 개념이 들어가는 게 옳지 않다고 느꼈습니다. 그래서 head에 대한 내용만 Sidebar쪽에 넣고 그 외적인 요소들은 children으로 받도록 처리를 해줬습니다.

renderSidebar 라는 함수도 그 역할이 와닿지 않는 것 같아요!

사이드바의 내용을 렌더링한다라는 의미로 renderSidebar라는 네이밍을 사용한 것이고 children외에 다른 위치에 렌더링할 때 render props를 사용하는 것을 참고해서 renderSidebar라는 prop을 사용한 것인데 참고 구체적으로 어떤 부분이 문제가 되는 것일까요? 더 설명해주시면 감사하겠습니다

body, footer 가 semantic 한 요소들이란 통념이 있어서 그런지 className 으로 다소 어색하게 느껴지네요...!! 혹시 해당 네이밍으로 하신 이유를 말씀해주실 수 있을까요??

으음,,,,, 원래 className을 사용한다고 하면 중복을 피하기 위해 sidebar-body, sidebar-footer로 사용할 텐데, 그럴 필요가 없으니 단순히 body, footer로 사용한 거죠. semantic한 요소들이다라고 생각하시는 이유가 body, footer라는 이름의 HTML 태그가 있기 때문이라고 생각됩니다. 근데 className 에까지 그러한 제약을 걸어둘 필요가 있을까 싶어서 body, footer라는 네이밍을 그대로 사용했던 거였는 데 그림님도 그렇고 요런 네이밍 때문에 혼란이 있는 것 같네요.

근데 그러면 어떤 네이밍을 사용해야 할까요?

Seogeurim

우와.... css 코드 완전 화려하고 멋있네용 !!
테스트 코드 통해서 컴포넌트 코드 구조 이해하고 스토리북으로 UI 확인하고..! 테스트 너무 좋습니다 ㅠㅠ!!!!!
몇가지 코멘트 남겨보았어요 !!! 👍🏻🤗

Seogeurim · 2022-01-08T06:20:40Z

src/App.tsx

+        {/* 
+          @TODO PrivateRoute로 변경
+            요 두 개의 페이지는 원래 Private으로 가야 하지만 아직은 테스트 용이성을 위해 Route로 남깁니다.
+            추후 채팅을 구현할 때 PrivateRoute로 변경합시다!
+         */}
        <Route path={Uri.serviceEdit} exact component={ServiceEditPage} />


👍🏻👍🏻 ServiceEditPage 라우터는 나중에 올라올 서비스 수정 API 연동 PR에서 수정되었습니닷!!!

Seogeurim · 2022-01-08T06:22:00Z

src/assets/images/setting-icon.svg

@@ -0,0 +1,3 @@
+<svg width="34" height="36" viewBox="0 0 34 36" fill="none" xmlns="http://www.w3.org/2000/svg">
+<path d="M33.125 22.4414L30.5664 20.2539C30.6875 19.5117 30.75 18.7539 30.75 17.9961C30.75 17.2383 30.6875 16.4805 30.5664 15.7383L33.125 13.5508C33.318 13.3856 33.4561 13.1655 33.521 12.9199C33.5859 12.6743 33.5745 12.4148 33.4883 12.1758L33.4531 12.0742C32.7487 10.1056 31.694 8.28061 30.3398 6.6875L30.2695 6.60547C30.1053 6.41231 29.8863 6.27346 29.6416 6.20722C29.3968 6.14097 29.1377 6.15044 28.8984 6.23438L25.7227 7.36328C24.5508 6.40234 23.2422 5.64453 21.8281 5.11328L21.2148 1.79297C21.1686 1.54313 21.0474 1.31329 20.8674 1.13398C20.6874 0.954661 20.457 0.834362 20.207 0.789063L20.1016 0.769531C18.0664 0.402344 15.9258 0.402344 13.8906 0.769531L13.7852 0.789063C13.5351 0.834362 13.3048 0.954661 13.1248 1.13398C12.9448 1.31329 12.8236 1.54313 12.7773 1.79297L12.1602 5.12891C10.7574 5.66027 9.45105 6.41768 8.29298 7.37109L5.09376 6.23438C4.85456 6.14977 4.59528 6.13996 4.35038 6.20625C4.10547 6.27253 3.88654 6.41178 3.72266 6.60547L3.65235 6.6875C2.2998 8.28173 1.24523 10.1064 0.53907 12.0742L0.503914 12.1758C0.328133 12.6641 0.472664 13.2109 0.867195 13.5508L3.45704 15.7617C3.33594 16.4961 3.27735 17.2461 3.27735 17.9922C3.27735 18.7422 3.33594 19.4922 3.45704 20.2227L0.867195 22.4336C0.67421 22.5988 0.536088 22.8188 0.471195 23.0645C0.406303 23.3101 0.417715 23.5696 0.503914 23.8086L0.53907 23.9102C1.2461 25.8789 2.29298 27.6953 3.65235 29.2969L3.72266 29.3789C3.88694 29.5721 4.10588 29.7109 4.35064 29.7772C4.5954 29.8434 4.85449 29.8339 5.09376 29.75L8.29298 28.6133C9.45704 29.5703 10.7578 30.3281 12.1602 30.8555L12.7773 34.1914C12.8236 34.4412 12.9448 34.6711 13.1248 34.8504C13.3048 35.0297 13.5351 35.15 13.7852 35.1953L13.8906 35.2148C15.9445 35.584 18.0477 35.584 20.1016 35.2148L20.207 35.1953C20.457 35.15 20.6874 35.0297 20.8674 34.8504C21.0474 34.6711 21.1686 34.4412 21.2148 34.1914L21.8281 30.8711C23.2416 30.3412 24.5576 29.581 25.7227 28.6211L28.8984 29.75C29.1376 29.8346 29.3969 29.8444 29.6418 29.7781C29.8867 29.7118 30.1057 29.5726 30.2695 29.3789L30.3398 29.2969C31.6992 27.6914 32.7461 25.8789 33.4531 23.9102L33.4883 23.8086C33.6641 23.3281 33.5195 22.7813 33.125 22.4414ZM27.793 16.1992C27.8906 16.7891 27.9414 17.3945 27.9414 18C27.9414 18.6055 27.8906 19.2109 27.793 19.8008L27.5352 21.3672L30.4531 23.8633C30.0108 24.8824 29.4524 25.8471 28.7891 26.7383L25.1641 25.4531L23.9375 26.4609C23.0039 27.2266 21.9648 27.8281 20.8398 28.25L19.3516 28.8086L18.6523 32.5977C17.5491 32.7227 16.4353 32.7227 15.332 32.5977L14.6328 28.8008L13.1563 28.2344C12.043 27.8125 11.0078 27.2109 10.082 26.4492L8.85548 25.4375L5.20704 26.7344C4.54298 25.8398 3.98829 24.875 3.54298 23.8594L6.49219 21.3398L6.23829 19.7773C6.14454 19.1953 6.09376 18.5938 6.09376 18C6.09376 17.4023 6.14063 16.8047 6.23829 16.2227L6.49219 14.6602L3.54298 12.1406C3.98438 11.1211 4.54298 10.1602 5.20704 9.26562L8.85548 10.5625L10.082 9.55078C11.0078 8.78906 12.043 8.1875 13.1563 7.76563L14.6367 7.20703L15.3359 3.41016C16.4336 3.28516 17.5547 3.28516 18.6563 3.41016L19.3555 7.19922L20.8438 7.75781C21.9648 8.17969 23.0078 8.78125 23.9414 9.54688L25.168 10.5547L28.793 9.26953C29.457 10.1641 30.0117 11.1289 30.457 12.1445L27.5391 14.6406L27.793 16.1992ZM17 10.7344C13.2031 10.7344 10.125 13.8125 10.125 17.6094C10.125 21.4063 13.2031 24.4844 17 24.4844C20.7969 24.4844 23.875 21.4063 23.875 17.6094C23.875 13.8125 20.7969 10.7344 17 10.7344ZM20.0938 20.7031C19.688 21.1101 19.2058 21.4328 18.6748 21.6526C18.1439 21.8725 17.5747 21.9853 17 21.9844C15.832 21.9844 14.7344 21.5273 13.9063 20.7031C13.4993 20.2974 13.1766 19.8151 12.9567 19.2842C12.7368 18.7532 12.6241 18.1841 12.625 17.6094C12.625 16.4414 13.082 15.3438 13.9063 14.5156C14.7344 13.6875 15.832 13.2344 17 13.2344C18.168 13.2344 19.2656 13.6875 20.0938 14.5156C20.5007 14.9214 20.8234 15.4036 21.0433 15.9346C21.2632 16.4655 21.3759 17.0347 21.375 17.6094C21.375 18.7773 20.918 19.875 20.0938 20.7031Z" fill="#4A7AFF"/>
+</svg>


svg 파일은 assets/icons 만들어서 그 안에 넣는 것이 어떨까요?

Seogeurim · 2022-01-08T06:29:46Z

src/components/nav/Sidebar/index.tsx

+        <IconTabBar.TabIcon src={LogoIcon} to={Uri.service} selectable={false}>
+          Logo
+        </IconTabBar.TabIcon>


요기는 저희 로고가 아니라 현재 선택한 서비스의 로고가 되어야 할 것 같아요!! 이 부분 나중에 API 연동하면서 수정할 수 있도록 TODO로 주석 남겨주실 수 있나요?

으음,,,,,, 좋습니다~~

Seogeurim · 2022-01-08T06:31:18Z

src/components/nav/Sidebar/index.tsx

+      <div css={sidebarContentStyle}>
+        <h1 className="head">
+          {SIDEBAR_HEADINGS[location.pathname] ?? 'Telbby'}
+        </h1>
+        <div className="body">{children}</div>


요기도 head, body 이렇게 주니까 어색하네요!!

Seogeurim · 2022-01-08T06:33:37Z

src/components/nav/Sidebar/index.tsx

+        </IconTabBar.TabIcon>
+        <IconTabBar.TabIcon src={SettingIcon} to={Uri.serviceEdit}>
+          {SIDEBAR_HEADINGS[Uri.serviceEdit]}
+        </IconTabBar.TabIcon>


TabIcon에 children으로 메뉴명을 넘겨주고 있는 부분은 불필요한 부분처럼 느껴지고 마크업 구조상으로도 (아이콘을 렌더링하는 곳인데 메뉴명까지 렌더링한다는 느낌이 들어서) 혼동이 있는 것 같은데, 어떤가요?

요건 children => alt로 변경하면서 해결될 것 같네요. 좋습니다.

Seogeurim · 2022-01-08T06:38:16Z

src/components/layout/SideLayout/index.tsx

+          <div className="body">{renderSidebar && renderSidebar()}</div>
+          <div className="footer">


요 body, footer 부분을 Sidebar 컴포넌트 안이 아닌 SideLayout에 정의한 이유가 있을까요?
Sidebar 안에서는 head, body가 있고 그 body 안에 children을 넣던데, 그게 또 SideLayout에서 정의되어서 body, footer를 가지니 뭔가 구조가 확실히 들어오지 않는 것 같아서요!!
renderSidebar 라는 함수도 그 역할이 와닿지 않는 것 같아요!

Seogeurim · 2022-01-08T06:50:08Z

src/components/common/IconTabBar/index.tsx

+            css={tabIconStyle}
+            className={`tab-icon ${selected && selectable ? 'selected' : ''}`}
+          >
+            <img src={src} alt={children} />


넹 제 생각도 children으로 안 받고 이미지 alt를 이 이미지 역할 정도로만 정의해줘도 될 것 같아요!

hseoy · 2022-01-08T10:26:39Z

리뷰 내용들이 고민해야 할 것들이 많은데, 저번 회의 때 개발 자체는 일단 Stop하고 추후 채팅 스터디가 종료된 이후에 다시 시작하는 걸로 결정된 것으로 알고 있습니다. => 요게 아니었다면 Comment를 달아주세요!!

요 리뷰 반영 자체는 추후에 진행하도록 하겠습니다!!!

hseoy added 18 commits December 31, 2021 16:38

[#67] feat: IconTabNavbar 추가

d891a2a

[#67] chore: sidebar에 필요한 이미지 리소스 추가

ddfa546

[#67] refactor: TabIcon에 대해 TabItem 컴포넌트 prop을 상속하도록 개선

4258290

[#67] test: IconTabNavbar에 대한 컴포넌트 테스트 추가

aece736

[#67] refactor: IconTabNavbar 컴포넌트의 display를 inline-flex로 수정

4d8f828

[#67] refactor: IconTabNavbar를 IconTabBar로 네이밍 변경 및 common 디렉토리로 이동

b9d2294

[#67] feat: Sidebar 컴포넌트 추가

ca1d915

[#67] refactor: Layout 컴포넌트를 layout 디렉토리로 이동

c303edd

[#67] refactor: Sidebar 내용물의 height를 100%로 만들어주기 위한 스타일 변경

10e29c1

[#67] refactor: 사이드바에서 로고 클릭 시 서비스 리스트 페이지로 이동하도록 URL 수정

4d2d607

[#67] feat: SideLayout 컴포넌트 추가

0a8d097

[#67] feat: FeedbackMessagePage 추가

d5ce487

[#67] refactor: 서비스 수정 페이지에서 Layout 컴포넌트 대신 SideLayout 컴포넌트 사용하도록 수정

d01eadd

[#67] test: storybook decorator에 BrowserRouter 컴포넌트 추가

1c3d672

[#67] test: storybook 분류를 Page 대신 Components 사용

278ffbe

[#67] test: IconTabBar storybook story 추가

c1cfb8e

[#67] refactor: Page.stories.tsx를 Jumbotron.stories.tsx로 네이밍 변경

3d843aa

[#67] test: Sidebar storybook story 추가

d6c9cf6

hseoy added the feature New feature or request label Jan 1, 2022

hseoy requested review from pumpkiinbell, Seogeurim and jiyong1 January 1, 2022 14:13

hseoy self-assigned this Jan 1, 2022

pumpkiinbell approved these changes Jan 4, 2022

View reviewed changes

Seogeurim reviewed Jan 8, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

사이드바 구현 #76

사이드바 구현 #76

hseoy commented Jan 1, 2022 •

edited

Loading

pumpkiinbell left a comment

pumpkiinbell Jan 4, 2022

hseoy Jan 8, 2022

pumpkiinbell Jan 4, 2022

Seogeurim Jan 8, 2022

hseoy Jan 8, 2022

pumpkiinbell Jan 4, 2022

pumpkiinbell Jan 4, 2022

hseoy Jan 8, 2022

pumpkiinbell Jan 4, 2022

Seogeurim Jan 8, 2022

hseoy Jan 8, 2022 •

edited

Loading

Seogeurim left a comment

Seogeurim Jan 8, 2022

Seogeurim Jan 8, 2022

Seogeurim Jan 8, 2022

hseoy Jan 8, 2022

Seogeurim Jan 8, 2022

Seogeurim Jan 8, 2022

hseoy Jan 8, 2022

Seogeurim Jan 8, 2022

Seogeurim Jan 8, 2022

hseoy commented Jan 8, 2022

		<div className="body">{renderSidebar && renderSidebar()}</div>
		<div className="footer">

사이드바 구현 #76

Are you sure you want to change the base?

사이드바 구현 #76

Conversation

hseoy commented Jan 1, 2022 • edited Loading

개요

이슈 번호

변경사항

특이사항

pumpkiinbell 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hseoy Jan 8, 2022 • edited Loading

Choose a reason for hiding this comment

Seogeurim 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hseoy commented Jan 8, 2022

hseoy commented Jan 1, 2022 •

edited

Loading

hseoy Jan 8, 2022 •

edited

Loading