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

Next.js 이동규 sprint2 #636

Conversation

ldkstellar
Copy link
Collaborator

@ldkstellar ldkstellar commented Jun 7, 2024

요구사항

기본

상품 등록 페이지 주소는 “/addboard” 입니다.(o)

  • 게시판 이미지는 최대 한개 업로드가 가능합니다.(o)

  • 각 input의 placeholder 값을 정확히 입력해주세요.(o)

  • 이미지를 제외하고 input 에 모든 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.(o)

  • 회원가입, 로그인 api를 사용하여 받은accessToken을 사용하여 게시물 등록을 합니다.(x)

  • ‘등록’ 버튼을 누르면 게시물 상세 페이지로 이동합니다.(x)

  • 상품 상세 페이지 주소는 “/addboard/{id}” 입니다.(x)

  • 댓글 input 값을 입력하면 ‘등록' 버튼이 활성화 됩니다.(x)

  • 활성화된 ‘등록' 버튼을 누르면 댓글이 등록됩니다.(x)

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@ldkstellar ldkstellar requested a review from endmoseung June 7, 2024 12:29
@ldkstellar ldkstellar added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Jun 7, 2024
Copy link
Collaborator

@endmoseung endmoseung left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 동규님!
제가 리뷰 드린부분 해결하시나 훨씬 코드의 가독성이 좋아진것 같네요!
조금 귀찮더라도 코드를 작성하는 습관을 들이시면 추후에 정말 도움이 될거에요~

src/api/api.ts Outdated
export const getBestPosts = async (params: string): Promise<writing[]> => {
const URL = `/articles?${params}`;
try {
const response: AxiosResponse<any> = await instance.get(URL);
const response: AxiosResponse<articles> = await instance.get(URL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

src/api/api.ts Outdated
@@ -30,7 +24,7 @@ export const getBestPosts = async (params: string): Promise<writing[]> => {
export const getTotalPosts = async (params: string): Promise<writing[]> => {
const URL = `/articles?${params}`;
try {
const response: AxiosResponse<any> = await instance.get(URL);
const response: AxiosResponse<articles> = await instance.get(URL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Api Type은 암묵적으로 앞글자를 대문자로 넣고 뒤에 ~~Type을 넣어줘요 아니면 컴포넌트랑 헷갈릴 수 있어요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵 알겠습니다.

Comment on lines +43 to +50
if (!Cookies.get('accessToken')) {
const response = await instance.post(TEMP_URL, {
email: '[email protected]',
password: 'abcd1234',
});
const { accessToken, refreshToken } = response.data;
Cookies.set('accessToken', accessToken);
Cookies.set('refreshToken', refreshToken);
Copy link
Collaborator

Choose a reason for hiding this comment

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

accessToken이나 refreshToken같은 키값은 따로 상수로 관리하는것도 좋아요.
왜냐하면 해당 키값들을 상수로 관리하면 흩어져있는 값들에 대한 관리가 가능하고 휴먼에러의 오류가 줄어드렁요.

Cookies.set('accessToken', accessToken);
Cookies.set('refreshToken', refreshToken);
}
} catch (error) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

error는 throw해줘서 해당 에러를 catch하는 곳에서 처리를 해주셔도 좋겠네요:)

src/api/api.ts Outdated
Comment on lines 55 to 62
export const postImage = async () => {
const URL = '/images/upload';

try {
const response = await instance.post(URL);
response.data();
} catch (error) {}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

만약 repsonse로 받은 값을 쓰지 않는다면 이렇게 해줘도 되겠네요!

Suggested change
export const postImage = async () => {
const URL = '/images/upload';
try {
const response = await instance.post(URL);
response.data();
} catch (error) {}
};
export const postImage = async () => {
const URL = '/images/upload';
try {
await instance.post(URL);
} catch (error) {}
};

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

감사합니다!

Comment on lines +14 to +23
if (e.target.name === 'title') {
setFormData((prev) => ({ ...prev, ['title']: e.target.value }));
} else if (e.target.name === 'content') {
setFormData((prev) => ({ ...prev, ['content']: e.target.value }));
} else if (e.target.name === 'image') {
const file = e.target.files?.[0];
if (file) setFormData((prev) => ({ ...prev, ['image']: file }));
}
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기서 e.target.name가 재사용되니 const targetName = e.target.name;으로 재사용해도 도겠네요

e.preventDefault();
};
useEffect(() => {
tempSignUP();
Copy link
Collaborator

Choose a reason for hiding this comment

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

tempSignup은 비동기함수인데 사용하는곳에서 비동기 설정을 안해주시면 그냥 동기함수처럼 사용됩니다! 여기도 비동기 함수로 바꿔주세요:)

Comment on lines +19 to +21
type='submit'
disabled={!(formData.title && formData.content)}
className={style.registerBtn}
Copy link
Collaborator

Choose a reason for hiding this comment

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

좋아요! 여기서 Type을 button으로 바꿔주면 form으로 엔터쳣을때 동작 안할거에요!

@@ -5,9 +5,13 @@ import SearchBar from './SearchBar';
const SelectAndSearchContainer = () => {
const router = useRouter();
const ref = useRef<HTMLInputElement>(null);
const [selectedOption, setSelectedOption] = useState('recent');
const [selectedOption, setSelectedOption] = useState<'recent' | 'like'>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Comment on lines +4 to 11
const day = date.getDay();
const month = attachZero(date);
return `${year}. ${month}. ${day} `;
};

const attachZero = (date: Date) => {
let month: number | string = date.getMonth() + 1;
if (month < 10) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@endmoseung endmoseung merged commit c5e1276 into codeit-bootcamp-frontend:Next.js-이동규 Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants