Feat: 일정 상세 페이지 추가 #58
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ); | ||
| }; | ||
|
|
||
| export default KeywordFilter; |
There was a problem hiding this comment.
이 코드 패치는 일정을 검색하기 위한 키워드 필터 컴포넌트를 구현하고 있습니다. 코드 자체에는 큰 문제가 없어 보이지만 몇 가지 개선 제안이 있습니다:
-
keywordFilterType과KeywordFilterProps의 인터페이스 이름을 보충적인 용어로 변경하는 것이 좋습니다. 예를 들어, 'KeywordOption' 및 'KeywordFilterComponentProps'와 같은 이름으로 바꿀 수 있습니다. -
<select>요소의name속성 값을 'option'에서 더 구체적이고 의미 있는 값으로 변경하는 것이 좋습니다. -
handleKeyDown함수의 매개변수인e의 타입을 더 명확하게 지정하는 것이 좋습니다. 예를 들어,KeyboardEvent<HTMLInputElement>대신에 `React.KeyboardEvent<HTMLInputE
| /> | ||
| <ScheduleContent /> | ||
| </div> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
해당 코드는 리액트 컴포넌트를 사용하는 스케줄 페이지의 일부분입니다. 몇 가지 개선점과 버그 위험을 제안하겠습니다:
- 초기 필터링 조건을 담고 있는
initialFilter객체는defaultCardList와 함께 받도록 변경합니다. filter,keywordFilter,sortFilter, 그리고cardList를 관리하기 위해 상태(useState)를 사용합니다.useEffect훅을 활용하여 필터링된 정보를 가져오는 비동기 요청을 수행합니다.- 기존 상태를 관리하던 함수들을 새로운 상태로 이관합니다.
- 액시오스(Axios) 라이브러리를 사용하여 API 요청을 보냅니다.
- JSX에서 필요한 속성 및 이벤트 처리기를 전달합니다.
이러한 수정으로 코드의 가독성과 유지보수 용이성을 향상시킬 수 있습니다.
| <label | ||
| key={`range-${i}`} | ||
| className="flex items-center mt-3 text-sm font-light text-zinc-800" | ||
| > | ||
| <input | ||
| type="radio" | ||
| className="w-5 h-5 radio" | ||
| name="personnelRange" | ||
| name="personnel" | ||
| value={range.value} | ||
| checked={personnel === range.value} | ||
| onChange={handlePersonnelChange} |
There was a problem hiding this comment.
위의 코드 패치에 대해 간단한 코드 리뷰를 도와드리겠습니다. 버그 위험 및 개선 제안을 환영합니다:
- 수정 전:
import { PersonnelRanges } from "@findSchedule/const"; - 수정 후:
import { personnelRanges } from "@findSchedule/const";
변경된 내용:
PersonnelRanges를personnelRanges로 변경하였습니다.
추가적으로, 같은 파일에서 다음과 같은 부분도 수정되었습니다:
- 수정 전:
name="personnelRange" - 수정 후:
name="personnel"
수정 사항:
PersonnelRanges를personnelRanges로 변경하여 변수 이름이 소문자 형태로 맞추어졌습니다.name속성을"personnelRange"에서"personnel"로 수정하여 일관성 있게 변수 이름이 설정되었습니다.
개선 사항:
코드 패치를 보고는 추가적인 버그 위험을 파악하기는 어렵지만, 주어진 코드 블록 자체에서는 명확한 문제가 없어 보입니다. 개발 콘텍스트 및 기능 요구사항에 따라 해당 코드의 안전성과 작동 방식을 검토하시면 좋을 것 같습니다.
| <div className="w-[260px] h-[420px] m-auto"></div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default ScheduleContent; |
There was a problem hiding this comment.
해당 코드는 React 컴포넌트인 ScheduleContent를 수정한 내용입니다. 몇 가지 개선 사항과 버그 리스크를 살펴보겠습니다:
- 개선 사항:
- Line 2:
ChangeEvent를 구조분해 할당으로 가져온 것은 좋습니다. - Line 4-5: Card와 KeywordFilter, SortFilter를 import하는 것은 잘못된 경로를 수정하였다고 가정합니다.
- 버그 리스크:
- Line 26:
<KeywordFilter>컴포넌트에onClickSearch프롭을 전달하려 하지만, 해당 컴포넌트에서는 이벤트 핸들러가 필요하지 않아 문제가 될 수 있습니다.
제안사항:
- Line 9:
handleKeywordChange함수의 인자 타입 선언이 정확하지 않습니다. 이 함수는 input 또는 select 요소에서 발생하는 이벤트와 관련하여 어떤 작업을 수행하는지 명확히 지정해주어야 합니다. - Line 13:
handleSortChange함수의 인자 타입 선언도 정확하지 않습니다. 해당 함수가 어떤 이벤트를 처리하는지 명시해야 합니다. - Line 37-44: 빈 카드를 생성하는 부분입니다. 왜 이러한 빈 카드가 필요한지에 대한 설명이 없습니다. 코드가 깔끔하게 유지되고 있는지 확인해야 합니다.
코드 개선 및 버그 수정을 위해 해당 사항들을 검토하시기 바랍니다.
| <LocationTab /> | ||
| <PersonnelTab | ||
| personnel={personnel} | ||
| handlePersonnelChange={handlePersonnelChange} | ||
| handlePersonnelChange={handleRadioChange} | ||
| /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
이 코드 패치는 ScheduleFilter 컴포넌트에 대한 수정 사항입니다. 몇 가지 버그 위험 및 개선 제안을 알려드리겠습니다:
-
handleStartDateChange와 handleEndDateChange 함수들은 삭제된 것으로 보입니다. 대신 handleDateChange 함수가 추가되었는데, 이 함수에서 시작일과 종료일 변경을 모두 처리할 수 있도록 구현되어야 합니다. 이를 확인하고 수정해야 합니다.
-
handleExpenseChange와 handlePersonnelChange 함수들은 handleRadioChange로 이름이 변경되었습니다. 이 컴포넌트의 라디오 버튼의 변경 이벤트를 처리하는 역할을 하는 것 같습니다. 함수 이름과 인터페이스를 일관되게 수정해야 합니다.
-
setTheme 함수가 ScheduleFilterProps 인터페이스에서 삭제되었습니다. theme 값을 변경하는 방법을 알 수 없으므로 이 부분을 다시 확인하고 수정해야 합니다.
-
div 요소의 클래스 속성에 "min-w-[280px]"와 "max-w-[280px]" 스타일 클래스가 추가되었습니다. 이로써 요소의 너비가 고정되어 최소 및 최대 너비를 유지합니다. 이 스타일이 필요한지 확인하고 필요에 따라 적절한 값으로 수정하세요.
-
마지막으로 LocationTab은 변경되지 않았으나, 해당 컴포넌트의 구현 및 사용 방법을 확인하여 필요에 맞게 사용해야 합니다.
이러한 사항들을 확인하고 수정하여 코드를 개선할 수 있습니다.
| ); | ||
| }; | ||
|
|
||
| export default SortFilter; |
There was a problem hiding this comment.
위의 코드 패치는 React 컴포넌트인 SortFilter를 정의하는 것으로 보입니다. 몇 가지 개선 제안과 버그 위험에 대해 설명하겠습니다:
-
sortFilterprop은 현재 선택된 정렬 기준을 나타내는 문자열이므로, 더 명확한 이름을 사용할 수 있습니다. 예를 들어selectedSortOption과 같은 이름을 고려해 볼 수 있습니다. -
<select>요소에서valueprop을 사용하여 선택된 옵션을 표시하고 있습니다. 그러나 초기 상태를 설정하는 구문이 없으므로 상위 컴포넌트에서sortFilterprop을 적절하게 초기화해야 합니다. 초기 상태를 설정하는 방법에 따라 필요한 로직을 추가하십시오. -
handleSortChange함수는onChange이벤트의 핸들러로 사용됩니다. 이 함수는e인자를 받으며, 해당 인자의 타입을ChangeEvent<HTMLSelectElement>로 정의하고 있습니다. 현재 코드에서는handleSortChange함수를 실제로 정의하지 않았기 때문에 해당 함수가 어떻게 동작해야 하는지 확인해야 합니다. 적절한 동작에 맞게 함수를 추가하거나 수정해야 할 수 있습니다. -
SVG 아이콘(사각형으로 보이는 화살표)을 표시하고 있으며,
pointer-events-none클래스가 포함된<div>요소를 사용하여 클릭 이벤트를 방지하려고 시도하고 있습니다. 그러나 해당<div>요소의 역할과 의도를 명확하게 설명하기 위해 주석을 추가하는 것이 좋을 수 있습니다. -
코드 자체는 큰 버그 위험이 보이지 않습니다. 그러나
SortFilter컴포넌트 자체에서 발생할 수 있는 버그 및 예외 상황을 고려하십시오. 예를 들어, 유효하지 않은sortFilterprop 값이 제공되는 경우에 대한 처리 방법을 확인하거나 필요한 유효성 검사를 추가할 수 있습니다.
| ); | ||
| }; | ||
|
|
||
| export default Tag; |
There was a problem hiding this comment.
해당 코드 패치는 JavaScript의 React 라이브러리를 사용합니다. Tag 컴포넌트는 속성(props)으로 tag라는 문자열을 받아와 해당 태그를 화면에 표시하는 역할을 수행합니다. 아래는 간단한 코드 리뷰입니다:
-
import 문: React 모듈을 임포트하고 있습니다. React를 사용하는 경우, 해당 모듈을 임포트해야 합니다.
-
interface 문: TagProps라는 인터페이스를 정의하고 있습니다. 이 인터페이스는 tag 속성(prop)이 필요함을 나타냅니다. 이는 해당 컴포넌트가 tag 속성을 받아야 한다는 것을 명시적으로 알려줍니다.
-
Tag 컴포넌트: 함수형 컴포넌트로 작성되어 있으며, tag 속성을 받아와 반환할 JSX 요소를 생성합니다.
-
JSX: div 요소를 반환하고, className 속성으로 여러 CSS 클래스를 지정합니다. h-[28px], p-1, text-sm, font-normal, rounded-md, bg-sky-100, text-stone-500 등의 클래스가 사용됩니다. 이 클래스들은 Tailwind CSS와 함께 사용되는 몇 가지 사전 정의된 스타일 클래스인 것으로 추측됩니다.
-
export 문: Tag 컴포넌트를 다른 파일에서 임포트할 수 있도록 내보내고 있습니다.
버그 리스크 및 개선 제안:
-
현재 코드에서는 특정한 버그 리스크가 보이지 않습니다. 그러나 Tailwind CSS를 사용하는 경우, 스타일 클래스의 목록을 확인하여 예상대로 동작하는지 확인해야 합니다.
-
추가 개선 사항은 컴포넌트 주석(documentation)입니다. 컴포넌트의 역할, 사용 방법, 기대되는 속성 등을 설명하는 주석을 추가하는 것이 좋습니다.
-
태그(Tag) 컴포넌트 자체의 세부적인 내용에 대해서 더 자세하게 파악할 수 있으면 문맥에 맞게 코드 리뷰를 진행할 수 있습니다.
더 자세한 코드 분석과 문제 해결을 위해서는 해당 코드를 호출하고 사용하는 곳의 전체적인 구조와 목적을 파악하는 것이 중요합니다.
| setTheme(value); | ||
| }; | ||
|
|
||
| const ThemeTab = ({ theme, handleTab }: ThemeTabProps) => { | ||
| return ( | ||
| <div className="p-5 border-t"> | ||
| <CategoryTitle title="테마" /> |
There was a problem hiding this comment.
아래는 코드 패치입니다. 버그의 위험과/또는 개선 제안에 대한 간단한 코드 리뷰를 도와드리겠습니다.
@@ -5,14 +5,10 @@ import CategoryTitle from "./CategoryTitle";
interface ThemeTabProps {
theme: string;
- setTheme: Dispatch<SetStateAction<string>>;
+ handleTab: (theme: string) => void;
}
-const ThemeTab = ({ theme, setTheme }: ThemeTabProps) => {
- const handleTab = (value: string) => {
- setTheme(value);
- };
-
+const ThemeTab = ({ theme, handleTab }: ThemeTabProps) => {
return (
<div className="p-5 border-t">
<CategoryTitle title="테마" />이 코드 패치에서 주요 변경 사항은 setTheme 매개변수를 handleTab 함수로 대체하는 것입니다. 이러한 변경으로 인해 setTheme 함수가 사용되지 않으므로 해당 함수를 제거했습니다. 코드 자체는 큰 문제가 없어 보입니다. 다만, 아래 몇 가지 개선 제안이 있습니다:
handleTab함수 내부에 있는 코드를 확인하고 변경사항에 맞추어 업데이트할 필요가 있습니다.CategoryTitle컴포넌트에 대한 추가 정보나 설명이 필요한 경우 해당 부분을 첨부하시면 더 나은 리뷰를 제공할 수 있습니다.- 변수 이름을 더 명확하게 지정하여 코드의 가독성을 개선할 수 있습니다.
- 이 코드 조각이 다른 부분과 함께 사용되는 방식이 불분명한 경우 해당 컨텍스트를 제공해주시면 더 나은 피드백을 드릴 수 있습니다.
런타임 문제가 없으므로 주요 관심사는 코드 변경 내용과 개선을 위한 적절한 컨텍스트입니다.
| { label: "작성자", value: "3" }, | ||
| { label: "내용", value: "4" }, | ||
| { label: "제목+내용", value: "5" }, | ||
| { label: "태그", value: "6" }, | ||
| ]; |
There was a problem hiding this comment.
이 코드 패치에는 몇 가지 버그 리스크와 개선 제안이 있습니다:
-
이미지 경로 변경:
themeList배열에서 "축제" 테마의 이미지 경로가 수정되었습니다. 수정 전과 후의 이미지 파일 경로를 다시 확인해야 합니다. -
가격 범위 값 변경:
priceRanges배열의 각 항목의value값이 변경되었습니다. 이전에는 구간을 나타내는 형태였으며, 새로운 값은 단순한 숫자로 변경되었습니다. 해당 변경으로 인해 이미 사용 중인 기능과 관련된 문제가 발생할 수 있으므로 사용하는 모든 부분을 확인해야 합니다. -
인원 수 범위 값 변경:
PersonnelRanges배열 이름이personnelRanges로 변경되었습니다. 또한, 각 항목의value값도 변경되어 구간 대신 단순한 숫자로 표현됩니다. 해당 변경으로 인해 이미 사용 중인 기능과 관련된 문제가 발생할 수 있으므로 사용하는 모든 부분을 확인해야 합니다. -
키워드 범위 추가:
keywordRanges배열이 추가되었습니다. 각 항목은 검색 옵션을 나타내는 값과 라벨로 구성되어 있습니다. 이 변경 사항은 해당 기능을 사용하는 부분을 업데이트해야 한다는 점에 유의하세요.
코드 리뷰를 수행할 때 이러한 사항들을 고려하여 각 변경 내용이 다른 코드 부분과의 호환성과 기능에 영향을 주는지 확인해야 합니다.
| }, | ||
| { | ||
| title: "일정 만들기", | ||
| path: "/create-schedule", | ||
| }, | ||
| { | ||
| title: "내 일정", | ||
| path: "/my-schedule", | ||
| path: "/schedule", | ||
| }, | ||
| ]; | ||
|
|
There was a problem hiding this comment.
위의 코드 패치를 간단히 검토하겠습니다.
- 120번 줄에서 타이틀 "일정 찾기"에 대한 path가 "/schedule"로 되어 있습니다. 이 부분을 "/find-schedule"로 수정하였습니다. 이 변경 사항이 잘못된 API 호출 또는 경로 일치 문제를 피하기 위한 것인지 확인이 필요합니다.
- 125번 줄에서 타이틀 "내 일정"에 대한 path가 "/my-schedule"로 되어 있습니다. 이 부분을 "/schedule"로 수정하였습니다. 마찬가지로, 이러한 변경 사항이 원하는 동작과 일치하는지 확인해야 합니다.
개선 제안:
- 탭 항목에 대한 설명 주석 추가: 탭 항목들의 역할과 기능에 대한 설명을 주석으로 추가하여 코드의 가독성을 향상시킬 수 있습니다.
- 상수 사용: 경로의 문자열 값(예: "/find-schedule")을 하드코딩하지 않고 상수로 정의하여 사용하면 오탈자와 문자열 중복을 방지할 수 있으며, 유지 및 관리가 용이해집니다.
주의 사항:
- 주어진 코드 패치는 어떤 컨텍스트나 다른 코드와의 상호작용에 대한 정보가 없으므로, 전체 애플리케이션 및 구현 상황을 고려하여 추가적인 검토를 수행해야 합니다.
| @@ -34,7 +34,7 @@ const Header = () => { | |||
|
|
|||
| return ( | |||
| <header | |||
| className={`fixed top-0 left-0 right-0 flex items-center justify-between px-20 py-2 bg-${headerColor} transition-background duration-500 ease-in-out`} | |||
| className={`fixed top-0 left-0 right-0 flex items-center justify-between px-20 py-2 bg-${headerColor} transition-background duration-500 ease-in-out z-50`} | |||
| > | |||
| <div className="flex items-center cursor-pointer"> | |||
| <Link href="/"> | |||
There was a problem hiding this comment.
@@ -34,7 +34,7 @@ const Header = () => {
return (
<header
-
className={`fixed top-0 left-0 right-0 flex items-center justify-between px-20 py-2 bg-${headerColor} transition-background duration-500 ease-in-out`}
-
className={`fixed top-0 left-0 right-0 flex items-center justify-between px-20 py-2 bg-${headerColor} transition-background duration-500 ease-in-out z-50`}<div className="flex items-center cursor-pointer"> <Link href="/">
위의 코드 패치에서는 헤더 요소에 "z-50" 클래스를 추가했습니다. 이 클래스는 헤더를 다른 요소들 위에 고정하기 위해 z-index 값 50을 설정합니다.
이 코드 패치는 개발자가 목적한 대로 작동할 수 있습니다. 하지만 몇 가지 개선 사항이 있을 수 있습니다:
- style 속성 대신 className 속성을 사용하여 CSS 스타일을 적용하는 것이 좋습니다. 예를 들어, 헤더 배경색을 클래스로 정의하고 해당 클래스를 className 속성에 추가할 수 있습니다. 이렇게 하면 컴포넌트의 가독성과 유지 보수성이 향상될 수 있습니다.
- "px-20"와 "py-2" 값을 변수로 추출하여 의미 있는 이름으로 지정하는 것이 좋습니다. 이렇게 하면 필요한 경우에 쉽게 수정하거나 다른 값을 사용할 수 있습니다.
- transition-duration 값인 "duration-500"을 변수화하는 것이 좋습니다. 코드에 사용된 하드 코딩된 값을 변수로 추출하면 유지 보수가 더 용이해집니다.
일반적으로 해당 코드 패치에는 큰 버그 위험이나 개선이 필요한 부분은 없어 보입니다. 단, 각 프로젝트의 컨벤션과 요구 사항에 따라 추가적인 개선이 필요할 수 있습니다.
| ...handlers, | ||
| ...myScheduleHandlers, | ||
| ...scheduleHandlers, | ||
| ); |
There was a problem hiding this comment.
이 코드 패치의 간단한 코드 리뷰를 도와드리겠습니다.
- 추가된 import 문은 다음과 같습니다:
import { scheduleHandlers } from "./handlers/findSchedule";./handlers/findSchedule에서 scheduleHandlers를 가져오는 것으로 보입니다. 이 부분에 대한 상세 내용을 알 수 없어서, 해당 파일이나 findSchedule 모듈 내용을 확인해야 합니다.
-
worker 설정 부분에서
setupWorker()함수에 전달되는 핸들러 배열에 새로 추가된...scheduleHandlers가 있습니다. -
개선 제안:
- 코드 리뷰를 위해서는
handlers.js와mySchedule.js내용을 알아야합니다. ...scheduleHandlers가 정확히 무엇을 하는지, 추가된 핸들러들이 어떤 동작을 수행하는지에 대한 설명이 필요합니다.- 코드 리뷰에서 가장 중요한 부분은 사전에 구현된 기능과 일관성을 유지하고 에러를 방지하기 위한 검사를 하는 것입니다.
추가적인 정보가 없어서 더 자세한 리뷰를 진행하기 어렵지만, 전반적으로 코드는 합리적으로 보입니다.
| }, | ||
| ], | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
위의 코드 조각은 cardDetail이라는 상수 배열을 내보내는 export문과 함께 객체를 포함하고 있습니다. 각 객체는 다양한 속성을 가지고 있으며, 이들은 특정 등산 일정에 대한 정보를 제공합니다.
가능한 버그 리스크:
- participateInfos 배열에서 id 3과 4가 중복되어 나타나고 있습니다. 이는 잘못된 데이터로 보입니다.
개선 제안:
- 프로필 이미지와 날짜를 나타내는 문자열("2023/12/10", "2023/12/13" 등)을 Date 객체로 변경하는 것이 좋습니다. 이렇게 하면 더욱 쉽게 날짜와 관련된 작업을 처리할 수 있습니다.
- 프로필 이미지 경로들을 모두 상수 변수로 추출하여 사용하는 것이 관리에 용이합니다.
- 단순히 문자열로 포맷된 긴 내용을 텍스트 멀티라인으로 작성하면 가독성이 높아집니다(예:
const content = \철쭉제로...`;`).
전반적으로 코드는 유효해보이며 크리티컬한 버그는 없어보입니다. 개선 사항은 주로 가독성 및 유지 보수 측면에서 제시된 것이므로 선택사항입니다.
| participateNum: 7, | ||
| participateCapacity: 11, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
이 코드 패치는 "card"라는 변수에 객체 배열을 할당하는 것으로 보입니다. 각 객체는 여러 속성을 가지고 있으며, 각각의 카드를 나타냅니다.
이 코드는 일부 개선할 수 있는 부분이 있습니다:
-
이미지 경로를 상대 경로 대신 절대 경로로 사용하십시오. 이렇게 하면 다른 환경에서 애플리케이션을 실행할 때 이미지를 찾을 수 있습니다.
-
날짜 포맷에 대해 ISO 형식 (YYYY-MM-DD)를 사용하는 것을 고려하십시오. 이 형식은 날짜 시스템 간 호환성이 높아 더 좋습니다.
-
태그에 대해 중복된 값이 있는 듯한데, 필요한 경우 중복을 제거하거나 이중 선택을 지원하도록 수정하십시오.
-
참여 인원 수와 용량 사이에 오류가 있어 보입니다. 이 부분을 확인하고 올바르게 처리하십시오.
-
주석이나 문서화 부분을 추가하여 코드를 이해하기 쉽도록 만드십시오.
앞으로 구현에 따른 버그 위험도 등을 고려하면, 위의 개선점 외에는 보이지 않습니다.
|
|
||
| return new Response(JSON.stringify(matchedItem)); | ||
| }), | ||
| ]; |
There was a problem hiding this comment.
위의 코드 패치를 간단히 검토해드리겠습니다. 다음은 개선점과 버그 위험에 대한 제안입니다:
import문에서"fs"를 가져오고 있지만, 코드에서 사용되지 않으므로 제거할 수 있습니다.baseUrl값을 하드코딩하고 있는데, 환경 변수 또는 설정 파일을 사용하여 구성 가능하게 수정하는 것이 좋습니다.- 필터링 로직이 길고 중복되는 부분이 있습니다. 이를 함수로 분리하여 재사용성을 높이는 것이 좋습니다.
card.filter및cardDetail.find를 사용하여 데이터를 필터링하고 있습니다. 이 데이터가 클 경우 성능 저하가 발생할 수 있으므로 적절한 인덱싱과 캐싱 기술을 고려해야 합니다.- 요청 URL 파라미터 값들을 가져올 때
null또는undefined일 경우를 처리하지 않고 있습니다. 이에 대한 유효성 검사와 오류 처리를 추가하는 것이 좋습니다. - 응답으로 반환되는 JSON.stringify 메서드로 직렬화된 문자열이 있습니다. 이는 HTTP 응답 객체가 자동으로 JSON으로 직렬화되도록 변경하는 것이 좋습니다.
- 에러 처리를 추가하면 좋을 것 같습니다. 예외적인 상황이 발생한 경우 적절한 HTTP 상태 코드와 함께 오류 응답을 반환하는 것이 좋습니다.
위의 제안 사항을 참고하여 코드를 개선하시면 됩니다.
|
|
||
| export const server = setupServer(...handlers); | ||
| export const server = setupServer(...handlers, ...scheduleHandlers); |
There was a problem hiding this comment.
이 코드 패치는 msw/node에서 setupServer를 가져와서 사용하는 것을 보여줍니다. 또한 ./handlers로부터 핸들러를 가져와서 사용하고 있습니다. 작성된 패치에는 주요한 버그 위험 요소가 보이지 않습니다.
개선 제안:
- 임포트 구문은 파일 맨 위에 배치하는 것이 일반적입니다. 따라서
import구문들을 패치의 시작 부분으로 이동시켜주세요. scheduleHandlers모듈을 가져온 후 어떻게 사용되는지에 대한 추가적인 정보가 없으므로, 해당 부분을 검토할 수는 없습니다. 필요하다면 그 내용을 확인하여 개선 가능한 부분이 있는지 파악해야합니다.
| @@ -169,5 +187,4 @@ a { | |||
| font-style: normal; | |||
| font-weight: 700; | |||
| src: url('/fonts/NotoSansKR-Bold.ttf'); | |||
| } | |||
|
|
|||
| } No newline at end of file | |||
There was a problem hiding this comment.
코드 리뷰:
-
check클래스에 대한 스타일이 추가됩니다. 이 스타일은 체크박스와 관련하여 다음과 같은 작업을 수행합니다:- 웹킷과 모질라 기반 브라우저에서 기본 외관을 숨깁니다.
- 너비와 높이를 18px로 설정합니다.
- 테두리를 1.5px의 실선으로 지정합니다.
- 선택 상태일 때 배경 이미지를 "/assets//detail/checked.svg"로 설정합니다.
- 체크박스가 선택되었을 때 테두리를 #f472b6 색상의 1.5px 실선으로 변경합니다.
-
.truncate-text클래스에 @apply 규칙이 추가됩니다. 이 규칙은 다음 클래스들을 포함시킵니다: whitespace-nowrap, overflow-hidden, overflow-ellipsis. -
마지막 행은 파일 종료 시에 개행이 없음을 의미합니다.
문제 및 개선 제안:
- 첫 번째 코드 블록에서 "a" 선택자에 중괄호(
{})가 누락되었습니다. 따라서 정확히 어떤 스타일이a에 적용되는지 확인해야 합니다. background-size속성 값을 "100%"로 설정하는 것보다는 "cover" 또는 "contain"을 사용하는 것이 더 적합할 수 있습니다. 그렇게 하면 배경 이미지가 가득 차거나 전체 영역에 맞춰 표시됩니다.- 폰트 파일 경로 및 이름은 정확한지 확인해야 합니다.
그 외에는 주어진 코드 패치에 문제가 없어 보입니다.
| const diff = futureDate.getTime() - currentDate.getTime(); | ||
|
|
||
| return Math.ceil(diff / (1000 * 60 * 60 * 24)); | ||
| }; |
There was a problem hiding this comment.
이 코드 패치를 간단히 코드 리뷰하겠습니다:
-
calculateNightsAndDays함수:- 입력으로 받은 날짜를 기준으로 숙박 일수와 박수를 계산하는 함수입니다.
- 날짜가 올바르게 파싱될 것이라는 가정 하에 문제 없어 보입니다.
-
countCommentsAndReComments함수:- 댓글과 대댓글의 총 개수를 계산하는 함수입니다.
comments배열에서 각 댓글의 대댓글 수를 합산합니다.- 주어진 코드에서는 존재 여부 확인을 위해 optional chaining (
?.)을 사용하였지만, 배열의 길이가 애초에 정해져 있으므로 일반적인 접근도 가능할 것으로 보입니다. - 코드는 유효한 것으로 보여지며, 추가 개선 사항은 없어 보입니다.
-
daysUntil함수:- 주어진 날짜까지 남은 날짜 수를 반환하는 함수입니다.
- 입력으로 받은 날짜와 현재 날짜를 비교하여 남은 일수(소수점 이하 반올림)를 계산합니다.
- 주어진 코드는 정상적으로 작동할 것으로 보입니다.
이 코드 패치에서 중대한 버그 리스크나 개선사항은 보이지 않으며, 주요 기능은 적절하게 동작하는 것으로 보입니다.
| @@ -2,4 +2,5 @@ export * from "./tag"; | |||
| export * from "./regex"; | |||
| export * from "./format"; | |||
| export * from "./logout"; | |||
| export * from "./calculate"; | |||
| export * from "./convert"; | |||
There was a problem hiding this comment.
주어진 코드 변경 내용을 간단히 검토해 드리겠습니다. import/export 문을 추가한 부분에 대한 문제가 없어 보입니다. 그러나 나머지 파일들의 구현이 있는지는 알 수 없기 때문에 자세한 리뷰는 어렵습니다. 코드의 다른 부분과 상호작용하는 방식, 의존성 및 함수 호출 등을 확인해야 할 것입니다. 현재 설명으로는 코드에서의 버그 가능성이나 개선 사항을 파악하기 어렵습니다. 더 자세한 정보가 있다면 알려주시면 도움 드릴 수 있습니다.
| @@ -25,6 +25,7 @@ | |||
| "@faq/*": ["faq/*"], | |||
| "@mypage/*": ["mypage/*"], | |||
| "@findSchedule/*": ["findSchedule/*"], | |||
| "@detail/*": ["detail/*"], | |||
| "@create-schedule/*": ["create-schedule/*"] | |||
| } | |||
| }, | |||
There was a problem hiding this comment.
주어진 코드 패치를 간단하게 검토해 드리겠습니다. 추가된 부분에 대한 버그 위험이나 개선 제안을 해보도록 하겠습니다.
- 이 코드 패치는 모듈 경로(alias)와 실제 파일 경로를 매핑하는 webpack의 별칭(alias) 설정 부분입니다.
- "@detail/"를 추가하여 "detail/"와 매핑되도록 했습니다.
- 주의할 점은, 실제 프로젝트 구조가 올바르게 매핑되도록 경로를 확인해야 합니다.
- 변경 내용에서 오류를 발견할 수 없지만, 세세한 내용을 파악하려면 전체 코드 및 프로젝트 구조를 살펴봐야 할 것입니다.
| {!onCommentToggle ? ( | ||
| <button | ||
| className="block py-1 pl-4 pr-3 bg-white border border-zinc-500" | ||
| onClick={() => setOnCommentToggle(true)} | ||
| > | ||
| <div className="flex items-center gap-2 text-base font-semibold text-neutral-400"> | ||
| <Image | ||
| src="/assets/detail/comment.svg" | ||
| alt="comment" | ||
| width={18} | ||
| height={18} | ||
| /> | ||
|
|
||
| <span className="pr-3 border-r-[1.5px]"> | ||
| 댓글 {countCommentsAndReComments(comments)} | ||
| </span> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width="14" | ||
| height="8" | ||
| viewBox="0 0 14 8" | ||
| fill="none" | ||
| > | ||
| <path d="M13 1L7 7L1 1" stroke="#888888" /> | ||
| </svg> | ||
| </div> | ||
| </button> | ||
| ) : ( | ||
| <button | ||
| className="block py-1 pl-4 pr-3 bg-white border border-emerald-500" | ||
| onClick={() => setOnCommentToggle(false)} | ||
| > | ||
| <div className="flex items-center gap-2 text-base font-semibold text-emerald-500"> | ||
| <Image | ||
| src="/assets/detail/comment_green.svg" | ||
| alt="comment" | ||
| width={18} | ||
| height={18} | ||
| /> | ||
| <span className="pr-3 border-r-[1.5px]"> | ||
| 댓글 {countCommentsAndReComments(comments)} | ||
| </span> | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width="14" | ||
| height="8" | ||
| viewBox="0 0 14 8" | ||
| fill="none" | ||
| > | ||
| <path d="M1 7L7 1L13 7" stroke="#00D179" /> | ||
| </svg> | ||
| </div> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
[Lv2] 이 부분은 하나로 합칠 수 있을 것 같습니다!
| comments: CommentWithReComments[], | ||
| ): number => { | ||
| // 댓글의 개수 | ||
| const totalComments = comments?.length; |
There was a problem hiding this comment.
[Lv1] comments가 무조건 들어와야 하는 상황이라면
| const totalComments = comments?.length; | |
| const totalComments = comments.length; |
optional 연산자가 없어도 될것같습니다!
만약 빈 배열이 아닌 다른 객체가 들어올 확률이 있다면
| const totalComments = comments?.length; | |
| const totalComments = comments?.length ?? 0; |
위와같이 예외상황에서의 기본 값을 같이 넣어주는게 좋을것같습니다!
| detailData: DetailCard; | ||
| } | ||
|
|
||
| const detail = ({ detailData }: DetailProps) => { |
There was a problem hiding this comment.
[Lv2] 가급적이면 컴포넌트명은 대문자로 시작하는게 좋습니다!
| } catch (error) { | ||
| console.error("Error fetching data", error); | ||
| return { | ||
| props: { data: "" }, |
There was a problem hiding this comment.
| props: { data: "" }, | |
| notFound: true |
[Lv3] api 통신중 에러가 발생했다면 (데이터가 없거나) 여기서는 notFound 페이지를 띄워주는게 맞습니다!
404, 500 페이지는 따로 만들수있기때문에 우선 이렇게 해두는게 나을것같아요~
| </span> | ||
| </div> | ||
|
|
||
| {updateComment ? ( |
There was a problem hiding this comment.
[Lv2] 삼항연산자의 연속적인 사용은 가독성을 떨어뜨리기 쉽습니다..! 이부분은 if구문을 사용하는 함수로 분리하는게 나아보입니다!
There was a problem hiding this comment.
그리고 중간에 리턴값이 같은 조건도 있는데 이부분도 합칠수있을것같습니다!
const GetByCommentType = () => {
if (updateComment) {
return (
<div className="flex mt-2">
<CommentInput
type="update"
comment={comment}
onChangeComment={onChangeComment}
onChangeHidden={onChangeHidden}
onClickSubmit={onClickSubmit}
/>
</div>
);
}
else if (
!isHidden ||
logginedInfo.userId === userId ||
logginedInfo.userId === hostId
) {
return <div className="mt-1">{content}</div>;
} else {
return <div className="mt-1">비밀 댓글입니다.</div>;
}
};| import CommentInput from "./CommentInput"; | ||
|
|
||
| interface CommentInputBoxProps { | ||
| type: "re" | "com"; |
There was a problem hiding this comment.
[Lv2] 댓글 타입은 따로 타입으로 분리해도 좋을것같습니다! 추가로, 타입이 각각 어떤것을 의미하는지 주석으로 추가해주시는것도 좋을것같아요
| if (type === "com") { | ||
| // 댓글 | ||
| try { | ||
| const data = { postId, ...comment }; |
There was a problem hiding this comment.
이부분만 빼고 위아래 코드가 다 동일한데 조건문을 조금 수정하면 하나로 사용할 수 있을것같습니다!
| const data = { postId, ...comment }; | |
| const data = { | |
| postId, | |
| ...comment, | |
| ...(type === "com" ? { commentId } : {}), | |
| }; |
| onChange={handleSortChange} | ||
| > | ||
| <option value="최신순">최신순</option> | ||
| <option value="경비 적은 순">경비 적은 순</option> |
There was a problem hiding this comment.
[Lv2] value는 api에서 요구하는 값을 넣어두는것이 좋습니다! 한글로 적어두면 이걸 사용하는곳에서 변환하는 함수를 따로 만들어야하는 불편함이 있을거에요. 지금 당장은 필요한 값이 안정해져있을수있으니 TODO로 작성만 해두시는것도 좋을것같습니다!
| value={range.value} | ||
| checked={expense === range.value} | ||
| checked={Number(expense) === range.value} |
There was a problem hiding this comment.
[Lv2] ExpenseTabProps 를 보면 expense 의 타입이 number || undefind라서 Number()로 감싸지않아도 될것같습니다!
| import React from "react"; | ||
|
|
||
| const InfoTop = () => { | ||
| // TODO: 모집 상태 정보 어떻게 받아올지 고민 |
There was a problem hiding this comment.
Detail 컴포넌트로 내려받는 props에 모집 상태 데이터가 없나요??
| <div className="flex gap-2 mt-4"> | ||
| <button | ||
| className={`w-[60px] h-[50px] flex flex-col items-center px-4 py-2 border rounded ${ | ||
| likeStatus |
There was a problem hiding this comment.
[Lv2] 연속된 삼항연산자의 사용은 가독성이 떨어지고 복잡하게 보일 수 있습니다. 삼항연산자의 중첩을 하나 풀어서 변수로 빼는것이 나아보여요
const getLikeButtonStyle = () => {
if (likeStatus) {
return likeHovered ? "border-pink-400 bg-white" : "border-pink-400 bg-red-100 bg-opacity-50";
} else {
return likeHovered ? "border-pink-400" : "border-gray-300 bg-white";
}
}
구현 사항
테스트
TODO
추가사항