-
Notifications
You must be signed in to change notification settings - Fork 2
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
[react-spa-routing 미션] 송혜정 미션 제출합니다. #1
base: Songhyejeong
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 혜정님! 이번 미션📰은 어떠셨나요?
점심 뭐 먹지
미션이 끝나고 새로운 미션을 맞이 하니 즐거우셨나요?ㅎㅎ.. 제가 만든 미션을 처음으로 이용해주셨네요..! 무한 감사할 따름입니다😆또, 미션을 진행하시면서 문서를 읽고 이건 좀 애매한데?
,이건 내가 훈수두고 싶은데?
하셨던 것들은 언제나 훈수 환영입니다! 꼭 알려주세요!
일단 코드에 대해서 이야기해보자면 "읽기 쉽고 이해하기 편한 코드" 였어요. 제가 해결사항으로 내드린 "이 문제" 에 대한 것들도 전부 해결해주셔서 좋았습니다. 에러나 로딩시 분기처리에 대한 로직도 스터디 때 다뤘던 주제들인데 전부 잘 적용해주셔서 나름 많이 뿌듯했네요😆 물론 이전에도 하셨던 것들이겠지만 이제 실압근 생활근육으로 자리잡은 것 같아요!
🍰 개선점
제가 혜정님의 코드를 돌려보면서 유저 입장에서 이상하게 여길만한 두 가지 정도 개선사항을 공유해볼게요.
1. 처음 앱을 구동시켰을 때 아무것도 보이지 않는다.
처음 앱을 구동시켰을 때 아무것도 보이지 않고 있습니다! 물론 사용하는데 지장은 없지만 처음 켰을 때 ALL
에 나와있는 뉴스들이 나오면 좋지 않을까요? 해결방안을 찾아서 적용해보세요.
2. 새로고침을 하면 focus되었던 header category가 풀립니다.
focus.issue.mp4
Category가 focus되있음을 유지시키려면 어떻게 해야할까요? 🤔
개선점을 해결해보시구 제가 드린 리뷰들에 대해서 고민해보신 후에 코드에 반영하면 좋을 것 같습니다! 이번 미션의 첫 구현도 성공적으로 수행하셨네요..!
수고 많으셨습니다!!! 🥇
|
||
.env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒
if (newsLoading || allLoading) { | ||
return ( | ||
<MainLayout> | ||
<h3>Loading News...</h3> | ||
</MainLayout> | ||
); | ||
} | ||
|
||
if (newsError || allError) { | ||
return ( | ||
<MainLayout> | ||
<h3>Error</h3> | ||
</MainLayout> | ||
); | ||
} | ||
|
||
const displayedArticles = | ||
category === 'all' ? allArticles.articles : newsItems.articles; | ||
|
||
if (!displayedArticles || displayedArticles.length === 0) { | ||
return ( | ||
<MainLayout> | ||
<h3>No articles available.</h3> | ||
</MainLayout> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 상태가 error거나 loading일 때 적절한 UI분기처리를 해 주신 것으로 보여요. 이 부분은 너무 잘하신 것 같습니다🎉
추가적으로 더 요구하면 욕심이긴 하지만 이 if문
들 안에서 h3
태그를 통해 UI를 나타내고 있는데 혹시 이 공통된 부분에 대해 컴포넌트를 만들어보는 것은 어떨까요?
저희가 예전에 학습했던 합성 컴포넌트로 이 컴포넌트를 만들어서 혹시 기획이 바뀌거나 다른 위치에 쓰일 일이 생길 때 대처할 수 있도록 재사용성을 극대화하는 방향으로 만들면 혜정님의 학습에 더 도움이 될 수 있을 것 같아요!
참고 링크
카카오기술블로그
리액트 합성패턴 번역본
svg { | ||
color: ${({ theme }) => theme.text}; | ||
} | ||
&:hover { | ||
border-radius: 100px; | ||
background: ${({ theme }) => theme.toggleBackground}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
역시 같이 학습했던 대로 props를 잘 이용해주셨네요👏🏻 CSS-in-JS만의 특징입니다!
<Suspense fallback={<div>...loading</div>}> | ||
<Header /> | ||
<main> | ||
<Routes> | ||
<Route path="/:category" element={<Main />}></Route> | ||
</Routes> | ||
</main> | ||
</Suspense> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 Suspense
는 어떻게 사용되고 있는지 설명해주실 수 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
import axios from 'axios'; | ||
import { useParams } from 'react-router-dom'; | ||
|
||
const apiKey = import.meta.env.VITE_MY_API_KEY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useGet~
으로 시작하는 custom hooks가 뭔가 모양새가 비슷하지 않나요? api endpoint
가 다를 때 각각의 로직으로 처리해서 하나의 hook으로 합칠 수도 있을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 좋습니다 💯
const ThemeProvider = ({ children }) => { | ||
const initTheme = | ||
localStorage.getItem('theme') === 'dark' ? darkTheme : lightTheme; | ||
const [theme, setTheme] = useState(initTheme ? initTheme : null); | ||
|
||
const toggleDarkmode = () => { | ||
const saveTheme = theme === lightTheme ? darkTheme : lightTheme; | ||
|
||
setTheme(saveTheme); | ||
|
||
localStorage.setItem('theme', saveTheme === lightTheme ? 'light' : 'dark'); | ||
}; | ||
|
||
return ( | ||
<ThemeContext.Provider value={{ theme, toggleDarkmode }}> | ||
{children} | ||
</ThemeContext.Provider> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localStorage
를 이용해서 다크모드전환을 구현하셨군요! 저도 애용하는 방식입니다.
하나 추가로 알려드리면 media query를 이용하면 사용자의 os가 이용하는 화면모드를 앱에 적용할 수 있어요.
import React, { useEffect, useState } from 'react';
import './App.css';
const App = () => {
const [isDarkMode, setIsDarkMode] = useState(window.matchMedia('(prefers-color-scheme: dark)').matches);
useEffect(() => {
const mediaQuery = window.matchMedia('(prefers-color-scheme: dark)');
const handleChange = (e) => setIsDarkMode(e.matches);
mediaQuery.addEventListener('change', handleChange);
return () => {
mediaQuery.removeEventListener('change', handleChange);
};
}, []);
const toggleTheme = () => setIsDarkMode(!isDarkMode);
return (
<div className={isDarkMode ? 'dark-mode' : 'light-mode'}>
<h1>{isDarkMode ? 'Dark Mode' : 'Light Mode'}</h1>
<button onClick={toggleTheme}>Toggle Theme</button>
</div>
);
};
export default App;
이런식으로 말이죠😆
<Link to={`/${item}`} key={item}> | ||
<h3 key={item}>{item.charAt(0).toUpperCase() + item.slice(1)}</h3> | ||
</Link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Header
에서 Link
를 사용하고 있네요! 혹시 NavLink
에 대해 아시나요? Link
와의 차이점은 뭘까요? 혜정님의 코드 즉, Header
에선 무엇을 사용하는 것이 좋을까요?
(거의 물음표 살인마인데..)답변 기다리겠습니다!
안녕하세요 리뷰어님! 🙂
드디어 점심 미션이 끝나고 새로운 미션이네요!
이번 미션은 지금까지 미션 때 학습한 것들을 종합해서 적용해본 느낌이었어요! 그리고 비교적 미션이 자유로워서 정말 재밌게 구현 했던 거 같아요. 👍 👍
라우팅
category 값을 동적 파라미터로 설정하고, category를 파라미터로 받아와서 페이지 전환 처럼 구현해봤습니다. 이때 header의 위치가 main 컴포넌트의 밖에서 독립적으로 존재하는 것이 좋을 것 같아서 뺐더니, category 값으로 Get요청을 해야할 때 이 값을
useParams
로 받아와야 하더라고요, header가 지금처럼 독립적으로 존재하는 것에 대해서 리뷰어님은 어떻게 생각하시는지 궁금합니다..!빈 데이터 처리
이번 미션의 요구 사항 중 하나였던 빈 데이터 값을 어떻게 처리할 지 고민 했었는데요, 뉴스 데이터의 속성 중 하나의 속성이라도 존재할 경우에는, 다른 값들을 default 값을 정해서 처리해봤어요. 이미지는 스켈레톤 이미지로 보여지게 하고 나머지는 No + '속성 값'으로 대체 했습니다. 아예 존재하지 않는 데이터([Removed]) 된 데이터는 사용자에게 보여지게 하지 않아도 될 거 같아 렌더링 되지 않게 구현해봤습니다!
SPA란?
SPA란 Single Page Application으로 웹에 필요한 모든 정적 리소스를 한 번에 다운로드 합니다. 이후, 페이지간 이동시, 필요한 데이터만 JSON으로 전달 받아 페이지를 갱신하므로 전체적인 트래픽을 감소할 수 있고, 전체 페이지를 다시 렌더링하지 않고 변경 되는 부분만을 갱신하므로 리렌더링이 발생하지 않습니다.
이와 같은 특징으로 속도, 사용성, 반응성의 향상으로 사용자 경험을 낫게 해줍니다.
기존의 방식(MPA)와의 차이
새로운 페이지 요청 시 마다 정적 리소스가 다운로드 되고 전체 페이지를 다시 렌더링하므로 새로고침이 발생되어 사용성이 좋지 않습니다. 페이지에서 필요 없는 부분을 포함하여 전체를 갱신하기 때문에 비효율적입니다. 또한, 추후에 모바일 앱 개발 시 추가적인 백엔드 작업이 필요하기 때문에 개발 생산성이 좋지 않을 수도 있습니다.
작동 영상
Screen.Recording.2025-01-14.at.10.26.19.PM.mov
리뷰 잘 부탁드립니다