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

[2주차] 송유선 미션 제출합니다. #7

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 14 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"@testing-library/jest-dom": "^5.17.0",
"@testing-library/react": "^13.4.0",
"@testing-library/user-event": "^13.5.0",
"moment": "^2.30.1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

moment 라이브러리를 사용해서 날짜 관리를 하셨네요!!
moment 라이브러리도 정말 좋은 라이브러리이지만, 용량적인 측면에서 리엑트 돔보다 2.3배 뷰보다 3.6배가 더 크다고 합니다. 또한, Moment.js 공식문서에서도 이 라이브러리를 레거시하다고 해서 신규 개발은 하지 않을 예정이라고 하네요..! dayjs 같은 더 가벼운 날짜 관리 라이브러리를 사용하는 것을 추천드려봐요!!

https://velog.io/@hamjw0122/%EC%9A%B0%EB%A6%AC%EA%B0%80-moment.js-%EB%8C%80%EC%8B%A0-day.js%EB%A5%BC-%EC%82%AC%EC%9A%A9%ED%95%B4%EC%95%BC-%ED%95%98%EB%8A%94-%EC%9D%B4%EC%9C%A0

Copy link
Author

Choose a reason for hiding this comment

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

검색하다가 dayjs랑 moment 둘 다 봤는데 아무 생각 없이 그냥 moment 썼었거든요..!! 이런 측면이 있었네요! 다음엔 dayjs로 써볼게요😚

"pretendard": "^1.3.9",
"react": "^18.3.1",
"react-calendar": "^5.0.0",
Copy link

Choose a reason for hiding this comment

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

이번 코드 리뷰를 하며 라이브러리의 활용의 필요성을 뼈저리게 느끼고 있습니다... ㅠㅠ!!!!! 저는 calendar 라이브러리를 활용할 생각을 하지 못했을까요 🥺🥺

그래도 유선 님과 희원 님의 훌륭한 코드를 보며 제 코드를 리팩토링 할 수 있게 되어 기쁩니다!!

우선 리뷰 시작에 앞서 디자인이 너무나 아름다운 것 같습니다... 취향 저격이에용 ㅠㅠ 진행률 bar가 매끄럽게 늘어났다 줄어들었다 하는 것도 인상적입니다... 👍🏻👍🏻

Expand Down
15 changes: 8 additions & 7 deletions src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ function App() {
useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

로컬 스토리지와의 연동을 useEffect 훅을 사용하여 유려하게 잘 구현하신 것 같아요! 훅을 잘 이해하고 사용하고 계신것 같아 멋져요ㅎㅎ

Choose a reason for hiding this comment

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

원투 훅

const today = new Date();
const year = today.getFullYear();
const month = today.getMonth() + 1;
const day = today.getDate();
const month = String(today.getMonth() + 1).padStart(2, "0");
const day = String(today.getDate()).padStart(2, "0");
const formattedToday = `${year}-${month}-${day}`;
setDate(formattedToday);
}, []);
Expand Down Expand Up @@ -51,7 +51,9 @@ function App() {
date={date}
setProgress={setProgress}
/>
{showCalendar && <TodoCalendar setDate={setDate} progress={progress} />}
{showCalendar && (
<TodoCalendar setDate={setDate} progress={progress} todos={todos} />
)}
</TodoContainer>
<FloatingButton onClick={toggleCalendar}>📅</FloatingButton>
</>
Expand All @@ -72,15 +74,14 @@ const TodoContainer = styled.div`

const FloatingButton = styled.button`
Copy link

Choose a reason for hiding this comment

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

이 달력 버튼이 fixed로 고정 되어있어 화면 전체 크기를 줄였을 때 TodoContainer은 반응형으로 줄어들지만 달력 버튼이 밖으로 벗어나지게 됩니다 🥺🥺 또 안의 달력 요소가 밖으로 삐져나오게 되는 부분 수정이 필요할 것 같습니다!!

width: 100%와 column-gap: 40px 때문에, 요소들이 가로로 나열되며 충분한 공간을 차지하는데, 화면 크기가 줄어들면 가로로 배치된 요소들이 더 이상 화면에 맞지 않게 되고, 그 결과 삐져나오는 현상이 발생하는 것 같습니다 ㅠㅠ

flex-wrap 속성을 추가하여 작은 화면에서는 세로 배치로 전환되게 하거나 미디어쿼리를 사용해 보시는 것은 어떨까요?!

더 니은 해결방법이 있다면 공유 부탁드립니다 👍🏻👍🏻

image

Copy link
Author

Choose a reason for hiding this comment

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

제 코드를 열심히 봐주신 것 같아요🥹 사실 저도 달력을 포함해서 그냥 반응형 자체를 이번에 많이 구현하지 못했어서.. 너무 아쉬웠답니다... 다음엔 꼭 민재님 말씀처럼 미디어쿼리 써서 반응형으로 해볼게요👍🏻

position: fixed;
right: 20px;
bottom: 20px;
right: 40px;
bottom: 30px;
width: 50px;
height: 50px;
border-radius: 50%;
background-color: #ff3898;
background-color: #ff3898af;
color: white;
font-size: 24px;
border: none;
cursor: pointer;
box-shadow: 0px 4px 10px rgba(0, 0, 0, 0.2);
`;
21 changes: 19 additions & 2 deletions src/components/ProgressBar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import styled from "styled-components";
const ProgressBar = ({ progress }) => {
return (
<ProgressBarWrapper>
<Progress style={{ width: `${progress}%` }} />
<Progress style={{ width: `${progress}%` }}>
{progress > 0 && progress < 100 && <Tooltip>{`${progress}%`}</Tooltip>}
</Progress>
</ProgressBarWrapper>
);
};
Expand All @@ -14,14 +16,29 @@ export default ProgressBar;
const ProgressBarWrapper = styled.div`
width: 100%;
height: 20px;
background-color: white;
background-color: #ffffff1a;
border-radius: 10px;
margin-top: 20px;
`;

const Progress = styled.div`
position: relative;
height: 100%;
background-color: #ff3898;
border-radius: inherit;
transition: width 0.3s ease-in-out;
`;

const Tooltip = styled.div`
position: absolute;
right: -14px;
top: -33px;
font-size: 11px;
font-weight: 500;
color: #24d46d;
padding: 4px 6px;
border-radius: 5px;
border: 0.3px solid #24d46d;
background: #ffffff1a;
z-index: 1;
`;
112 changes: 108 additions & 4 deletions src/components/TodoCalendar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,36 @@ import Calendar from "react-calendar";
import styled from "styled-components";
import "react-calendar/dist/Calendar.css";
import ProgressBar from "./ProgressBar";
import moment from "moment";

const TodoCalendar = ({ setDate, progress }) => {
const TodoCalendar = ({ setDate, todos, progress }) => {
const handleDateChange = (selectedDate) => {
const year = selectedDate.getFullYear();
const month = selectedDate.getMonth() + 1;
const day = selectedDate.getDate();
const month = String(selectedDate.getMonth() + 1).padStart(2, "0");
const day = String(selectedDate.getDate()).padStart(2, "0");
const formattedDate = `${year}-${month}-${day}`;
setDate(formattedDate);
};

return (
<CalendarWrapper>
<Calendar onChange={handleDateChange} />
<Calendar
locale="ko"
onChange={handleDateChange}
next2Label={null}
prev2Label={null}
formatDay={(locale, date) => moment(date).format("DD")}
showNeighboringMonth={false}
minDetail="year"
tileContent={({ date }) => {
const existTodos = todos.some(
Copy link
Collaborator

Choose a reason for hiding this comment

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

tileContent에서 todos.some()을 통해 모든 날짜마다 todos 배열을 순회해 일치하는 항목을 찾는 방식으로 구현되어 있는데,이러한 방식보다, useMemo()를 사용해서 todos 배열을 미리 날짜별로 매핑하는 방식으로 변경하는 것은 어떨지 추천드려요! 그래서 todos가 변경될 때만 날짜별 투두 항목을 매핑을 업데이트 하면, 더욱 효율적일 것 같단 생각입니다!!!

Copy link
Author

@s-uxun s-uxun Sep 24, 2024

Choose a reason for hiding this comment

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

와..! 제가 봐도 더 효율적인 방법같아요..!!😂👍🏻 다음엔 이런 방식으로도 써볼게요!

(todo) => todo.date === moment(date).format("YYYY-MM-DD")
);
return existTodos ? (
<StyledDot key={moment(date).format("YYYY-MM-DD")} />
) : null;
Comment on lines +33 to +35

Choose a reason for hiding this comment

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

간단하게 표시해도 좋을 것 같아요

Suggested change
return existTodos ? (
<StyledDot key={moment(date).format("YYYY-MM-DD")} />
) : null;
return existTodos && <StyledDot />

}}
/>
<ProgressBar progress={progress} />
</CalendarWrapper>
);
Expand All @@ -26,4 +43,91 @@ export default TodoCalendar;
const CalendarWrapper = styled.div`

Choose a reason for hiding this comment

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

프론트는 디자인이 생명인데 ..
image
여기까지할게요

Copy link
Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

Copy link
Author

@s-uxun s-uxun Sep 24, 2024

Choose a reason for hiding this comment

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

아니 내비게이션까지 다 볼 줄 몰랐는데...

display: flex;
flex-direction: column;
height: 95%;
justify-content: space-evenly;

.react-calendar {
background-color: #ffffff1a;
border: none;
border-radius: 2rem;
color: white;
padding: 3% 5%;
}

.react-calendar__month-view {
abbr {
color: white;
}
}

.react-calendar__navigation {
justify-content: center;
}

.react-calendar__navigation button {
font-weight: 600;
font-size: 1rem;
color: #ff3898;
}

.react-calendar__navigation button:focus,
.react-calendar__navigation button:hover,
.react-calendar__navigation button:disabled {
background-color: none;
border-radius: 0.8rem;
}

.react-calendar__month-view__weekdays abbr {
text-decoration: none;
font-weight: 800;
}

.react-calendar__month-view__weekdays__weekday abbr {
color: #cb84a6;
}

.react-calendar__tile--now {
background: none;
abbr {
color: #ff3898;
}
}

.react-calendar__year-view__months__month {
border-radius: 0.8rem;
background-color: none;
padding: 0;
}

.react-calendar__tile--hasActive {
background-color: ${(props) => props.theme.primary_2};
abbr {
color: none;
}
}

.react-calendar__tile {
padding: 5px 0px 18px;
position: relative;
}

.react-calendar__tile:enabled:hover,
.react-calendar__tile:enabled:focus,
.react-calendar__tile--active {
background-color: #ff38981e;
border-radius: 0.3rem;
}
`;

export const StyledCalendar = styled(Calendar)``;

export const StyledDot = styled.div`
background-color: #ff3898;
border-radius: 50%;
width: 0.3rem;
height: 0.3rem;
position: absolute;
top: 70%;
left: 50%;
transform: translateX(-50%);
`;
3 changes: 2 additions & 1 deletion src/components/TodoContent.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const TodoContent = ({ todos, setTodos, date, setProgress }) => {

const todoCount = useMemo(() => {
const doneCount = todos.filter((todo) => todo.done).length;
const progress = todos.length === 0 ? 0 : (doneCount / todos.length) * 100;
const progress =
todos.length === 0 ? 0 : Math.round((doneCount / todos.length) * 100);
setProgress(progress);

return `${doneCount} / ${todos.length}`;
Expand Down