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

[박준환] sprint8 #279

Open
wants to merge 1 commit into
base: React-박준환
Choose a base branch
from

Conversation

park521
Copy link
Collaborator

@park521 park521 commented Jan 5, 2025

요구사항

기본

  • [x]
  • []
  • []

심화

  • [x]
  • []

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@park521 park521 requested a review from devToram January 5, 2025 14:38
@park521 park521 self-assigned this Jan 5, 2025
@park521 park521 added the 순한맛🐑 마음이 많이 여립니다.. label Jan 5, 2025
import "./Header.css";

// react-router-dom의 NavLink를 이용하면 활성화된 네비게이션 항목을 하이라이트해줄 수 있어요!
function getLinkStyle({ isActive }: { isActive: boolean }) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
function getLinkStyle({ isActive }: { isActive: boolean }) {
function getLinkStyle(isActive: boolean) {

으로 간결하게 쓸 수 있을 거 같아요!

setIsDropdownVisible(false);
}}
>
최신순
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입스크립트로 바꾸시는 김에 <최신순>, <인기순> 도 타입화해보시면 좋을 거 같아요!

Copy link
Collaborator

@devToram devToram left a comment

Choose a reason for hiding this comment

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

아직 확장자만 ts 이고, 실제 타입스크립트를 적용한 부분이 많지 않아서 간략하게 리뷰했습니다~

Comment on lines +31 to +34
onClick={() => {
onSortSelection("favorite");
setIsDropdownVisible(false);
}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

렌더부에서 넣어줄 값이 없는 경우에는 해당 값을 따로 함수로 빼서 넣어주시는 걸 추천드려요!

const handleItemClick = () => {
     onSortSelection("favorite");
     setIsDropdownVisible(false);
}
Suggested change
onClick={() => {
onSortSelection("favorite");
setIsDropdownVisible(false);
}}
onClick={handleItemClick}

Comment on lines +31 to +33
size,
fillColor,
outlineColor,
Copy link
Collaborator

Choose a reason for hiding this comment

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

요런 친구들도 props 인 경우는 타입 써주시는 게 좋습니다!

function AddItemPage() {
const [name, setName] = useState("");
const [description, setDescription] = useState("");
const [price, setPrice] = useState("");
Copy link
Collaborator

Choose a reason for hiding this comment

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

price 의 경우 숫자라고 생각이 드는데 초기값이 "" 라서 타입병기해주시면 좋을 거 같아요!

Suggested change
const [price, setPrice] = useState("");
const [price, setPrice] = useState<string>("");

const [name, setName] = useState("");
const [description, setDescription] = useState("");
const [price, setPrice] = useState("");
const [tags, setTags] = useState([]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

배열의 경우에도 안에 어떤 값이 올 지 적어주심 좋아요!

Suggested change
const [tags, setTags] = useState([]);
const [tags, setTags] = useState<Array<string>>([]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
순한맛🐑 마음이 많이 여립니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants