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

[전수빈] week13 #467

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .babelrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"presets": ["next/babel"],
"plugins": [
[
"babel-plugin-styled-components",
{
"fileName": true,
"displayName": true,
"pure": true
}
]
]
}
71 changes: 71 additions & 0 deletions components/button/AddFloatingButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import styled from "styled-components";
import FolderAddWhIcon from "@/public/assets/button/img_add.png";
import { device } from "@/styles/globalStyle";
import Image from "next/image";
import { modalState } from "@/recoil/modal";
import { useRecoilState } from "recoil";

const AddFloatingBtn = () => {
const [, setModalOpened] = useRecoilState(modalState);

return (
<AddFloatingBtnContainer
onClick={() =>
setModalOpened((prev: any) => ({
...prev,
defaultModal: {
display: true,
content: {
id: 0,
title: "",
},
state: "folderAdd",
},
}))
}
>
<div className="floatingBtnTitle">폴더 추가</div>
<Image
width="16"
height="16"
className="floatingBtnIcon"
src={FolderAddWhIcon}
alt="folderAddIcon"
/>
</AddFloatingBtnContainer>
);
};

export default AddFloatingBtn;

const AddFloatingBtnContainer = styled.div`
display: none;

@media all and (${device.mobile}) {
display: flex;
align-items: center;
justify-content: center;
padding: 0.8rem 2.4rem;
border-radius: 2rem;
border: 1px solid #fff;
background: var(--primary);
width: fit-content;
height: 3.7rem;
box-sizing: border-box;
gap: 0.4rem;
position: fixed;
z-index: 5;
bottom: 10.1rem;
margin: 0 auto;
left: 50%;
transform: translate(-50%, 0);

.floatingBtnTitle {
color: var(--gray10);
text-align: center;
font-size: 1.6rem;
font-weight: 500;
letter-spacing: -0.3px;
}
}
`;
35 changes: 35 additions & 0 deletions components/button/DefaultButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import styled from "styled-components";

interface IProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

interface 선언시 I- prefix를 붙이는것은 잘 받아들여지는 컨벤션은 아닙니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Prop type을 선언하실때는 ${ComponentName}Props컨벤션을 사용하시는 것이 좋습니다.

type DefaultButtonProps = {};
export default function DefaultButton() {}

children: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

button 안에 string만 받기보다는 ReactNode 를 받아야 span태그를 받을수도, svg 아이콘을 받을수도 있습니다.

onClick: (e: React.MouseEvent) => void;
type: "red" | "default";
Copy link
Collaborator

Choose a reason for hiding this comment

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

red 라는 타입은 너무 변경에 취약합니다.
예를들어 현재 개발할때는 red 가 강조 색상이었는데, 개편 후 blue 가 강조색상이 된다면 코드를 전체적으로 뜯어고쳐야 하는 상황이 나옵니다.
특정 색깔이 아니라 해당 색상이 의미하는 바가 무엇인지를 type으로 받으면 좋습니다.
일반적으로 primary | secondary 와 같은 variant를 사용합니다.

}

const DefaultBtn = ({ children, onClick, type }: IProps) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

컴포넌트명은 축약어 (Btn)를 이용하지 마시고 명확하게 full name을 사용해주세요 DefaultButton

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한 DefaultButton이라는 이름 자체가 어색합니다. Button의 기본은 Button입니다.
Button을 확장한 다른 버튼이 나올수 있구요

return (
<DefaultBtnContainer onClick={onClick} type={type}>
{children}
</DefaultBtnContainer>
);
};

export default DefaultBtn;

export const DefaultBtnContainer = styled.button<{ type: string }>`
border-radius: 0.8rem;
background: ${(props) =>
props.type === "red"
? "var(--red)"
: "linear-gradient(91deg, var(--primary) 0.12%, #6ae3fe 101.84%)"};
Copy link
Collaborator

Choose a reason for hiding this comment

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

확장성이 매우 떨어지는 로직이죠?
이외 케이스에는 전혀 대응을 할 수 없습니다.

일단 저라면 아래와 같이 처리할 것 같습니다.

  1. Button(DefaultButton) 은 기본 시스템 색상 (primary, secondary)에 대한 단색 디자인을 제공한다.
  2. styled component의 확장 기능을 이용해 GradientButton 을 하나 더 만든다.
export const Button = styled.button<{type: 'primary' | 'secondary'}>`
  color: ${(props) => `var(--${props.type})`}
`;
export const GradientButton = styled(Button)`
  color: linear-gradient(91deg, ${(props) => `var(--${props.type})`}, 0.12%, #6ae3fe 101.84%)
`

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jayjnu
멘토님 궁금한 부분이 있습니다.
현재 버튼이 그라데이션 버튼, 삭제버튼(type:red - 빨간색버튼) 두가지가 존재합니다.
말씀주신 방법에서 따로 type을 받지 않고 기본 color: red로만 처리 후 이를 확장해서 그라데이션 버튼을 만드는 것도 괜찮은가요?
이렇게 하는 이유가 이후에 단색 버튼 색상 추가 혹은 현재 버튼 색상의 변화 등에 대응하기 위해서라고 이해했는데 맞을까요?
아니면 Button을 그라데이션 디자인으로 사용 후 이를 확장해서 DeleteButton(빨간색 버튼)으로 만드는 것이 좋을까요?
디자인에서 기본적으로 사용되는 버튼이 그라데이션 버튼이라 위와 같은 생각이 들었습니다. 혹시 이 방법이 확장성이 떨어지는 형태일까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

오늘 말씀드린것처럼 확장에는 열려있되 수정에는 닫혀있게 하기 위함입니다.
만약 추가적인 요구사항이 나올때마다 Button을 수정하거나 새로운 컴포넌트를 만들어야 한다면 확장성이 떨어진다고 볼 수 있겠져

color: #f5f5f5;
padding: 1.6rem 2rem;
font-size: 1.8rem;
font-weight: 600;
line-height: 2.2rem;
box-sizing: border-box;
display: flex;
justify-content: center;
align-items: center;
cursor: pointer;
`;
165 changes: 165 additions & 0 deletions components/card/Card.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
import moment from "moment";
Copy link
Collaborator

Choose a reason for hiding this comment

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

moment는 browser js에서 사용하기에 너무 무거워서 사용을 지양하셔야 합니다.
만약 시간 계산관련 유틸리티가 필요하면 date-fns 를 추천드립니다.

import LogoImg from "@/public/assets/common/img_logo.png";
import {
CardContainer,
CardImgContainer,
CardWrapper,
ContentContainer,
OptionMenuContainer,
} from "./cardStyled";
import OptionIcon from "@/public/assets/card/img_option.png";
import StarIcon from "@/public/assets/card/img_star.png";
import { useState } from "react";
import Image from "next/image";

const calculateTimeAgo = (createdAt: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

별도의 utility 함수로 분리하는 것이 좋습니다.
Card에서만 사용하긴 하지만, Card의 주요 관심사는 아니므로 코드에 많은 노이즈가 낍니다.

const createdDate = moment(createdAt, "YYYY-MM-DDTHH:mm:ss[Z]");
const currentDate = moment();
const diff = currentDate.diff(createdDate, "seconds");

if (diff < 120) {
return "1 minute ago";
} else if (diff <= 3540) {
return `${Math.floor(diff / 60)} minutes ago`;
} else if (diff < 3600) {
return "1 hour ago";
} else if (diff <= 82800) {
return `${Math.floor(diff / 3600)} hours ago`;
} else if (diff < 86400) {
return "1 day ago";
} else if (diff <= 2592000) {
return `${Math.floor(diff / 86400)} days ago`;
} else if (diff <= 28512000) {
return `${Math.floor(diff / 2592000)} months ago`;
} else if (diff <= 31536000) {
return "1 year ago";
} else {
return `${Math.floor(diff / 31536000)} years ago`;
}
};

interface ICardProp {
cardData: any;
onClickDelete?: any;
onClickAdd?: any;
isFolder: boolean;
}

const Card = ({ cardData, onClickDelete, onClickAdd, isFolder }: ICardProp) => {
const ago = calculateTimeAgo(cardData.created_at || cardData.createdAt);
Copy link
Collaborator

Choose a reason for hiding this comment

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

일정시간이 지났음을 표현할 때 좋은 단어로는 elapse라는 동사가 있습니다. timesElapsedText 와 같은 이름을 붙이면 더 직관적이지 않을까 싶네요

const createdAtFormat = moment(
cardData.created_at || cardData.createdAt
).format("YYYY.MM.DD");
const [isOpenOption, setIsOpenOption] = useState(false);

const openUrl = () => {
window.open(cardData.url, "__blank");
Copy link
Collaborator

Choose a reason for hiding this comment

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

정말 피치못할 사정이 있는것이아니면 영역을 클릭했을 때 url 이동이 필요하거나 새 페이지를 열거나 하는 기능이 필요한 경우, 해당 기능은 a 태그를 이용해서 구현하시는 것이 좋습니다.

};

return (
<CardWrapper>
<CardContainer onClick={openUrl}>
{cardData.image_source || cardData.imageSource ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

이미지 컴포넌트 렌더링영역에만 분기를 치면 어떨까 싶어요
불필요하게 StartIcon 로직도 중복이 발생하고, 코드를 다 읽어봐야 a일때와 b일때 뭐가 다른지 파악이 가능해서 가독성이 떨어집니다.

<CardImgContainer>
<Image
priority
className="cardImage"
src={cardData.image_source || cardData.imageSource}
alt="cardImg"
fill
/>
{isFolder && (
<Image
src={StarIcon}
className="starIcon"
alt="starIcon"
width="34"
height="34"
/>
)}
</CardImgContainer>
) : (
<CardImgContainer>
<Image
priority
src={LogoImg}
alt="logoImg"
className="noImgLogo"
width="133"
height="24"
/>
{isFolder && (
<Image
src={StarIcon}
className="starIcon"
alt="starIcon"
width="34"
height="34"
/>
)}
</CardImgContainer>
)}

<ContentContainer>
<div className="contentOptionContainer">
<div className="contentAgo">{ago}</div>
{isFolder && (
<Image
className="optionBtn"
src={OptionIcon}
alt="optionIcon"
width="21"
height="17"
onClick={(e) => {
e.stopPropagation();
setIsOpenOption(!isOpenOption);
}}
/>
)}
</div>
<div className="content">{cardData.description}</div>
<div className="contentAt">{createdAtFormat}</div>
</ContentContainer>
</CardContainer>

{isOpenOption && (
<OptionMenu
onClickDelete={onClickDelete}
onClickAdd={onClickAdd}
content={{ id: cardData.id, title: cardData.url }}
/>
)}
</CardWrapper>
);
};

export default Card;

interface IOptionMenuProp {
onClickDelete: any;
onClickAdd: any;
content: {
id: number;
title: string;
};
}

const OptionMenu = ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

하나의 컴포넌트 파일에는 하나의 컴포넌트만 정의해주시면 좋을 것 같네요.
어차피 card라는 폴더로 묶어두셨으니 내부 컴포넌트는 별도 파일로 분리해도 문제 없을듯합니다.

onClickDelete,
onClickAdd,
content,
}: IOptionMenuProp) => {
return (
<OptionMenuContainer>
<div
className="optionMenuItem"
onClick={() => onClickDelete("linkDelete", content)}
>
삭제하기
</div>
<div className="optionMenuItem" onClick={() => onClickAdd()}>
폴더에 추가
</div>
</OptionMenuContainer>
);
};
Loading