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

KDT0_LeeYeongUk #60

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

KDT0_LeeYeongUk #60

wants to merge 1 commit into from

Conversation

wowba
Copy link

@wowba wowba commented Aug 18, 2023

No description provided.

@wowba wowba changed the title commit KDT0_LeeYeongUk Aug 18, 2023
Comment on lines +1 to +12
export default function validation(data) {
const {name, email, team, job, position, img} = data

if (
name === "" || email === "" || team === "" ||
job === "" || position === "" || img === ""
) {
return false
} else {
return true
}
}

Choose a reason for hiding this comment

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

직원등록페이지에서 빈칸체크하는데 쓰신 로직같은데, 짧고 깔끔한 코드인 것 같아요!
추후에 회원가입기능이나 로그인 기능 만들 때 참고할 수 있을 것 같습니다👍

Copy link

@Eojoonhyuk Eojoonhyuk left a comment

Choose a reason for hiding this comment

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

폴더구조가 깔끔한거같아요!

Copy link

@zoeyourlife zoeyourlife 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 +1 to +16
import Component from "../../core/Component";

export default class CreateButton extends Component {
constructor() {
super({
tagName: "button"
})
}
render() {
this.el.classList.add("create-button")
this.el.innerText = "임직원 생성"
this.el.addEventListener("click", () => {
location.href = '#/create'
})
}
}

Choose a reason for hiding this comment

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

전반적으로 각 기능 별로 컴포넌트화 해서 개발하신 코드라 디렉토리 구조 보기도 편하고 좋았습니다.

@applevalley
Copy link

GraphQL 사용하신 부분이 인상적이었습니다!

이전까지는 아무런 고민 없이 REST API만을 사용하곤 하였는데요. 그 전의 경험을 되돌아보니 원하는 것은 객체 내 특정 필드 일부분인데 단순 조회 요청을 통해 너무 많은 데이터를 받아오게 되거나, 한 객체에 대한 정보로는 내용이 부족해 다른 객체에 대한 조회 요청까지 보내게 되었던 아쉬움이 있었습니다.

그 과정에서 불필요한 요청도 많아지게 되고, 무엇보다 사용자 경험에도 직접적인 영향을 주게 되었던 것 같습니다.
GraphQL을 통해 필요한 필드만 응답을 받게 만든다면, 이런 문제를 해결할 수 있을 것 같다는 생각을 하게 되었습니다!

Copy link
Member

@iamidlek iamidlek left a comment

Choose a reason for hiding this comment

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

깔끔하고 필요한 기능을 잘 구현하신 것 같습니다.
컴포넌트를 잘 나누어 구성하고
각 컴포넌트의 역할이 명확하여 좋은 것 같습니다.
특별히 코멘트 할 부분은 없는 것 같습니다.

고생하셨습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants