feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. #77

innocarpe · 2023-09-07T16:16:28Z

📌 이슈 링크

close 🌟 FEATURE: [2 Week] 메인 페이지를 개발합니다. #47

📖 작업 배경

스타일링 쪽을 해봐야 할 듯해서, 찾아보니 emotion 을 제일 범용적으로 쓰는 듯해 기존 css 코드를 이걸로 대체하는 작업을 해 봅니다.

🛠️ 구현 내용

emotion 의존성을 추가합니다.
공용 배너 컴포넌트와 Container 컴포넌트를 emotion 기반으로 작성합니다.
- 위치가 고민되었었는데 컴포넌트 폴더 하위에 home/ 같이 페이지 별로 나뉘다 보니, shared/ 폴더를 만들어서 관리하려고 하는데 좋은 방향일까 모르겠네요 🤔
위 작업을 하고 보니 기존 공용 컴포넌트도 같이 옮겨가는게 좋을 듯해 shared/ 폴더로 옮깁니다.
HomeBanner 를 styled 기반으로 변경하고 관련 css 코드를 제거합니다.
HomePageContainer 를 styled 기반으로 변경하고 관련 css 코드를 제거합니다.

💡 참고사항

feat(app, home): 홈페이지 내부를 React 컴포넌트로 분리하고, 공용 레이아웃으로 감싸도록 페이지 내부를 리팩토링합니다. #74 의 커밋이 포함되어 있는 체이닝 PR입니다.
styled component 를 만드는 Best Practice 는 아직 잘 모르겠어서, 적당적당히 나누어 보았습니다. 부모 컴포넌트를 만드는게 맞는 방향일까 싶은데 기존 css 가 그렇게 되어 있는지라 일단은 그대로 따라갔습니다.

🖼️ 스크린샷

기존과 동일하게 스타일이 잘 표시되고 있는 상태

InSeong-So

우성님 고생하셨습니다 🙌
코멘트 남겼는데 도움이 되면 좋겠네요!

InSeong-So · 2023-09-09T08:23:30Z

apps/react-world/src/components/home/HomeBanner.tsx

+
 export const HomeBanner: React.FC = () => {
  return (


React.FC를 사용하지 말라는 여러가지 커뮤니티의 의견이 있는데요,

function 기명식 컴포넌트 선언에서는 타이핑 함수를 사용할 수가 없습니다.

항상 children을 가질 수 있으므로 인식의 차이가 발생할 수 있습니다.

React에서 제공하는 defaultProps를 사용할 수 없게 합니다.

더 자세한 내용을 보고 싶다면 이 블로그를 참조하면 좋을 것 같습니다!

코멘트 감사합니다! 블로그 포함해서 관련 내용은 더 살펴봐야겠지만 우선은 안 쓰는 방향으로 반영해나갈게요!

이후 PR에서 반영했습니다!

InSeong-So · 2023-09-09T08:25:32Z

apps/react-world/src/components/shared/Container.tsx

+  @media (min-width: 544px) {
+    max-width: 576px;
+  }
+
+  @media (min-width: 768px) {
+    max-width: 720px;
+  }
+
+  @media (min-width: 992px) {
+    max-width: 940px;
+  }
+
+  @media (min-width: 1200px) {
+    max-width: 1140px;
+  }


이렇게 반응형 미디어 쿼리를 여러 곳에서 사용하게 되면 아래와 같은 상황이 헷갈리게 됩니다.

데스크탑 우선 vs 모바일 우선: min-width / max-width 차이

px 기준

그래서 보통은, 디자인 토큰이나 상수로 width 값을 관리하게 됩니다.
추후에는 분리해도 좋을 것 같네요!

이렇게 반응형 미디어 쿼리를 여러 곳에서 사용하게 되면 아래와 같은 상황이 헷갈리게 됩니다.

데스크탑 우선 vs 모바일 우선: min-width / max-width 차이

px 기준

그래서 보통은, 디자인 토큰이나 상수로 width 값을 관리하게 됩니다. 추후에는 분리해도 좋을 것 같네요!

오 그렇군요 이 부분은 향후 더 좋은 방향을 접하는대로 조금씩 개선해 나가겠습니다!

InSeong-So · 2023-09-09T08:28:32Z

apps/react-world/src/components/shared/Layout.tsx

+import { Navbar } from './NavBar';
+import { Footer } from './Footer';


named export를 활용한다면 컴포넌트 export/import를 정말 직관적으로 바꿀 수 있습니다.
현재 프로젝트 코드 내에서 제가 추천하는 방향은 아래와 같습니다.

- Layout - index.ts - Layout.tsx - NavBar.tsx - Footer.tsx

Layout/index.ts : 새로 생성합니다. 기존 Layout.tsx는 그대로 두시면 돼요.

export * from './Layout.tsx';

NavBar, Footer도 레이아웃 하위에 포함을 시키느냐? 시키지 않느냐는 내부 컨텐츠를 주입하게 할 건지, 혹은 갖고 있을 건지 선택해서 설계하시면 되겠습니다 🙌

@InSeong-So

내부 컨텐츠를 주입하게 할 건지, 혹은 갖고 있을 건지 선택해서 설계하시면 되겠습니다 🙌

현재 UI를 보았을 때에는 변화되는 지점이 없긴 해서 (있다면 네비바 우측 상단 눌렀을 때 눌러진 상태를 변경하는 부분인데 여기만 내부적으로 처리하는 게 문제가 없다고 한다면) 주입하지 않고 내부에서 처리하도록 해도 될 것 같은데 이렇게 간다면 Layout 하위에 포함하도록 가면 될까요~?

우선 Layout 폴더 안에 전부 넣는 쪽으로 수정했습니다!

InSeong-So · 2023-09-09T08:29:23Z

apps/react-world/src/components/shared/Layout.tsx

 type LayoutProps = PropsWithChildren;



제가 급하게 작성해서 이러한 코드 뭉치를 그대로 두었는데, 이러한 타입 alias 기법은 제3자로 하여금 그 의도를 흐리게 만들기 때문에 가급적 사용하지 않는게 좋긴 합니다🥲

Suggested change

type LayoutProps = PropsWithChildren;

type LayoutProps = PropsWithChildren<{}>;

차라리 이렇게 하거나...

Suggested change

type LayoutProps = PropsWithChildren;

type LayoutProps = {

children?: ReactNode;

}

이렇게 하는 것을 추천드립니다 💪

오 코멘트 감사합니다 저는 고르라면 후자쪽을 고를 것 같습니다 ㅎㅎ

-> 후자쪽으로 수정했습니다!

InSeong-So · 2023-09-09T08:30:55Z

apps/react-world/src/components/shared/banner/Banner.tsx

+export const Banner = styled.div`
+  color: #fff;
+  background: #333;
+  padding: 2rem;
+  margin-bottom: 2rem;
+`;


현재 Banner 관련 속성이 이렇게 나뉘어져 있는데요,

Banner.tsx

BannerTitle.tsx

apps/react-world/src/components/shared/banner/Banner.styled.ts 로 파일을 하나 만들어서... 분리되어 있는 스타일 구문을 합치는 것이 좀 더 직관적일 것 같은데요, 우성님께서는 어떻게 생각하실까요?

Suggested change

export const Banner = styled.div`

color: #fff;

background: #333;

padding: 2rem;

margin-bottom: 2rem;

`;

// Banner.styled.ts

import styled from '@emotion/styled';

export const Banner = styled.div`

color: #fff;

background: #333;

padding: 2rem;

margin-bottom: 2rem;

`;

export const BannerTitle = styled.h1`

text-shadow: 0px 1px 3px rgba(0, 0, 0, 0.3);

margin-bottom: 0px;

`;

.styled.ts 방식으로 스타일을 모아두는 형태도 가능하군요..! 스타일 코드가 많지 않아서 이 방향 좋은 것 같습니다 🙌

@InSeong-So 말씀대로 한다면 banner 폴더 안에 기존 파일 두 개는 지워지고 스타일 파일 Banner.styled.ts(x?) 파일 하나만 남도록 하면 되는 것 맞을까요? 폴더 안에 컴포넌트 파일이 따로 없고 styled 파일 안에 styled component 선언만 있는게 원래 자연스러운 방식인가 헷갈려서요 ㅎ

우선 위처럼 했습니다! 하면서 하나 추가로 들었던 궁금증은, HomeBanner 쪽도 컴포넌트 선언과 스타일이 같이 있는데 여기도 동일한 기준일까 싶어서 HomeBanner.tsx 와 HomeBanner.styled.ts 두 개로 분리해보았습니다.

오 저도 파랑의 말씀에 동의해요~! HomeBanner 쪽도 동일한 기준으로 가져가는 것도 좋은 것 같아요!

저는 .ts/.tsx로 분리하는 기준은 그 파일 안에 JSX(Html 태그가 아닌 Element 태그 ex: <HomeBannerTitle /> 등)이 있는지 여부로 판단해요!
styled 파일엔 스타일 선언부만 존재하기에 .ts로 사용되는 것이 맞는 것 같아요!

오 저도 파랑의 말씀에 동의해요~! HomeBanner 쪽도 동일한 기준으로 가져가는 것도 좋은 것 같아요!

저는 .ts/.tsx로 분리하는 기준은 그 파일 안에 JSX(Html 태그가 아닌 Element 태그 ex: <HomeBannerTitle /> 등)이 있는지 여부로 판단해요! styled 파일엔 스타일 선언부만 존재하기에 .ts로 사용되는 것이 맞는 것 같아요!

오 자세한 설명 감사합니다! ㅎㅎ

…도록 함

…하도록 변경

… 를 걸지 않고 children 을 직접 props 에 선언하도록 변경

innocarpe requested a review from InSeong-So September 7, 2023 16:16

innocarpe added the feature A new feature label Sep 7, 2023

innocarpe requested review from ludacirs, areumsheep and limgyumin September 7, 2023 16:18

innocarpe added the chore Updating build tasks, package manager configs, etc; no production code change label Sep 7, 2023

innocarpe force-pushed the carpe/home-extract-styling branch from d2e9f15 to 07d2697 Compare September 8, 2023 13:29

innocarpe mentioned this pull request Sep 8, 2023

feat(app): emotion 기반으로 공용 네비바 로직을 변경합니다. #78

Merged

innocarpe added the status: dependent 이전 PR의 커밋이 포함되어 있어 대기해야 함 label Sep 8, 2023

innocarpe changed the title ~~feat(app, home) emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다.~~ feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. Sep 9, 2023

InSeong-So approved these changes Sep 9, 2023

View reviewed changes

Base automatically changed from carpe/home-extract-component to team6/innocarpe September 9, 2023 08:44

innocarpe removed the status: dependent 이전 PR의 커밋이 포함되어 있어 대기해야 함 label Sep 9, 2023

innocarpe added 6 commits September 9, 2023 22:39

chore(app): emotion 의존성 추가

92e7ed8

chore(app): 공용 컴포넌트를 shared 하위로 이동

240dc71

chore(app): 공용 컴포넌트 Container 작성

625303d

chore(app): 공용 배너 컴포넌트 작성

a42d9dd

feat(home): HomeBanner 를 styled 기반으로 변경

0560aa3

feat(home): HomePageContainer 를 styled 기반으로 변경

8a746b0

innocarpe force-pushed the carpe/home-extract-styling branch from 07d2697 to 8a746b0 Compare September 9, 2023 13:40

innocarpe added 5 commits September 9, 2023 22:54

chore(app): Layout/NavBar/Footer 를 layout/ 폴더 안으로 이동

b3ab47e

chore(app): layout/ 폴더 안에 index.ts 을 작성하고 Layout 내부 의존성들을 전부 export 하…

9e145ba

…도록 함

fix(home): PropsWithChildren 로 alias 를 걸지 않고 children 을 직접 props 에 선언…

61a3dcc

…하도록 변경

fix(app): Banner 관련 스타일 코드를 Banner.styled.tsPropsWithChildren 로 alias…

b13fe19

… 를 걸지 않고 children 을 직접 props 에 선언하도록 변경

fix(home): HomeBanner 에서 스타일 관련 코드를 HomeBanner.styled.ts 로 추출

a637e81

innocarpe merged commit 70679e4 into team6/innocarpe Sep 9, 2023

innocarpe deleted the carpe/home-extract-styling branch September 9, 2023 14:12

innocarpe mentioned this pull request Sep 9, 2023

fix(app): 프로젝트 내에서 React.FC 를 안 쓰도록 리팩토링합니다. #83

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. #77

feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. #77

innocarpe commented Sep 7, 2023

InSeong-So left a comment

InSeong-So Sep 9, 2023

innocarpe Sep 9, 2023

innocarpe Sep 10, 2023

InSeong-So Sep 9, 2023

innocarpe Sep 9, 2023

InSeong-So Sep 9, 2023

innocarpe Sep 9, 2023

innocarpe Sep 9, 2023

InSeong-So Sep 9, 2023

innocarpe Sep 9, 2023 •

edited

Loading

InSeong-So Sep 9, 2023

innocarpe Sep 9, 2023

innocarpe Sep 9, 2023

innocarpe Sep 9, 2023

areumsheep Sep 10, 2023

innocarpe Sep 10, 2023

		import { Navbar } from './NavBar';
		import { Footer } from './Footer';

	type LayoutProps = PropsWithChildren;
	type LayoutProps = PropsWithChildren<{}>;

-type LayoutProps = PropsWithChildren;
+type LayoutProps = {
+  children?: ReactNode;
+}

feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. #77

feat(app, home): emotion 기반으로 홈 배너와 홈 페이지 컨테이너 로직을 변경합니다. #77

Conversation

innocarpe commented Sep 7, 2023

📌 이슈 링크

📖 작업 배경

🛠️ 구현 내용

💡 참고사항

🖼️ 스크린샷

InSeong-So 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

innocarpe Sep 9, 2023 • 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

innocarpe Sep 9, 2023 •

edited

Loading