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

[김현서] Week14 #452

Conversation

khs0727
Copy link
Collaborator

@khs0727 khs0727 commented May 19, 2024

요구사항

기본

  • 로그인, 회원가입 페이지를 만들어 주세요.
  • 로그인 페이지의 url path는 ‘/signin’, 회원가입 페이지의 url path는 ‘/signup’ 입니다.

심화

  • 로그인, 회원가입 기능에 react-hook-form을 활용해 주세요.

주요 변경사항

  • api 라우팅 사용

멘토에게

  • 아직 타입스크립트가 어려워서 Card 컴포넌트를 두 개로 나누게 됐는데 더 좋은 방법을 알려주시면 감사하겠습니다..!

@khs0727 khs0727 requested a review from devToram May 19, 2024 14:13
@khs0727 khs0727 added the 순한맛🐑 마음이 많이 여립니다.. label May 19, 2024
Copy link
Collaborator

@devToram devToram left a comment

Choose a reason for hiding this comment

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

수고하셨습니다:)

Comment on lines +9 to +13
if (response.status === 200) {
return true; //로그인 성공
} else {
return false; //로그인 실패
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (response.status === 200) {
return true; //로그인 성공
} else {
return false; //로그인 실패
}
return response.status === 200

<div className={styles.buttonwrapper}>
<div className={styles.actionbutton}>
<Image
className={styles.actionbutton_image}
Copy link
Collaborator

Choose a reason for hiding this comment

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

classname 규칙 맞춰주셔요!

import styles from "./AllButton.module.css";

const AllButton = ({ active, onClick }) => {
interface AllButtonProps {
active: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

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

boolean 값은 is~, has~ 로 시작하는 게 일반적이라 isActive 추천드려요!

Comment on lines +11 to +15
<button
className={`${styles.StyledButton} ${buttonClass}`}
onClick={onClick}
color-type={btnType}
>
Copy link
Collaborator

Choose a reason for hiding this comment

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

ButtonHTMLAttributes 을 쓰신만큼 바로 활용해주셔도 좋을 거 같아요!

Suggested change
<button
className={`${styles.StyledButton} ${buttonClass}`}
onClick={onClick}
color-type={btnType}
>
const Button = ({ children, btnType, onClick, ...rest }: ButtonProps) => {
...
<button
className={`${styles.StyledButton} ${buttonClass}`}
onClick={onClick}
color-type={btnType}
...rest
>

Comment on lines +35 to +38
{Array.isArray(link) ? (
link.map((item, index) => (
<Card key={index} link={item} isFolderPage={isFolderPage} />
))
Copy link
Collaborator

Choose a reason for hiding this comment

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

스크린샷 2024-05-21 오전 12 18 42

러프하게 생각해봤을 때 2차원 배열이면 위의 사진과 같이 1차원 배열로 만들어서 넣어주는 방법이 있구용,,! 아니면 이 컴포넌트를 부르는 곳에서 직접 map 을 해서 넣어주시는 것도 좋을 거 같아요!

const { data: folderData, isLoading } = useFolderList();

if (isLoading) return <div>Loading...</div>;
if (!folderData) return null;

const folderId = null;
const folderId = "";
Copy link
Collaborator

Choose a reason for hiding this comment

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

const 값을 굳이 정해줄 필요 없을 거 같아요!

Comment on lines +35 to +39
if (modalState[modalType]) {
closeModal(modalType);
} else {
openModal(modalType);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

early return 적용해주시면 좋을 거 같아요~

if (!Array.isArray(folders) || folders.length === 0) {
interface FolderMenuListProps {
folders: Folder;
activeButton: number | string | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

activeButtonId 일 거 같아요...? (진짜 DOM 이 아니라면용)

return (
<div className={styles.footericons}>
{SOCIAL_SHARES.map((share, index) => (
<Link key={index} href={share.url} target="_blank">
Copy link
Collaborator

Choose a reason for hiding this comment

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

key 를 index 로 주는 건 안주는 것과 동일해요! share.url 은 아마도 고유한 값일 거 같은데 이 경우 고유한 값을 넣어주면 좋을 거 같습니다!

@devToram devToram merged commit 9c87efd into codeit-bootcamp-frontend:part3-김현서 May 20, 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.

3 participants