Skip to content
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

Merged
1 change: 0 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ module.exports = {
],
plugins: ["react", "react-hooks", "@typescript-eslint", "jsx-a11y", "import", "prettier"],
rules: {
"prettier/prettier": "error",
"react/react-in-jsx-scope": "off", // Next.js doesn't require React to be in scope
// 'import/prefer-default-export': 'off',
// '@typescript-eslint/explicit-module-boundary-types': 'off',
Expand Down
2 changes: 1 addition & 1 deletion .prettierrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module.exports = {
printWidth: 200,
printWidth: 100,
tabWidth: 2,
};
3 changes: 2 additions & 1 deletion components/BoardList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Article } from "@/src/types/article";
import Link from "next/link";
import WriterInfo from "./WriterInfo";
import Styles from "./BoardList.module.scss";
import ImgProductEmpty from "@/src/img/Img_product_empty-sm.png";

interface articleListProps {
Copy link
Collaborator

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를 유지하는게 타입스크립트 진영 컨벤션인데요, 이 컨벤션을 따르면 어떨까요?

order: string;
Expand Down Expand Up @@ -58,7 +59,7 @@ export function BoardList({ order = "", pageSize = 0, keyword = "", page = undef
</h3>
{article.image && (
<figure className={Styles.image}>
<Image width="72" height="72" src={article.image} alt="이미지" />
<Image width="72" height="72" src={article?.image ?? ImgProductEmpty} alt="이미지" />
</figure>
)}
</div>
Expand Down
15 changes: 12 additions & 3 deletions components/Header.tsx
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">
Expand All @@ -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="유저 이미지" />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alt는 해당 요소가 무엇인지를 설명할 수 있으면 좋아요.
단순 "유저 이미지" 라는 문구보단, 로그인 사용자의 정보를 갖고 있으니 로그인한 사용자의 아바타 와 같은 이름을 alt로 넣으면 어떨까요

) : (
<Link href="/signin" className="btn-login">
로그인
</Link>
)}
</div>
</div>
</header>
Expand Down
8 changes: 5 additions & 3 deletions components/Input/EmailInput.tsx
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

두가지 개선방향을 이야기 해볼 수 있겠어요.

  1. 공통 input 콤포넌트를 만들고 활용하기
  2. form validation 로직의 관리책임 분리.

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가 업데이트만 되도록 해주면 좀 더 유지보수성이 좋은 프로젝트가 될 거에요

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

또한 입력된 값이 비어있는지 여부만을 관리하기 위해 useState이 사용되고 있는데, 이를 상태로 두어 관리하기보단, value.trim().length === 0 과 같은 fixed 값을 연산하는걸로도 충분히 처리가 가능할텐데요. useState을 활용하게 되면 아무래도 메모리 공간이 낭비되다보니, 단순 연산에 대한 결과를 기록하기 위한 상태 생성은 퍼포먼스에 영향을 미칠 수 있다는 점을 고려하면 좋겠어요!

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);
Expand All @@ -24,6 +25,7 @@ export default function EmailInput({ name, value, id, className, required, setIs
} else {
setIsEmpty(false);
}
onChange(e);
};
return (
<>
Expand Down
2 changes: 1 addition & 1 deletion components/Input/FileInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default function FileInput({ name, value, onChange }: FileInputProps) {

{preview && (
<div className={Styles["file-view__preview"]}>
<img src={preview} alt="이미지 미리보기" className={Styles.img} />
<Image src={preview} width={282} height={282} alt="이미지 미리보기" className={Styles.img} />
<button type="button" onClick={handleClearClick} className={Styles["btn-close"]}>
<Image width="8" height="8" src={icoX} alt="아이콘" aria-hidden="true" />
</button>
Expand Down
9 changes: 6 additions & 3 deletions components/Input/PasswordInput.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { ChangeEvent, useState } from "react";
import { ChangeEvent, ChangeEventHandler, useState } from "react";
import Styles from "./Input.module.scss";

interface PasswordInputProps {
id: string;
name: string;
value: string;
className?: string;
required?: boolean;
inputRef: React.RefObject<HTMLInputElement>;
setIsInvalid: (value: boolean) => void;
onChange: (e: ChangeEvent<HTMLInputElement>) => void;
}

export default function PasswordInput({ id, className, required, inputRef, setIsInvalid }: PasswordInputProps) {
export default function PasswordInput({ id, name, value, className, required, inputRef, setIsInvalid, onChange }: PasswordInputProps) {
const [isEmpty, setIsEmpty] = useState(false);

const handleCheck = (e: ChangeEvent<HTMLInputElement>) => {
Expand All @@ -34,7 +37,7 @@ export default function PasswordInput({ id, className, required, inputRef, setIs
return (
<>
<span className="section-form__pw-box">
<input type="password" className={`${Styles.input} ${className}`} placeholder="비밀번호를 입력해주세요" minLength={8} required={required} ref={inputRef} onChange={handleChange} />
<input type="password" name={name} value={value} className={`${Styles.input} ${className}`} placeholder="비밀번호를 입력해주세요" minLength={8} required={required} ref={inputRef} onChange={handleChange} />
<input type="checkbox" id={id} className="blind chk-visibility" onChange={handleCheck} />
<label htmlFor={id} className="spr visibility-off">
<span className="blind">비밀번호 보기/숨기기</span>
Expand Down
1 change: 1 addition & 0 deletions components/Input/TextInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ interface TextInputProps {
}

export default function TextInput({ name, value, onChange, id, className, required, placeholder }: TextInputProps) {
const [content, setContent] = useState("");
Copy link
Collaborator

Choose a reason for hiding this comment

The 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) {
Expand Down
3 changes: 2 additions & 1 deletion components/ItemCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Image from "next/image";
import icoHeart from "@/src/img/ic_heart.svg";
import { Item } from "@/src/types/item";
import Styles from "./ItemCard.module.scss";
import ImgProductEmpty from "@/src/img/Img_product_empty-sm.png";

interface ItemCardProps {
item: Item;
Expand All @@ -13,7 +14,7 @@ export default function ItemCard({ item }: ItemCardProps) {
<>
<div className={Styles["img-wrap"]}>
<Link href={`/items/${item.id}`} className="link">
<img width="221" height="221" src={item?.images[0]} alt={item.name + " 이미지"} className={Styles.img} />
<Image width="221" height="221" src={item?.images[0] ?? ImgProductEmpty} alt={item.name + " 이미지"} className={Styles.img} />
</Link>
</div>
<div className={Styles.content}>
Expand Down
7 changes: 1 addition & 6 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,4 @@ import globals from "globals";
import pluginJs from "@eslint/js";
import pluginReactConfig from "eslint-plugin-react/configs/recommended.js";


export default [
{languageOptions: { globals: globals.browser }},
pluginJs.configs.recommended,
pluginReactConfig
];
export default [{ languageOptions: { globals: globals.browser } }, pluginJs.configs.recommended, pluginReactConfig];
2 changes: 1 addition & 1 deletion next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
protocol: "https",
hostname: "example.com",
port: "",
pathname: "/**",
pathname: "/...",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example.com 이라는 호스트를 기록관리할 필요가 있나요?
그리구, pathname: "/..." 도 잘못 입력된 것으로 보입니다.

},
{
protocol: "https",
Expand Down
5 changes: 3 additions & 2 deletions pages/_app.tsx
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>
);
}
63 changes: 34 additions & 29 deletions pages/addboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Copy link
Collaborator

Choose a reason for hiding this comment

The 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) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleChangehandleInputChange 둘 다 결국 폼에서 사용된 값을 관리하기 위한 요소인데
이원화된 인터페이스를 제공함으로써 동일한 기능을 위한 여러 함수를 제공하는걸로 사료되어요.
handleChange처럼 단순 함수호출을 간소화 하기 위한 래퍼함수를 또 만들어 사용하기보단,
각 단위의 기능이 명확한 함수를 작성해보면 더 좋겠습니다!

setValues((prevValues) => ({
...prevValues,
[name]: value,
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 폼데이터를 다시 json으로 변환해 서버에 전달하는듯 보이는데
그렇다면 굳이 formdata를 사용할 필요는 없었던게 아닐까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

또한 response가 서버단에서 반환된 객체인가요?
그렇다면 상관없겠지만, 우리가 UI에서 바로 사용할 수 있도록 한번 전처리 되어 꺼내주는 값이라면
response라는 이름보단 좀 더 시맨틱한 변수명으로 사용하는게 좋겠어요

if (!response) return;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!response를 오류가 발생한 모든케이스로 잡으면 안될듯해요.
또한 postArticle 구현체를 보니 에러를 잡아주는 구간이 아예 존재하지 않아서요
그렇다고 nextjs 자체의 _error.tsx나 500tsx도 보이지 않는데, 이 부분은 한번 체계적인 기획이 필요해보입니다!


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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그인 된 사용작 ㅏ없다면 빈 화면을 보여주는게 맞을까요?

return (
<>
<Header />
Expand All @@ -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>
Expand All @@ -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">
Expand All @@ -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>
Expand Down
Loading
Loading