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

[ Design ] JuniorPromise 시간 선택 부분 퍼블리싱 #31

Merged
merged 13 commits into from
Jul 9, 2024
4 changes: 4 additions & 0 deletions src/assets/svgs/ic_check.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/assets/svgs/index.tsx
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
// 명명규칙은 xxxxIc으로 통일
// export { default as DefaultProfileIc } from './defaultProfileIc.svg?react';
export { default as ButtonCheckIc } from './ic_check.svg';
17 changes: 16 additions & 1 deletion src/pages/juniorPromise/JuniorPromisePage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
import styled from '@emotion/styled';
import TimeSelectionButton from './components/TimeSelectionButton';
import TimeSelectionTitleWrapper from './components/TimeSelectionTitleWrapper';

const JuniorPromisePage = () => {
return <div>JuniorPromise</div>;
return (
<TimeSelectionContainer>
<TimeSelectionTitleWrapper />
<TimeSelectionButton />
</TimeSelectionContainer>
Comment on lines +7 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

P3) 여기도 마찬가지로 컴포넌트 네이밍 규칙을 지키면 좋을 거 같아요!
컴포넌트 네이밍 규칙 : Wrapper → Layout → Container → Box 순서로 내리기

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영했습니당 👍

);
};

export default JuniorPromisePage;

const TimeSelectionContainer = styled.div`
Copy link
Collaborator

Choose a reason for hiding this comment

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

p3) 여기도 아까 말로 설명해준 것 처럼 전체 페이지 wrapper에 padding 2rem 적용해서 전체 레이아웃 잡아주고 내부 값들의 width를 100%로 조정해보는 방법 사용하면 좋을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그럼 코드 작성을 줄이고 유지보수에도 좋을거 같네요! 반영했씁니다!

display: flex;
flex-direction: column;
gap: 2rem;
`;
65 changes: 65 additions & 0 deletions src/pages/juniorPromise/components/TimeSelectionButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import styled from '@emotion/styled';
import { ButtonCheckIc } from '../../../assets/svgs';
import { TIME_SELECTION_TITLE } from '../constants/constants';

function TimeSelectionButton() {
const buttonValue = '7월 8일 13:00 ~ 13:20';
// const buttonValue = null;

return (
<Container>
{TIME_SELECTION_TITLE.map((item, index) => (
<Wrapper key={index} isActive={buttonValue !== null}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) map을 돌릴 때 각각의 key값은 고유해야해요! 해당 key값을 보고 해당 요소를 삭제하거나 추가하는 동작을하게 되기 때문인데요! 리액트의 렌더링 방식에서 리스트의 인덱스는 변경될 수 있기 때문에 key 값을 index로 주면 예기치 않은 오류를 발생시킬 수 있어요 !
때문에 저는 보통 key값을 부여할 때는 서버에서 온 데이터에 있는 id값 혹은 name과 같이 고유한 값으로 지정하곤 합니다.

아래는 공식문서 캡쳐본이에요!

스크린샷 2024-07-08 오후 11 50 48

key값으로 index 쓰는 것을 지양해야 하는 이유

이러한 사항 반영해서 key값 다시 부여하면 좋을 것 같습니다~

Copy link
Contributor

Choose a reason for hiding this comment

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

P1) 엇! 저도 이부분 왕초보스터디에서 공부했었어서..!! 관련해서 간단하게 정리했던거 남겨 봅니다..!! 하핳.,,

☑️ React에서 key가 필요한 이유

: 리액트는 상태가 바뀌면 이전 상태의 트리와 이후 상태의 트리를 비교한다. 이때 키값이 같다면 동일한 요소라고 판단하고 업데이트를 진행하지 않는다.

☑️ 단순히 index를 쓰는 것이 왜 안좋은지

: index를 사용하는 경우 배열의 순서가 변경될때 불필요하게 전체 다 렌더링되는 현상이 발생한다.

☑️ 어떤 값을 key로 사용하는게 좋은지

→ 배열의 순서나 내용이 변경되지 않는다면 아무 key나 사용해도 된다.

→ 순서나 내용이 변화되는 상황이라면 요소를 특정할 수 있는 key를 사용한다.(ex. id)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

알고 있는 내용이었는데, 미처 놓친 부분이었네요!!
모르고 넘어갈뻔 했는데 짚어주셔서 감사합니다 :>

Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) styled-component에 prop을 넘길 때, 특히 boolean 타입을 prop을 넘길 때는 transient-prop이라는 걸 사용하는게 좋아요!
transient-prop이란 Prop 앞에 $표시를 붙여서 prop을 전달하는 방식으로 사용돼요.
이러한 형식으로 prop을 넘길 때의 장점은,

  • 해당 속성이 실제 html에 존재하는 속성이 아닌 임의로 부여한 속성이라는 것을 DOM에 알려주는 기능을 합니다. 따라서 지원하지 않는 속성이라는 에러를 마주하지 않게돼요.
  • 개발을 할 때 기능(값)을 위한 prop인지 스타일링을 위한 prop인지 가시적으로 구분 가능하게 해줍니다.

따라서 style과 관련된 prop을 넘길 때는 $prop의 형식을 사용해보는게 좋을 것 같아요!
transient props에 대한 간단한 글

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

반영했습니다!

<Title2>{buttonValue ? buttonValue : item.title}</Title2>

Choose a reason for hiding this comment

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

p5) 지나가던 행인입니다...!

Suggested change
<Title2>{buttonValue ? buttonValue : item.title}</Title2>
<Title2>{buttonValue || item.title}</Title2>

단락평가를 잘 이용하면 요로케 삼항연산자를 축약할 수도 있습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

우왁 이런 피드백 너무 좋습니다... 새로운걸 알게 되었네요!!
반영했습니다 :>

{buttonValue && (
<SvgWrapper>
<img src={ButtonCheckIc} alt="check complete button" />
</SvgWrapper>
Copy link
Collaborator

Choose a reason for hiding this comment

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

p1) 저희는 svg파일을 react-componen로 변환해서 사용해주고 있기 때문에 따로 img 태그 안에 작성해주지 않아도 돼요 !
custom.d.ts 파일에 보면 svg파일을 리액트 컴포넌트로 변환해주는 타입 모듈이 작성되어 있답니다 !
svgr세팅을 진행한 것도 위와같은 것을 하기 위해 작성해둔 것이에요.

말 그대로, 피그마에서 추출해 온 svg파일을 index.ts에서 리액트 컴포넌트로 export한 후 그대로 컴포넌트처럼 사용하는 거랍니다!

따라서 다음과같이 사용해보면돼요!

Suggested change
<SvgWrapper>
<img src={ButtonCheckIc} alt="check complete button" />
</SvgWrapper>
<ButtonCheckIc />

만약 svg에 스타일링을 지정해야 한다면, 다음과 같은 방법으로 사용하면 돼요!

const ButtonCheckIcon = styled(ButtoncheckIc)`
 적용해야할 스타일 작성하기!
`

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오..! 이번에 알게된 내용이네요!! './ic_check.svg?react' svg 파일은 ?react 를 붙여 export 하면 되겠네요 감사합니다!

)}
</Wrapper>
))}
</Container>
);
}

export default TimeSelectionButton;

const Wrapper = styled.div<{ isActive: boolean }>`
display: flex;
gap: 1.2rem;
align-items: center;

width: 33.5rem;
height: 4.8rem;
border: 1px solid ${({ theme, isActive }) => (isActive ? theme.colors.transparentBlue_50 : theme.colors.grayScaleLG2)};
border-radius: 8px;

background-color: ${({ theme, isActive }) => (isActive ? theme.colors.transparentBlue_5 : theme.colors.grayScaleWG)};
`;

const Title2 = styled.span`
position: relative;
left: 2rem;

${({ theme }) => theme.fonts.Title2_M_16}
color: ${({ theme }) => theme.colors.grayScaleDG};
`;

const Container = styled.div`
display: flex;
flex-direction: column;
gap: 1.2rem;
justify-content: center;
align-items: center;
`;

const SvgWrapper = styled.div`
display: flex;
justify-content: center;
align-items: center;
position: relative;
left: 14rem;

width: 2rem;
height: 2rem;
`;
35 changes: 35 additions & 0 deletions src/pages/juniorPromise/components/TimeSelectionTitleWrapper.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import styled from '@emotion/styled';

const TimeSelectionTitleWrapper = () => {
return (
<Container>
<Wrapper>
<TimeSelectionTitle>선약 시간을 선택해주세요</TimeSelectionTitle>
<TimeSelectionDescription>제안하신 3가지 일정 중 선배가 하나를 선택해요</TimeSelectionDescription>
</Wrapper>
Copy link
Contributor

Choose a reason for hiding this comment

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

P3) 컴포넌트 네이밍 규칙 보면
컴포넌트 네이밍 규칙 : Wrapper → Layout → Container → Box 순서로 내리기
로 되어 있어서..! Container와 Wrapper 순서를 바꾸면 어떨까요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

짚어주셔서 감사해요! 반영했습니다!!

</Container>
);
};

export default TimeSelectionTitleWrapper;

const TimeSelectionTitle = styled.h3`
${({ theme }) => theme.fonts.Head2_SB_18};
`;
const TimeSelectionDescription = styled.span`
${({ theme }) => theme.fonts.Body1_M_14};
color: ${({ theme }) => theme.colors.grayScaleMG2};
`;
const Wrapper = styled.div`
display: flex;
flex-direction: column;
gap: 0.2rem;

width: 33.5rem;
height: 4.9rem;
`;
const Container = styled.div`
display: flex;
justify-content: center;
align-items: center;
`;
7 changes: 7 additions & 0 deletions src/pages/juniorPromise/constants/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const TIME_SELECTION_TITLE = [
{
title: '첫 번째 일정 선택하기',
},
{ title: '두 번째 일정 선택하기' },
{ title: '세 번째 일정 선택하기' },
];
Loading