-
Notifications
You must be signed in to change notification settings - Fork 77
[이경원] sprint8 #518
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
base: React-이경원
Are you sure you want to change the base?
The head ref may contain hidden characters: "React-\uC774\uACBD\uC6D0-sprint8"
[이경원] sprint8 #518
Changes from 1 commit
b2e37bd
6f8bbb0
e11e25f
212e864
4dc5dd0
ddb35a8
644200d
b8b97b0
c4dbe93
2251cec
bfbdec2
a8182ee
1ecb2b0
d145418
a993960
327648c
24284d6
4d6001d
6ca4142
9b7b75d
23c8847
3f054ad
7d6cf75
cc0bb7c
c220d80
62d72fa
d0c318c
f6a2b54
df81b03
09fbd90
3311d64
c44727f
3d8ae12
2c503c3
dff82dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| import React, { useState } from "react"; | ||
| import React, { useState, ChangeEvent } from "react"; | ||
| import styled from "styled-components"; | ||
| function CommentInput() { | ||
| const [comment, setComment] = useState(""); | ||
|
|
||
| const handleInputChange = (event) => { | ||
| interface RegisterButtonProps { | ||
| isEnabled: boolean; | ||
| } | ||
|
|
||
| function CommentInput(): JSX.Element { | ||
| const [comment, setComment] = useState<string>(""); | ||
|
|
||
| const handleInputChange = (event: ChangeEvent<HTMLTextAreaElement>): void => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 const handleInputChange: ChangeEventHandler<HTMLTextAreaElement> = (event) => {
setComment(event.target.value);
}; |
||
| setComment(event.target.value); | ||
| }; | ||
|
|
||
|
|
@@ -65,7 +70,7 @@ const InputArea = styled.textarea` | |
| } | ||
| } | ||
| `; | ||
| const RegisterButton = styled.button` | ||
| const RegisterButton = styled.button<RegisterButtonProps>` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A컴포넌트 구현에 바로위애 A컴포넌트의 인터페이스가 작성되는게 자연스러운 패턴으로 알고 있습니다 |
||
| display: flex; | ||
| flex-direction: row; | ||
| justify-content: center; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,16 @@ import axios from "axios"; | |
| import noneComments from "../images/noneComments.svg"; | ||
| import dots from "../images/3dots.svg"; | ||
|
|
||
| interface Comment { | ||
| id: number; | ||
| content: string; | ||
| image: string; | ||
| nickname: string; | ||
| updatedAt: string; | ||
| } | ||
|
|
||
| function CommentList() { | ||
| const [comments, setComments] = useState(null); | ||
| const [comments, setComments] = useState<Comment[]>([]); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| const { productId } = useParams(); | ||
|
|
||
| const getComments = async (productId) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,17 @@ | ||
| import { useEffect, useRef, useState } from "react"; | ||
| import React, { useEffect, useRef, useState } from "react"; | ||
| import styled from "styled-components"; | ||
| import X from "../images/ic_X.svg"; | ||
| import plus from "../images/ic_plus.svg"; | ||
| function FileInput({ name, value, initialPreview, onChange }) { | ||
|
|
||
| interface FileInputProps { | ||
| name: string; | ||
| value: File | null; | ||
| initialPreview?: string; | ||
| onChange: (name: string, nextName: string | null) => void; | ||
| } | ||
| function FileInput({ name, value, initialPreview, onChange }: FileInputProps) { | ||
| const [preview, setPreview] = useState(initialPreview); | ||
| const inputRef = useRef(); | ||
| const inputRef = useRef<HTMLInputElement>(null); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 read-only로 안전하게 사용하셨군요. useRef는 첫 인자를 null로 전달하면 read-only가 되는 특징이 있습니다. useRef를 cmd를 누른채로 클릭해서 타입선언을 확인해보세요 😄 2024-05-26.10.46.46.mov |
||
|
|
||
| const handleChange = (e) => { | ||
| const nextValue = e.target.files[0]; | ||
|
|
@@ -43,8 +50,10 @@ function FileInput({ name, value, initialPreview, onChange }) { | |
| <InputText>이미지 등록</InputText> | ||
| </InputWrapper> | ||
| <Container1> | ||
| {value && <Image src={preview} alt="이미지 등록"></Image>} | ||
| {value && <Delete onClick={handleClearClick} src={X}></Delete>} | ||
| <> | ||
| {value && <Image src={preview} alt="이미지 등록"></Image>} | ||
| {value && <Delete onClick={handleClearClick} src={X}></Delete>} | ||
| </> | ||
| </Container1> | ||
| </Container> | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,32 +2,47 @@ import React, { useState } from "react"; | |
| import styled from "styled-components"; | ||
| import DeleteIcon from "../images/ic_X_gray.svg"; | ||
|
|
||
| const InputTag = ({ tags, setTags, setIsTagsEmpty }) => { | ||
| interface TagInputProps { | ||
| tags: string[]; | ||
| setTags: React.Dispatch<React.SetStateAction<string[]>>; | ||
|
Comment on lines
+5
to
+7
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인터페이스는 TagInput가 어떤 컴포넌트인지 설명해줍니다. 조금 더 TagInput컴포넌트의 역할을 드러내려면, 또는 TagInput이 할 수 있는 동작을 제한하려면 이렇게 바꿔 볼 수 있겠죠. interface TagInputProp{
tags: stirng[];
addTag: (tagName: string) => void;
removeTag: (tagIndex: number) => void;
}그럼 이 외부에서 선언만 보고도, TagInput이 tags를 무엇으로든 set할 수 있기보단, 추가/삭제를 하는 기능이 있다는 걸 알 수 있습니다. ( 그렇게 되면, 사실 Tag"Input" 이라는 이름이 안어울린다는 사실도 더 잘 보이게 되죠. 그리고나서 구현을 보면, input과 list를 같이 가지고 있고요 - 시각적으로는 input에 들어가있을지라도요 -. 무엇을 하는 컴포넌트인지를 더 잘 드러내면 네이밍, 코드파악, 컴포넌트분리, 리팩토링, 버그수정에 도움이 됩니다 |
||
| setIsTagsEmpty?: React.Dispatch<React.SetStateAction<boolean>> | undefined; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 전달하는 곳이 없어서 제거해도되겠네요. 이런 부분에서도 ?를 지워봤을때, 타입이 깨지는지 확인하고, 어디서 어떻게 사용하는지, 사용할 필요가 없는지 확인하면서 타입체크의 장점을 누릴 수 있습니다 |
||
| placeholder?: string; | ||
| } | ||
| const TagInput = ({ | ||
| tags, | ||
| setTags, | ||
| setIsTagsEmpty, | ||
| placeholder, | ||
| }: TagInputProps) => { | ||
| const [inputValue, setInputValue] = useState(""); | ||
|
|
||
| const handleInputChange = (e) => { | ||
| const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| setInputValue(e.target.value); | ||
| }; | ||
|
|
||
| const handleInputKeyDown = (e) => { | ||
| const handleInputKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => { | ||
| if (e.key === "Enter" && inputValue.trim() !== "") { | ||
| setTags([...tags, inputValue.trim()]); | ||
| e.preventDefault(); | ||
| setInputValue(""); | ||
| setIsTagsEmpty(false); | ||
| if (setIsTagsEmpty) { | ||
| setIsTagsEmpty(false); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| const handleTagDelete = (index) => { | ||
| const newTags = [...tags]; | ||
| newTags.splice(index, 1); | ||
| setTags(newTags); | ||
| setIsTagsEmpty(newTags.length === 0); | ||
| if (setIsTagsEmpty) { | ||
| setIsTagsEmpty(newTags.length === 0); | ||
| } | ||
| }; | ||
|
|
||
| return ( | ||
| <InputTagContainer> | ||
| <TagInput | ||
| <InputTag | ||
| type="tag" | ||
| value={inputValue} | ||
| onChange={handleInputChange} | ||
|
|
@@ -54,7 +69,7 @@ const InputTagContainer = styled.div` | |
| align-items: flex-start; | ||
| `; | ||
|
|
||
| const TagInput = styled.input` | ||
| const InputTag = styled.input` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 기존 네이밍이 더 좋아보여요. 이 자체로는 특별한 기능없는 표준 Input 요소이고, Tag(도메인) 를 입력하기 위한 특별한 스타일을 가진 input 요소니까, TagInput이 더 좋아보입니다. export되고 있지않고 이 파일안에서만 사용하고, 이미 "Tag"라는 맥락이 충분히 이 파일안에서 파악가능하니까 그냥 Input이라고만 해도 무방할거같고요. (사실 그러면 공통컴포넌트같아지는 이슈가 있지만...) |
||
| width: 100%; | ||
| height: 56px; | ||
| background: #f3f4f6; | ||
|
|
@@ -111,4 +126,4 @@ const CloseIcon = styled.img` | |
| width: 20px; | ||
| height: 20px; | ||
| `; | ||
| export default InputTag; | ||
| export default TagInput; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제네릭을 활용하셨네요 👍
다만 initialState로 "" 을 전달하셨기 때문에, 제네릭없이도 string으로 잘 추론됩니다. 그걸 활용하시는 편이 좋습니다.
function useState<S>(initialState: S | (() => S)): [S, Dispatch<SetStateAction<S>>];