[박세현] 2주차 과제 끗...#1
Open
pakxe wants to merge 16 commits into
Open
Conversation
devHudi
requested changes
May 8, 2022
devHudi
left a comment
devHudi
left a comment
There was a problem hiding this comment.
안녕하세요 세현님! 리액트를 처음 접해보아 어색하셨을 텐데, 과제 잘 수행해주셨네요!
특히 그라데이션까지 적용해주신 정성이 돋보이네요.
몇가지 수정하면 좋을 것들을 코멘트로 남겨드릴게요. 참고만 해주세요 😁
Comment on lines
+29
to
+34
| <> | ||
| <CardWrapper> | ||
| <ColorWrapper color={props.color}/> | ||
| <TextBox fontweight="550" color={props.color}>{props.color}</TextBox> | ||
| </CardWrapper> | ||
| </> |
There was a problem hiding this comment.
이미 CardWrapper 컴포넌트가 최상위에 단 하나만 존재하고 있어요. <> </>를 지워도 오류가 나지 않을 것 같은데, 한번 지워볼까요?
(참고로 <> </> 를 Fragment 라고 부릅니다 😁)
| border: 1px black solid; | ||
| text-align: center; | ||
| margin: 5px; | ||
| // |
Comment on lines
+41
to
+57
| <> | ||
| <Main> | ||
| <PhotoWrapper src={props.src}> | ||
| </PhotoWrapper> | ||
| <TextWrapper> | ||
| <TextBox fontsize="30px" fontweight="800"> | ||
| {props.title} | ||
| </TextBox> | ||
| <TextBox fontsize="20px" fontweight="200" color="grey"> | ||
| {props.internal} | ||
| </TextBox> | ||
| </TextWrapper> | ||
| <ButtonWrapper btncolor={props.btncolor}> | ||
| <TextBox fontsize="20px" color="white">Action</TextBox> | ||
| </ButtonWrapper> | ||
| </Main> | ||
| </> |
There was a problem hiding this comment.
마찬가지로 이 코드에서도 Fragment 를 지워도 괜찮을 것 같아요.
|
|
||
| const TextBox = (props) => { | ||
| return <StyledText color={props.color}>{props.children}</StyledText>; | ||
| return <StyledText color={props.color} size={props.fontsize} weight={props.fontweight}>{props.children}</StyledText>; |
There was a problem hiding this comment.
fontsize 와 fontweight 에는 camelCase 를 적용해볼 수 있을 것같은데요, fontSize 와 fontWeight 로 변경하면 더 좋겠군요!
변수명을 규칙에 맞게 수정하였습니다.
미디어카드의 예제 5개가 전부 같은 이미지로만 만들어진걸, 각각 다른 이미지를 보여주도록 수정
이미 감싸져서 필요없는 fragment와 //를 제거함
This reverts commit 956911f.
필요없는 fragment와 //제거
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
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.
No description provided.