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주차 과제 #7

Open
wants to merge 49 commits into
base: minji2219
Choose a base branch
from

Conversation

minji2219
Copy link

안녕하세요! 멘토님 2주차 과제 step1 완료 후 1차 PR입니다.

STEP1 질문

폴더 구조에 대한 질문입니다. main페이지에서만 사용하는 component같은 경우에는 page/main/해당컴포넌트 의 구조로 하는 것과 components/main/해당컴포넌트 이런식으로 components 하위 폴더에 위치 시켜야하는지 궁금합니다.

minji2219 added 30 commits July 2, 2024 00:53
router 컴포넌트 생성
메인, 로그인, 내계정, 테마 페이지 생성 후 router로 각 path별 페이지 생성 및 연결
Header, Footer를 모든 페이지에서 볼 수 있게 Layout으로 생성
Outlet 이용
기념일 마다의 선물 카테고리를 뜻하는 annual category 컴포넌트 구현
선물 랭킹 중 인물이 카테고리인 컴포넌트 생성
선물랭킹 중 상황을 선택하는 카테고리 컴포넌트 생성
AnnualCateogry 컴포넌트 명을 Theme로 파일명 및 컴포넌트 명 변경
*22개의 더미 데이터 생성 후 사용
*실시간 급상승 선물랭킹 항목 보여주는 컴포넌트
theme페이지 구현 중 header max-width로 인한 깨짐 발생으로 width수정
Link태그로 감싸 pointer css불필요해짐
minji2219 and others added 15 commits July 4, 2024 15:01
Theme 컴포넌트
Ranking컴포넌트
Situation category 컴포넌트
유지보수 등을 위해 경로 상수로 만들어 사용
username과 setusername을 value로 하는 contextAPI
userName sessionStorage에 저장
contextAPI로 userName 공유
로그인 후 전 페이지로 이동
로그인 되어 있을 경우 "내 계정"버튼
로그인 안되어 있을 경우 "로그인"버튼
로그인 되지 않았을 때 my-account로 접근 시 login페이지로 이동
sessionStorage값 clear
context값 ''로 초기화
로그인 된 상태일 때는 내계정 클릭시 my-account로 이동
로그인 안된 상태일 때는 로그인 클릭시 login로 이동
authToken의 값이 존재한다면 authToken값으로 초기화
2주차 과제 step2구현
@minji2219 minji2219 changed the title 전남대 FE_서민지 - 2주차 과제(step1) 전남대 FE_서민지 - 2주차 과제(step2) Jul 4, 2024
<Link to={ROUTE_PATHS.ROOT}>선물하기</Link>
</Logo>
<Link to={userName === auth ? '/my-account' : '/login'}>
{userName === auth ? '내 계정' : '로그인'}
Copy link

Choose a reason for hiding this comment

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

userName 과 sessionStorage 의 authToken 을 비교해서 로그인 상태를 판단하는 이유는 뭘까요 ?

Copy link
Author

Choose a reason for hiding this comment

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

꼼꼼한 리뷰 감사합니다!
userInfo context를 처음 만들 때 defaultUserInfo의 userName을 '' 이런식으로 지정했다가
authToken값이 존재할 경우 해당 값으로 초기화 하는 것으로 코드를 바꿨습니다.
처음처럼 코드를 작성했을 때 저 비교가 필요하다고 판단했는데 한 값만 넣어도 될거 같습니다!

혹시 저 상황에서는 auth값이 존재할 경우만 확인하면 될까요?

{userName === auth ? (
<Route path={ROUTE_PATHS.MYPAGE} element={<MyAccount />} />
) : (
<Route path={ROUTE_PATHS.MYPAGE} element={<Login />} />
Copy link

Choose a reason for hiding this comment

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

로그인되지 않은 상태라면 path 자체도 Login page 에 경로여야 할 거 같습니다.

import { Button } from '@/components/common/Button';
import { breakpoints } from '@/styles/variants';

export const Badge = () => {
Copy link

Choose a reason for hiding this comment

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

Badge 라는 네이밍이 해당 컴포넌트에 적합하지 않은것 같은 생각이 드네요. 더 나은 네이밍이 있을까요 ?


export const Badge = () => {
return (
<Wrapper>
Copy link

Choose a reason for hiding this comment

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

const [category, setCategory] = useState<string>('all');
const handleCategory = (e: React.ChangeEvent<HTMLInputElement>) => {
setCategory(e.target.value);
console.log(category);
Copy link

Choose a reason for hiding this comment

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

Suggested change
console.log(category);

const { setUserName } = useContext(UserInfo);

const hanldeClick = () => {
sessionStorage.setItem('authToken', name);
Copy link

Choose a reason for hiding this comment

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

보통 authToken 이라고 하면 login api 호출하여 취득한 accessToken 을 말합니다.
유저의 이름을 sessionStorage 에 저장하여 authToken 처럼 사용하는건 지양해야 합니다.

const hanldeClick = () => {
sessionStorage.setItem('authToken', name);
setUserName(name);
navigate(-1);
Copy link

Choose a reason for hiding this comment

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

로그인 이후에는 로그인 된 상태로 홈으로 이동해야 합니다. navigation 뒤로가기를 하면 브라우저에서 앞으로 가기 했을 때 다시 로그인 페이지로 이동할거에요.

width: 88px;
`;
const Container = styled.div`
// display: flex;
Copy link

Choose a reason for hiding this comment

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

Suggested change
// display: flex;

`;
const Container = styled.div`
// display: flex;
// flex-direction: column;
Copy link

Choose a reason for hiding this comment

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

Suggested change
// flex-direction: column;

const Container = styled.div`
// display: flex;
// flex-direction: column;
// gap: 40px;
Copy link

Choose a reason for hiding this comment

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

Suggested change
// gap: 40px;

@tlsehdfl
Copy link

tlsehdfl commented Jul 5, 2024

폴더 구조에 대한 질문입니다. main페이지에서만 사용하는 component같은 경우에는 page/main/해당컴포넌트 의 구조로 하는 것과 components/main/해당컴포넌트 이런식으로 components 하위 폴더에 위치 시켜야하는지 궁금합니다.

선택하기에 따라 다릅니다 ! 실무 입장에서는 보통 어플리케이션이 확장함을 염두하여 feature based structure 를 지향합니다.
말씀해주신 선택지의 전자에 해당합니다.

ref. https://maestros.io/structure-by-type-vs-feature

@minji2219 minji2219 changed the title 전남대 FE_서민지 - 2주차 과제(step2) 전남대 FE_서민지 - 2주차 과제 Jul 5, 2024
minji2219 added 2 commits July 8, 2024 16:07
*불필요한 context값 sessionStorage값 비교 제거
*불분명한 컴포넌트 네이밍 수정(Badge->SubMent)
*Wrapper, Container 구분
*불필요한 console.log()제거
*navigate 수정
*cateogry type string대신 해당 type 지정
*불필요한 console.log() 제거
*pw -> password로 변경
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