Skip to content

✨ HTMLViewer에 MainImage 기능 추가 #18

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 8 commits into from
Aug 3, 2023
Merged

✨ HTMLViewer에 MainImage 기능 추가 #18

merged 8 commits into from
Aug 3, 2023

Conversation

yeolyi
Copy link
Collaborator

@yeolyi yeolyi commented Aug 3, 2023

스크린샷 2023-08-03 오후 5 20 20

변경 사항

  • HTMLViewer가 image prop을 받을 수 있도록 수정

추가 사항

  • /news/[id]에서 확인해보실 수 있습니다.
  • 게시물 내용이 아주 짧을 때를 대비해 flow-root를 사용했습니다.
  • next.js의 Image를 사용할 때 이미지의 정확한 크기를 제공하거나 이미지가 parent 컴포넌트를 fill하도록하는 두가지 옵션이 있었습니다. 전자는 실제 이미지의 크기와 다르면 에러를 뱉어서 후자의 방법을 사용했습니다. 실제 이미지 크기와 디자인상에서의 이미지의 크기가 다른 경우가 있을 것 같음도 이유입니다.
    • 다만 이경우 console에서 sizes prop을 제공하라고 warning을 띄우는데 어떻게 해결해야될지는 고민중입니다. 디자인 전체에서 이미지 크기 목록을 받아와 배열로 제공해야되나,,,?

@yeolyi yeolyi requested review from Limchansol and whwoohw August 3, 2023 08:26
import { usePathname } from 'next/navigation';

import { SegmentNode, main } from '@/types/page';

export default function useCurrentSegmentNode(): SegmentNode {
Copy link
Collaborator

Choose a reason for hiding this comment

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

아마 내비 만들 때까지만 해도 hooks 폴더가 없었을 거라 여기에 써두었을 것으로 추정되는데 얘도 훅이라면 hooks에 넣는 건 어떠신가요!
이 함수 밑에 있는 findChildWithName은 페이지들이랑 관련된 것 같으니 utils/page.ts에 넣어놓고...?
크게 중요한 부분은 아니니 애매하다면 편한 대로 해도 무방할 듯합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

findChildWithName은 export할 필요가 없어서 useCurrentSegmentNode랑 같이 가도 좋을 것 같고, useCurrentSegmentNode는 hooks로 옮길게요!

@yeolyi yeolyi merged commit b8c84ff into main Aug 3, 2023
@yeolyi yeolyi deleted the feat/HTMLViewer branch August 3, 2023 19:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants