-
Notifications
You must be signed in to change notification settings - Fork 79
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
[이율] Sprint11 #733
[이율] Sprint11 #733
Changes from all commits
d7d4b8a
343f8c3
76f1878
ecc1437
7082d10
0a1283b
361af1f
68d023a
fd78a5f
90af397
4b58b27
6e8cc79
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
module.exports = { | ||
printWidth: 200, | ||
printWidth: 100, | ||
tabWidth: 2, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,13 @@ | ||
import Link from "next/link"; | ||
import Logo from "./Logo"; | ||
import GNB from "./GNB"; | ||
import { useAuth } from "@/src/contexts/AuthProvider"; | ||
import Image from "next/image"; | ||
import ImgUser from "@/src/img/ic_profile.svg"; | ||
|
||
export default function Header() { | ||
const { user, isAuth } = useAuth(true); | ||
|
||
return ( | ||
<header className="header"> | ||
<div className="header-wrap"> | ||
|
@@ -13,9 +18,13 @@ export default function Header() { | |
<GNB /> | ||
</div> | ||
<div> | ||
<Link href="/signin" className="btn-login"> | ||
로그인 | ||
</Link> | ||
{isAuth ? ( | ||
<Image src={ImgUser} width={40} height={40} alt="유저 이미지" /> | ||
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. alt는 해당 요소가 무엇인지를 설명할 수 있으면 좋아요. |
||
) : ( | ||
<Link href="/signin" className="btn-login"> | ||
로그인 | ||
</Link> | ||
)} | ||
</div> | ||
</div> | ||
</header> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,19 @@ | ||
import { ChangeEvent, useState } from "react"; | ||
import { ChangeEvent, ChangeEventHandler, useState } from "react"; | ||
import Styles from "./Input.module.scss"; | ||
|
||
interface EmailInputProps { | ||
name: string; | ||
value: string; | ||
onChange: () => void; | ||
onChange: (e: ChangeEvent<HTMLInputElement>) => void; | ||
id: string; | ||
className: string; | ||
required: boolean; | ||
setIsInvalid: (value: boolean) => void; | ||
} | ||
|
||
Comment on lines
4
to
12
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이나 버튼같은 요소는 공통적으로 많이 활용되는데요, 이때마다 새로운 콤포넌트를 만드는건 어찌보면 비효율적으로 판단됩니다. 예를 들어 type InputProps = Omit<
InputHTMLAttributes<HTMLInputElement>,"children"
> &
InputVariants & {
icon?: keyof typeof Icons;
};
const Input = forwardRef<HTMLInputElement, InputProps>(
({ className, icon, ...props }, ref) => {
return (
<div className="relative w-full">
{icon && (
<Icon
name={icon}
size={size}
className="absolute left-2 top-1/2 -translate-y-1/2 text-input-foreground"
/>
)}
<input
type={type}
className={cn(
inputVariants(),
icon && "!pl-10",
className,
)}
ref={ref}
{...props}
/>
</div>
);
},
);
Input.displayName = "Input";
export { Input }; 와 같이 한번 만들어두면, 로그인 폼의 경우 <form>
<div>
<Input type="email" id="email" name="email" value={loginInfo.email} onChange={e => setLoginInfo(prev => ({...prev, [e.target.name]: e.target.value})}/>
</div>
</form> 와 같은 식으로 그냥 바로 활용해볼 수 있겠죠. 또한 데이터 정합성 체크 - validation - 의 경우, 브라우저의 네이티브 validation으로 처리를 해도 충분할 수 있지만, 대다수의 경우 그 외의 요구사항들이 반영되어야 하는 경우도 많이 있습니다. 따라서 form 데이터 및 정합성을 관리해 줄 수 있도록 폼 데이터 상태관리를 해주고, component들은 그에 대한 결과에 따라 UI가 업데이트만 되도록 해주면 좀 더 유지보수성이 좋은 프로젝트가 될 거에요 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. 또한 입력된 값이 비어있는지 여부만을 관리하기 위해 |
||
export default function EmailInput({ name, value, id, className, required, setIsInvalid }: EmailInputProps) { | ||
export default function EmailInput({ name, value, id, className, required, setIsInvalid, onChange }: EmailInputProps) { | ||
const [isEmpty, setIsEmpty] = useState(false); | ||
|
||
const handleChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
if (e.target.checkValidity()) { | ||
setIsInvalid(false); | ||
|
@@ -24,6 +25,7 @@ export default function EmailInput({ name, value, id, className, required, setIs | |
} else { | ||
setIsEmpty(false); | ||
} | ||
onChange(e); | ||
}; | ||
return ( | ||
<> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ interface TextInputProps { | |
} | ||
|
||
export default function TextInput({ name, value, onChange, id, className, required, placeholder }: TextInputProps) { | ||
const [content, setContent] = useState(""); | ||
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. content가 필요 없는걸로 보입니다! |
||
const [isEmpty, setIsEmpty] = useState(false); | ||
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
if (e.target.value.length === 0) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ module.exports = { | |
protocol: "https", | ||
hostname: "example.com", | ||
port: "", | ||
pathname: "/**", | ||
pathname: "/...", | ||
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. 음 |
||
}, | ||
{ | ||
protocol: "https", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,11 @@ | ||
import { AuthProvider } from "@/src/contexts/AuthProvider"; | ||
import type { AppProps } from "next/app"; | ||
import "src/styles/style.scss"; | ||
|
||
export default function MyApp({ Component, pageProps }: AppProps) { | ||
return ( | ||
<div> | ||
<AuthProvider> | ||
<Component {...pageProps} /> | ||
</div> | ||
</AuthProvider> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,24 +3,23 @@ | |
import { useEffect, useState } from "react"; | ||
import { useRouter } from "next/navigation"; | ||
import useAsync from "@/src/hooks/useAsync"; | ||
import { createItems } from "@/src/api/api"; | ||
import { createItems, postArticle, uploadImg } from "@/src/api/api"; | ||
import Input from "@/components/Input"; | ||
import Button from "@/components/Button/Button"; | ||
import Header from "@/components/Header"; | ||
import { useAuth } from "@/src/contexts/AuthProvider"; | ||
|
||
const INITIAL_VALUES = { | ||
name: null, | ||
description: null, | ||
images: null, | ||
}; | ||
|
||
export default function AddBoardPage({ initialValues = INITIAL_VALUES }) { | ||
export default function AddBoardPage() { | ||
const { user } = useAuth(true); | ||
const [isLoading, loadingError, onSubmitAsync] = useAsync(createItems); | ||
const [isDisableSubmit, setIsDisableSubmit] = useState(true); | ||
const [values, setValues] = useState(initialValues); | ||
const [values, setValues] = useState({ | ||
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. values라는 이름보다는 이 값이 무엇을 표현하는지 설명하는게 더 좋을듯해요 |
||
title: "", | ||
content: "", | ||
image: "", | ||
}); | ||
const router = useRouter(); | ||
|
||
const handleChange = (name: keyof typeof INITIAL_VALUES, value: string) => { | ||
const handleChange = (name: string, value: string) => { | ||
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.
|
||
setValues((prevValues) => ({ | ||
...prevValues, | ||
[name]: value, | ||
|
@@ -29,31 +28,37 @@ export default function AddBoardPage({ initialValues = INITIAL_VALUES }) { | |
|
||
const handleInputChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
const { name, value } = e.target; | ||
handleChange(name as keyof typeof INITIAL_VALUES, value); | ||
handleChange(name, value); | ||
}; | ||
|
||
const handleSubmit = async (e: React.FormEvent<HTMLFormElement>) => { | ||
e.preventDefault(); | ||
const formData = new FormData(); | ||
formData.append("name", values.name || ""); | ||
formData.append("description", values.description || ""); | ||
formData.append("images", values.images || ""); | ||
formData.append("title", values.title); | ||
formData.append("content", values.content); | ||
if (values.image) { | ||
const imgForm = new FormData(); | ||
imgForm.append("image", values.image); | ||
|
||
if (typeof onSubmitAsync !== "function") { | ||
console.error("onSubmitAsync is not a function"); | ||
return; | ||
const response = await uploadImg(imgForm); | ||
if (!response) return; | ||
formData.append("image", response); | ||
} | ||
|
||
const result = await onSubmitAsync(formData); | ||
if (!result) return; | ||
const jsonObject: { [key: string]: any } = {}; | ||
formData.forEach((value, key) => { | ||
jsonObject[key] = value; | ||
}); | ||
|
||
router.push("/items"); | ||
}; | ||
const response = await postArticle(jsonObject); | ||
Comment on lines
+48
to
+53
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. 음 폼데이터를 다시 json으로 변환해 서버에 전달하는듯 보이는데 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. 또한 response가 서버단에서 반환된 객체인가요? |
||
if (!response) return; | ||
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. !response를 오류가 발생한 모든케이스로 잡으면 안될듯해요. |
||
|
||
useEffect(() => { | ||
setIsDisableSubmit(Object.values(values).every((el: any) => el !== "" && el !== null && el.length !== 0)); | ||
}, [values]); | ||
router.push(`/boards/${response.id}`); | ||
}; | ||
|
||
if (!user) { | ||
return null; | ||
} | ||
Comment on lines
+59
to
+61
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. 로그인 된 사용작 ㅏ없다면 빈 화면을 보여주는게 맞을까요? |
||
return ( | ||
<> | ||
<Header /> | ||
|
@@ -62,7 +67,7 @@ export default function AddBoardPage({ initialValues = INITIAL_VALUES }) { | |
<div className="section-wrap"> | ||
<header className="section-header"> | ||
<h2 className="section-tit">게시글 쓰기</h2> | ||
<Button type="submit" id="submit-article" size="small" disabled={!isDisableSubmit} className="btn-small btn-submit"> | ||
<Button type="submit" id="submit-article" size="small" disabled={values.title === "" || values.content === ""} className="btn-small btn-submit"> | ||
등록 | ||
</Button> | ||
</header> | ||
|
@@ -73,7 +78,7 @@ export default function AddBoardPage({ initialValues = INITIAL_VALUES }) { | |
</sup> | ||
제목 | ||
</h3> | ||
<Input.Text name="name" value={values.name} onChange={handleInputChange} placeholder="제목을 입력해주세요" /> | ||
<Input.Text name="title" value={values.title} onChange={handleInputChange} placeholder="제목을 입력해주세요" /> | ||
</section> | ||
<section className="section-addItem-content"> | ||
<h3 className="section-tit"> | ||
|
@@ -82,11 +87,11 @@ export default function AddBoardPage({ initialValues = INITIAL_VALUES }) { | |
</sup> | ||
내용 | ||
</h3> | ||
<Input.Textarea name="description" value={values.description} onChange={handleInputChange} size="large" placeholder="내용을 입력해주세요" /> | ||
<Input.Textarea name="content" value={values.content} onChange={handleInputChange} size="large" placeholder="내용을 입력해주세요" /> | ||
</section> | ||
<section className="section-addItem-content"> | ||
<h3 className="section-tit">상품 이미지</h3> | ||
<Input.File name="images" value={values.images} onChange={handleChange} /> | ||
<Input.File name="image" value={values.image} onChange={handleChange} /> | ||
</section> | ||
</div> | ||
</form> | ||
|
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.
BoardList
콤포넌트에 사용될 prop에 대한 인터페이스를articleListProps
라고 하신 이유가 있나요?특별한 이유가 없다면 콤포넌트 이름에 Props를 추가하는 형태로 타입 설정을 해보면 어떨까 싶습니다.
추가로, 인터페이스 또는 타입 이름을 선언하는 경우, PascalCase를 유지하는게 타입스크립트 진영 컨벤션인데요, 이 컨벤션을 따르면 어떨까요?