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

전남대_FE_이은진 2주차 과제 step1 #21

Open
wants to merge 11 commits into
base: eunjin210
Choose a base branch
from

Conversation

eunjin210
Copy link
Collaborator

안녕하세요 멘토님 !
2주차 과제 step1까지 제출입니다. 고쳐야 할 사항이나 불필요한 부분이 있다면 말씀 부탁드립니다.

@eunjin210 eunjin210 changed the title 전남대_FE_이은진 2주차 과제 전남대_FE_이은진 2주차 과제 step1 Jul 5, 2024
import Header from '@/Layout/Header';

interface LayoutProps {
isLoggedIn: boolean;
Copy link

Choose a reason for hiding this comment

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

Layout 에서 props 로 전달받는 것 보다는 사용처에서 context 에 접근하여 로그인 상태를 가져오게끔 할 수 있을거 같습니다.

ref. https://react.dev/learn/passing-data-deeply-with-context

import PersonButton from '@/components/common/HomeComponents/Ranking/PersonButton';
import RankTypeButton from '@/components/common/HomeComponents/Ranking/RankButton';

const initialFilterOption = {
Copy link

Choose a reason for hiding this comment

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

targetTyperankType 이 별개의 타입 같은데 하나의 state 로 관리하는 이유가 있을까요 ?

분리하고 타입 지정하는게 더 나을거 같아요.

// e.g.
const [targetType, setTargetType] = useState<TargetType>('ALL');
const [rankType, setRankType] = useState<RankType>('MANY_WISH');


export const ItemList = () =>{

const [All, setAll] = useState(false);
Copy link

Choose a reason for hiding this comment

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

Suggested change
const [All, setAll] = useState(false);
const [all, setAll] = useState(false);

@tlsehdfl
Copy link

tlsehdfl commented Jul 5, 2024

lint 설정 제대로 되어 있는지 전반적으로 확인 부탁드려요 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants