Card, RandomRecipe, HotRecipe 컴포넌트 마이그레이션, Skeleton 카드 리팩토링, Next.js Storybook 설정 #61
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
고려해야하는 부분 Next를 사용하면서 Loading없이 Detail을 페이지로 들고, Dialog 컴포넌트를 사용하지 않으므로 이에따른 로직 변경이 필요해짐 기존의 컴포넌트, 로직과 많이 달라진 부분이 있어 이에대한 논의와 설계가 다시 필요할것 같습니다.
hustle-dev
added
🌟 feature
hustle-dev
changed the title
hustle-dev
changed the title
jhyj0521
reviewed
Mar 28, 2022
src/components/Card/Card.tsx
Outdated
|
|
||
| return ( | ||
| <Link href={`${router.pathname}?id=${id}`}> | ||
| <CardLink role="button"> |
Contributor
There was a problem hiding this comment.
기존과는 다르게 모달이 아니라 페이지를 전환하기 때문에 role="button" 어트리뷰트를 제거해도 될 것 같습니다!
hanana1253
reviewed
Mar 28, 2022
src/components/Card/Card.tsx
Outdated
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
||
| const router = useRouter(); | ||
| // useAuthUser 훅 이후 주석해제 |
Member
There was a problem hiding this comment.
이 로직은 detail 페이지에서 사용하는 것이기 때문에 삭제해도 괜찮을 것 같습니당
hustle-dev
commented
Mar 28, 2022
| const getRecipe = useCallback(() => { | ||
| setRecipe(savedRecipe); | ||
| const { data } = useGetRandomRecipeQuery(1); | ||
| if (data) { |
Contributor
Author
There was a problem hiding this comment.
여기서 받아오는 값이 RandomRecipe | undefined로 나와서 이렇게 해주었는데 더 효과적, 효율적으로 하는 방법은 없을지?
| if (isLoading) { | ||
| return <SkeletonCard type="wide" background="white" hasSummary={true} headingPosition="bottomLeft" />; | ||
| } else { | ||
| const { id, title, summary, image } = recipe as RandomRecipeType; |
Contributor
Author
There was a problem hiding this comment.
타입 단언말고 알아서 추론하게 해주는 방법이 없는지?
hhhyyo
reviewed
Mar 28, 2022
src/components/Card/Card.tsx
Outdated
| title, | ||
| summary = '', | ||
| }: CardProps): JSX.Element => { | ||
| const [recipeData, setRecipeData] = useState({}); |
Contributor
There was a problem hiding this comment.
디테일 페이지에 필요한 로직이라서 지우시면 될 것 같습니다!
src/components/Card/Card.tsx
Outdated
|
|
||
| return ( | ||
| <Link href={`${router.pathname}?id=${id}`}> | ||
| <CardLink role="button"> |
Contributor
There was a problem hiding this comment.
원래는 모달창을 여는 버튼이라 role속성이 필요했는데
이제 진짜 페이지 이동이라서 없어도 될 것 같습니다
src/components/Card/Card.tsx
Outdated
| // }; | ||
|
|
||
| return ( | ||
| <Link href={`${router.pathname}?id=${id}`}> |
Contributor
There was a problem hiding this comment.
'/recipe/${id}' 형식으로 변경해주시면 됩니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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
#15
#12
#9
What does this PR do?
Other information