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

템플릿 CRUD 카테고리, 태그 추가 #275

Merged
merged 43 commits into from
Aug 6, 2024

Conversation

Hain-tain
Copy link
Contributor

⚡️ 관련 이슈

close #181 #241

📍주요 변경 사항

1. 카테고리, 태그 기능 추가

2. 템플릿 수정, 생성 페이지 컴포넌트 통일 (TemplateEdit)

3. 커스텀 훅 분리

🎸기타

- 아직 리펙토링이 상당히 많이 필요하지만 1차 공유를 위해 먼저 PR 올립니다. 코드리뷰 많이 달아주세요.

  • 특히 hook 분리와 관련하여 코드리뷰 많이 남겨주세요..! (헤드리스 컴포넌트로 구성하기 위해 컴포넌트 내부에서 필요한 상태나 함수들을 모두 hook에서 만들어서 내보내고 있는데, 하나의 훅에서 너무 많은 책임을 가지고 있는것 같고, 너무 많은 것을 한꺼번에 export 해주고 있다는 생각이 듭니다.)

@Hain-tain Hain-tain added feature 기능 추가 FE 프론트엔드 labels Aug 5, 2024
@Hain-tain Hain-tain added the refactor 요구사항이 바뀌지 않은 변경사항 label Aug 5, 2024
@Hain-tain Hain-tain added this to the 3차 스프린트 🐤 milestone Aug 5, 2024
@Hain-tain Hain-tain self-assigned this Aug 5, 2024
.env
.env.development
.env.production
.env*
Copy link
Contributor

Choose a reason for hiding this comment

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

요거는 바뀌면 안될거 같아요@!

Comment on lines 37 to 41
} catch (error) {
console.error(error);
throw new Error(errorMessage);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

customFetch는 에러 핸들링 관련 PR에서 바뀔거 같긴 해요!

Comment on lines +40 to +45
border-radius: 7px;
&:hover {
color: ${({ theme }) => theme.color.light.white};
background-color: ${({ theme }) => theme.color.light.primary_500};
}
`;
Copy link
Contributor

Choose a reason for hiding this comment

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

(농담) 7px 말고 8px은 별로였나요 ㅋㅋㅋ

여기 color가 white라면 위에 background-color도 같은 theme 변수를 써도 좋았을거 같아요!

Comment on lines +47 to +51
{options?.map((option, idx) => (
<S.Option key={idx} onClick={() => handleCurrentValue(option)}>
{getOptionLabel(option)}
</S.Option>
))}
Copy link
Contributor

Choose a reason for hiding this comment

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

여기서 options가 옵셔널이 아닌거 같아요!

@Jaymyong66 Jaymyong66 merged commit ac94118 into woowacourse-teams:dev/fe Aug 6, 2024
1 check passed
@Hain-tain Hain-tain deleted the refactor/template_CRUD branch October 7, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 프론트엔드 feature 기능 추가 refactor 요구사항이 바뀌지 않은 변경사항
Projects
Status: Weekend Done
Development

Successfully merging this pull request may close these issues.

3 participants