[A팀] 웹 프론트엔드 파트 코드리뷰용 PR #10
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
asset들을 public 폴더에 넣어 같은 서버에서 내려줄 수 있을 것 같은데 이렇게 하신 이유가 궁금해요
src/components/button/ChatBot.tsx
Outdated
| const FloatingChatbotButton: React.FC = () => { | ||
| const router = useRouter(); | ||
|
|
||
| const handleClick = () => { | ||
| router.push("/Chatbot/Chatbot"); | ||
| }; | ||
|
|
||
| return ( | ||
| <button | ||
| onClick={handleClick} | ||
| className="absolute bottom-4 right-4 flex items-center justify-center transition duration-300 ease-in-out" | ||
| > | ||
| <Image src={chatbotIcon} alt="히트존 챗봇" width={48} height={48} /> | ||
| </button> | ||
| ); | ||
| }; |
src/components/button/ChatBot.tsx
Outdated
| import Image from "next/image"; | ||
| import chatbotIcon from "../../assets/svg/chatbot_button.svg"; | ||
|
|
||
| const FloatingChatbotButton: React.FC = () => { |
src/components/layout/NavBar.tsx
Outdated
| ]; | ||
|
|
||
| return ( | ||
| <div className="fixed bottom-0 w-full max-w-[500px] bg-grayscale-0 border-t border-grayscale-10 flex justify-around p-2"> |
There was a problem hiding this comment.
nav와 같은 시멘틱 태그를 이용할 수 있지 않을까요?
이용한다면 어떤 이점이 있을까요?
src/pages/Culture/Culture.tsx
Outdated
There was a problem hiding this comment.
일반적으로 url에서는 대소문자 구분이 없긴 합니다. (구분되는 환경도 있지만요)
각 페이지는 소문자로 시작하면 어떨까요?
| interface DropdownProps { | ||
| options: string[]; | ||
| selectedOption: string; | ||
| onSelect: (option: string) => void; | ||
| } | ||
|
|
||
| export default function Dropdown({ options, selectedOption, onSelect }: DropdownProps) { |
There was a problem hiding this comment.
서면 리뷰로도 남긴 내용인데
Next.js config의 pageExtension 설정을 안하고 이렇게 두면
/Main/components/Dropdown도 페이지로 잡히게 돼요
src/pages/_app.tsx
Outdated
| // 현재 URL 경로 가져오기 | ||
| const [currentPath, setCurrentPath] = useState<string>(''); | ||
|
|
||
| useEffect(() => { | ||
| // 클라이언트 측에서 경로 설정 | ||
| setCurrentPath(window.location.pathname); | ||
| }, []); | ||
|
|
||
|
|
||
| // 현재 경로에 따라 컴포넌트 선택 | ||
| const renderComponent = () => { | ||
| switch (currentPath) { | ||
| case '/login': | ||
| return <Onboarding />; | ||
| case '/signup': | ||
| return <SignupPage />; | ||
| case '/': | ||
| return <Main />; | ||
| case '/recommend/question': | ||
| return <Question />; | ||
| case '/guide': | ||
| return <Guide />; | ||
| case '/culture': | ||
| return <Culture />; | ||
| case '/mypage': | ||
| return <MyPage />; | ||
| } | ||
| }; |
There was a problem hiding this comment.
이렇게 클라이언트 사이드에 라우팅을 전부 위임하게 되면 Next.js를 사용하는 이유가 없어지는 것 아닌가요?
이렇게 하신 이유는 소문자를 대응하기 위함인가요?
There was a problem hiding this comment.
꼼꼼하게 봐주셔서 감사합니다!
레포지토리를 체계젹으로 폴더화하여 정리하려다보니 url과 맞추기가 힘들었던 것 같습니다ㅠ
예를 들어, 메인홈 페이지는 main 폴더에서 관리하고 싶은데 /main을 경로로 하기는 싫어서 위와 같이 작성하였는데 더 좋은 방법이 있을까요?!
There was a problem hiding this comment.
현재 작성하신 모습은 굉장한 안티패턴이라고 생각돼요
라우팅을 제공하는 프레임워크에서 라우팅을 구현한 것이라서요
성능적으로도 그렇고요
말씀하신것처럼 관리 부분을 나누고 싶다면 페이지 단에서 굉장히 큰 페이지라고 부를만한 컴포넌트를 사용하시면 되긴 하겠지만, 그것도 권장되는 방향은 아니라고 생각해요.
ㄴ pages
ㄴ index.tsx
ㄴ components
ㄴ main
ㄴ Foo.tsx
이런식 혹은 다른 디렉터리 명을 써서 구분하면 될 거 같은데, 다른 많은 오픈소스 프로젝트나 예시를 찾아보시면 좋을 거 같아요
잠실종합운동장 야구문화 페이지 디자인
야구문화 페이지에서 확인할 수 있는 먹거리와 즐길거리 카테고리 컴포넌트화
#️⃣ 연관된 이슈
작업한 내용과 관련된 Issue Number를 작성해주세요.
ex) #이슈번호
✅ PR 유형
어떤 변경 사항이 있었나요?
💻 작업 내용
이번 PR에서 작업한 내용에 대해 설명해주세요.
[컴포넌트명]
🙏 리뷰 요구사항
리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요.
ex) OO컴포넌트의 OO메소드명 검토 부탁드립니다.
✅ 체크리스트(PR 올리기 전 아래 내용을 확인해 주세요)