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

[Feature/BAR-155] 참고하는 레이아웃 수정 및 폴더 저장 삭제 기능 추가 #63

Merged
merged 6 commits into from
Feb 16, 2024

Conversation

miro-ring
Copy link
Contributor

Summary

구현 내용 및 작업한 내역을 요약해서 적어주세요

To Reviewers

PR을 볼 때 주의깊게 봐야하거나 말하고 싶은 점을 적어주세요

react-responsive-masonry를 사용하여 벽돌식 컴포넌트를 구성하였습니다.
기존 카드 컴포넌트를 래핑하는 방식이기 때문에 큰 영향 없을 것 같습니다.

How To Test

  • 벽돌 컴포넌트 및 개행 확인
  • 폴더 저장/삭제 확인

Copy link
Member

@dmswl98 dmswl98 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

Comment on lines 36 to 43
{
onSuccess: async () => {
await queryClient.invalidateQueries({
queryKey: ['templates'],
});
showToast({ message: '글이 저장됐어요.' });
},
},
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 useSaveTemplate 내부에 작성해도 되지 않을까요?
그리고 onSuccess에 async, await 키워드는 없어도 됩니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 부분 반영해보았습니다 감사합니다 ㅎㅎ

/>
))}
</ul>
<ResponsiveMasonry columnsCountBreakPoints={{ 768: 2, 1080: 3 }}>
Copy link
Member

Choose a reason for hiding this comment

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

여러 곳에서 활용할 수 있게 합성 컴포넌트로 만들어보는 건 어떨까요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmswl98
기존 Card 컴포넌트 내부에 해당 요소를 넣자는 말씀이실까요?

Copy link
Member

Choose a reason for hiding this comment

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

엇 아뇨!!

다른 페이지도 768px일 때 2줄, 1080일 때 3줄이라

const Responsive = ({ children }) => {
  return <ResponsiveMasonry columnsCountBreakPoints={{ 768: 2, 1080: 3 }}>{children}</ResponsiveMasonry>
}

이렇게 구현해두면 어디에서든 쓸 수 있을 것 같아서요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dmswl98
아아 해당 부분도 반영해두었습니다 !! ㅎㅎ

</ul>
<ResponsiveMasonry columnsCountBreakPoints={{ 768: 2, 1080: 3 }}>
<Masonry className={styles.referCardsWrapper} gutter="16px">
{data.content.map((data) => (
Copy link
Member

Choose a reason for hiding this comment

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

어떤 코드인지 불명확해서...! 아래처럼 변경되면 좋을 것 같아요

Suggested change
{data.content.map((data) => (
const { data: templates } = useGetTemplates({
category: selectedFilterHeader,
sort: selectedFilterButton,
});
{templates.content.map((template) => ()}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 부분 반영해보았습니다 감사합니다 ㅎㅎ

import { COLORS } from '@styles/tokens';

import useDeleteTemplate from '../queries/useDeleteTemplate';
import useSaveTemplate from '../queries/useSaveTemplate';

interface FolderDialogProps {
templateId: number;
Copy link
Member

Choose a reason for hiding this comment

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

요런 것도 정의된 타입에 의존시켜 작성하는 건 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해당 부분 반영해보았습니다 감사합니다 ㅎㅎ

@dmswl98
Copy link
Member

dmswl98 commented Feb 15, 2024

image

768px 이하인 경우에는 padding이 아예 없는 것 같아요...!

@miro-ring
Copy link
Contributor Author

image 768px 이하인 경우에는 padding이 아예 없는 것 같아요...!

@dmswl98
디자인 상에 768px 미만인 경우에 대한 대응이 아예 없어서 그렇긴합니다....!

Copy link
Member

@dmswl98 dmswl98 left a comment

Choose a reason for hiding this comment

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

👍🏻👍🏻👍🏻

@miro-ring miro-ring merged commit f87e8fd into main Feb 16, 2024
@miro-ring miro-ring deleted the feature/BAR-155 branch February 16, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants