Skip to content

[박준환] sprint8 #279

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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>>([]);

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

Successfully merging this pull request may close these issues.

2 participants