-
Notifications
You must be signed in to change notification settings - Fork 93
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
[17팀 정소윤][Chapter 1-1] 프레임워크 없이 SPA 만들기 #9
Changes from all commits
b509516
5a87400
c2c7e2f
4756577
576194e
ac5c0a8
b22171b
0a6d993
61ec7a3
2dd5567
2b17dbd
a5561bf
117b9c8
b54575c
01fdd6e
e8ef1e5
7eb087f
4debd96
0c05461
ca02e35
a3cd34e
ed8e0e4
9c7f3b1
92570ab
ac4d7b2
5f80d2c
e0622c4
8353655
435ba8c
4f1d4da
94b36d2
8129a84
948cb23
eb35c60
73a00d8
b8fc056
d8d851b
bd25dca
2b835c2
131412f
7ce0458
a31d266
71d396f
7aed1b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"singleQuote": true, | ||
"trailingComma": "all", | ||
"printWidth": 120, | ||
"tabWidth": 2, | ||
"semi": true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,15 @@ | ||
<!doctype html> | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="UTF-8"> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
<title>항해플러스 SNS</title> | ||
<script src="https://cdn.tailwindcss.com"></script> | ||
</head> | ||
<body> | ||
<div id="root"></div> | ||
<script type="module" src="/src/main.js"></script> | ||
</body> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
<title>항해플러스 SNS</title> | ||
<script src="https://cdn.tailwindcss.com"></script> | ||
<link rel="icon" href="./public/favicon.ico" /> | ||
</head> | ||
<body> | ||
<div id="root"></div> | ||
|
||
<script type="module" src="/src/main.js"></script> | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
const Footer = () => { | ||
return /* HTML */ `<footer class="bg-gray-200 p-4 text-center"> | ||
<p>© 2024 항해플러스. All rights reserved.</p> | ||
</footer>`; | ||
}; | ||
|
||
export default Footer(); | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { store } from '../store'; | ||
|
||
const Header = (currentPath) => { | ||
const isLoggedIn = store.getState('isLoggedIn'); | ||
|
||
const menus = [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ name: '홈', path: '/', active: true }, | ||
{ name: '프로필', path: '/profile', active: isLoggedIn }, | ||
{ name: '로그인', path: '/login', active: !isLoggedIn }, | ||
]; | ||
|
||
const render = () => { | ||
return /* HTML */ `<header class="bg-blue-600 text-white p-4 sticky top-0"> | ||
<h1 class="text-2xl font-bold">항해플러스</h1> | ||
</header> | ||
|
||
<nav class="bg-white shadow-md p-2 sticky top-14"> | ||
<ul class="flex justify-around"> | ||
${menus | ||
.map((menu) => { | ||
if (!menu.active) return ''; | ||
return /* HTML */ `<li> | ||
<a href=${menu.path} class="${currentPath === menu.path ? 'text-blue-600 font-bold' : 'text-gray-600'}" | ||
>${menu.name}</a | ||
> | ||
</li>`; | ||
}) | ||
.join('')} | ||
Comment on lines
+19
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. menus.filter(v => v.active).map() 이렇게 미리 filter로 걸러주면 어떨까요!? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
안 그래도 이 부분이 고민이 되던 부분인데요! menues.filter().map()을 할 경우 loop가 두 번 돌아야 한다는 점이 걸려, map으로 한 바퀴만 돌면서 처리하고자 했습니다. 사실 메뉴 수가 많지 않아 크게 영향을 받을 부분은 아니지만 loop를 최소화 해야겠다는 생각이 마음 한 자리에 자리잡고 있어서요!🤣 이런 이유에도 처음에 filter랑 같이 쓰는 것을 고민했던 이유는 가독성 때문이었는데요, 코치님께서 filter로 처리하는 것을 제안주신 이유는 무엇일지 궁금합니다 :) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 상황에 따라 판단하는 게 좋겠군요! 자세한 공유 감사합니다 :) |
||
${isLoggedIn ? '<li><button id="logout" class="text-gray-600">로그아웃</button></li>' : ''} | ||
</ul> | ||
</nav>`; | ||
}; | ||
|
||
let headerHTML = render(); | ||
return headerHTML; | ||
}; | ||
|
||
export default Header; |
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.
소윤님
Footer
처럼 함수를 실행해서 export하는 케이스와Header
처럼 실행하지 않고 export하는 두 가지 케이스가 있더라구요!혹시 내부에서 상태 관리를 하지 않는 컴포넌트는 바로 실행해서 export 하도록 의도하신 걸까요?!
제 생각에는 나중에 Footer 함수에 상태 관리 로직이 추가될 경우 내부에
render
함수를 구현해야 될 것 같은데,"컴포넌트의 구조가 통일되는 것이 나중을 위해 더 좋지 않을까..?"하고 생각하는데 소윤님 생각은 어떤지 궁금합니다!
함수형 컴포넌트라 이 부분이 더 어려웠을 것 같네요 😥
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.
승우님! 리뷰 감사합니다 :)
말씀주신 것처럼 내부에서 상태 관리를 하지 않는 컴포넌트는 바로 실행해서 export 하려는 의도가 맞습니다!
하지만 승우님 말씀 들어보니 컴포넌트는 현재 렌더링을 하든 하지 않든 통일성을 맞추는 게 좋을 것 같네요!
좋은 의견 감사합니다 👍👍