EmptyPage와 Layout, Pagination 컴포넌트 작업 #50
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TODO: - Header에 맞는 로고와 검색인풋, 버튼 등 넣어야합니다. - 스타일링 해야합니다.
- 모든 페이지에 렌더링 될 Header을 가지고 있는 Layout 컴포넌트를 만들었습니다. - scroll to top 버튼 만들면 추가하기 (기존 코드에서는 header에 포함되어있었으나 별도로 두는 게 좋을듯)
- dom 요소를 추가했습니다.
- 404 페이지 및 검색결과/마이레시피 결과 없을 때 보여줄 EmptyPage입니다.
- 스타일링이 들어간 Empty Page 컴포넌트로 만들었습니다. :
hanana1253
added
🌟 feature
hustle-dev
reviewed
Mar 17, 2022
| import Link from 'next/link'; | ||
| import { NextPage } from 'next'; | ||
|
|
||
| const NotFound: NextPage = () => { |
Contributor
There was a problem hiding this comment.
404 next.js config를 활용하여 redirect하는부분도 있으면 좋을거같아요
Member
Author
There was a problem hiding this comment.
특정 url을 redirect하는 건 next.config.js에서 쉽게 할 수 있는 것 같은데 없는 페이지에 대해서 일괄적으로 404로 redirect하는 것은 조사해봐야 할 것 같아요.
- 404에서 불러올 때 경로 수정 - component/index.ts에서 re-export 구문 추가 - 스토리북에서 decorator 추가 및 변경된 타입구조 적용 - styling 코드에서 theme으로 primaryOrange 컬러 적용
- default export를 수정하면서 경로 변경되어 story파일을 수정했습니다.
- className 프롭은 emotion을 사용하므로 더이상 필요없게 되어 삭제 - 앞 페이지 목록으로 넘어가는 ellipsis 버튼이 딱히 사용성이 있게 느껴지지 않아 삭제
- 공식문서 및 사용법을 보니 Link 태그와 a 태그를 함께 쓰네요
hanana1253
changed the title
hanana1253
commented
Mar 18, 2022
| <EmptyPage> | ||
| <h2>Page Not Found</h2> | ||
| <Link href="/"> | ||
| <a>Go to main page?</a> |
Member
Author
There was a problem hiding this comment.
정민님이 지적해주신 것처럼 Link태그 안에 요소가 들어오는 경우엔 a 태그로 꼭 감싸줘야 하더라구요.
children이 text로만 넘어오면 태그로 감싸주긴 하지만 일관적인 사용을 위해 a 태그 추가했습니다.
이유는 아래 블로그에 잘 설명되어있는데 블로그 쓰신 분 열정이 대단해서 참고해보시면 좋을 것 같아요.
https://uchanlee.dev/nextjs/Why-using-a-tag-in-nextjs-Link/
Contributor
There was a problem hiding this comment.
ㄷㄷ 블로그 글 쓰신분 확인하려고 내부 코드까지 뜯어보다니 대단하시네여..
hustle-dev
reviewed
Mar 21, 2022
| export const Pagination = ({ limit, currentPage, onClick: handleClick, totalResults }: PaginationProps) => { | ||
| const lastPageNum = Math.ceil(totalResults / limit); | ||
| const pageStartNum = Math.max(currentPage - 2, 1); | ||
| const pageEndNum = Math.min(currentPage + 2, lastPageNum); |
Contributor
There was a problem hiding this comment.
이 부분을 분리해서 커스텀훅을 만들어 연산이 많이 필요한 경우 useMemo를 사용해도 될것 같아요 🙂
Member
Author
There was a problem hiding this comment.
좋은 아이디어네요 바로 반영하겠습니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Type
What kind of change does this PR introduce?
Related Issues
solved #34
solved #37
What does this PR do?
Other information