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

Junsbae todo, 개포동 배달음식, cabi 웹 사이트 만들기 제출, react 로그인 페이지, react cabi 페이지 구현 #16

Open
wants to merge 5 commits into
base: junsbae
Choose a base branch
from

Conversation

wet6123
Copy link

@wet6123 wet6123 commented Aug 29, 2024

  • todo list
  • 개포동 배달음식
  • cabi 웹 사이트
  • React Cabi login 페이지
  • React Cabi main 페이지

@wet6123 wet6123 self-assigned this Aug 29, 2024
@wet6123 wet6123 changed the title Junsbae todo, 개포동 배달음식, cabi 웹 사이트 만들기 제출 Junsbae todo, 개포동 배달음식, cabi 웹 사이트 만들기 제출, react 로그인 페이지 구현 Sep 6, 2024
@wet6123 wet6123 changed the title Junsbae todo, 개포동 배달음식, cabi 웹 사이트 만들기 제출, react 로그인 페이지 구현 Junsbae todo, 개포동 배달음식, cabi 웹 사이트 만들기 제출, react 로그인 페이지, react cabi 페이지 구현 Sep 9, 2024
Copy link
Member

Choose a reason for hiding this comment

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

applet, h1 과 같이 사용하지 않는 코드는 삭제하는게 깔끔할것같습니당

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 추가적으로 작성한 거면 몰라도, reset.css 에 기본적으로 포함된 요소들이라 극한의 최적화를 위해서가 아니라면 굳이 안 지워도 될 것 같다고 생각합니다...!
물론 지우면 그만큼 최종 크기는 줄겠지만요

Copy link
Member

Choose a reason for hiding this comment

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

@junyoung2015 reset.css가 무슨 파일인가요? 처음 보는 파일이네여

Copy link
Member

Choose a reason for hiding this comment

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

reset.css랑 style.css랑 나눠서 사용하는 이유가 궁금합니당

</main>
<nav id="tapbar">
<ul>
<li class="tapbar-btn">Home</li>
Copy link
Member

Choose a reason for hiding this comment

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

35~38행과 이 부분에서 Home, 2층, 4층, 5층이 하드코딩으로 돼있는데 2군데 이상에서 사용되는 값은 변수로 정의해 사용하는게 유지보수면에서 좋아서 변수 사용을 추천합니다

prevSectionSlide,
} from "./src/section/sectionBtn/slideSection.js";

let state = {
Copy link
Member

Choose a reason for hiding this comment

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

어떤 변수인지 좀 더 구체적인 네이밍을 쓰는게 오랜 시간 지나고 난 후 다시 봤을때 또는 다른 팀원이 봤을때 파악하기가 수월할것같슴다

height: 100vh;
`;

const LadingPage = () => {
Copy link
Member

Choose a reason for hiding this comment

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

오타 발견!

import styled from "styled-components";
import intro from "../img/intro-img.svg";

const StyledSection = styled.section`
Copy link
Member

Choose a reason for hiding this comment

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

참고로 까비에선 styled components 정의를 코드 맨 아래에 배치하고, 끝에 Styled를 작성합니당

body,
div,
span,
applet,
Copy link
Member

Choose a reason for hiding this comment

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

applet, h1 과 같이 사용하지 않는 코드는 삭제하는게 깔끔할것같습니당
코멘트 남기는 사이 outdated 돼서 여기도 남깁니다

@@ -0,0 +1,213 @@
html {
Copy link
Member

Choose a reason for hiding this comment

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

reset.css랑 style.css랑 나눠서 사용하는 이유가 궁금합니당

@jnkeniaem
Copy link
Member

과제 요구 사항들 구현하신것 잘 확인했습니다
온보딩하느라 고생하셨습니다~

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.

3 participants