-
Notifications
You must be signed in to change notification settings - Fork 0
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
[#99] 로그인 페이지 구현 #102
The head ref may contain hidden characters: "99-\uB85C\uADF8\uC778-\uD398\uC774\uC9C0-\uAD6C\uD604"
[#99] 로그인 페이지 구현 #102
Changes from 6 commits
841d8fd
e0e5f6d
1329b3c
d345c25
44bdcdc
b914050
25d513e
25d41f6
b5a3e3c
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,40 @@ | ||
import { css } from '@style/css'; | ||
|
||
const GITHUB_AUTH_URL = 'http://101.101.208.240:3000/auths/github'; | ||
|
||
export default function Login() { | ||
const handleLogIn = async () => { | ||
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. In -> in 으로 바꾸주세요 LogIn은 너무 불편한 것이에요 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. await 가 없는 async 함수네요. |
||
try { | ||
window.location.href = GITHUB_AUTH_URL; | ||
} catch (e) { | ||
const error = e as Error; | ||
console.error(error.message); | ||
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. 개발자에게 알려주기 위해서는 이렇게 console.error |
||
} | ||
}; | ||
|
||
return ( | ||
<section className={loginWrapperStyle}> | ||
<header className={loginHeaderStyle}>Algo With Me</header> | ||
<button onClick={handleLogIn}>Github으로 로그인</button> | ||
</section> | ||
); | ||
} | ||
|
||
const loginWrapperStyle = css({ | ||
border: '1px solid white', | ||
borderRadius: '10px', | ||
width: '100%', | ||
height: '100%', | ||
maxWidth: '500px', | ||
maxHeight: '200px', | ||
display: 'flex', | ||
flexDirection: 'column', | ||
justifyContent: 'space-between', | ||
}); | ||
|
||
const loginHeaderStyle = css({ | ||
fontSize: '3rem', | ||
fontWeight: 'bold', | ||
textAlign: 'center', | ||
padding: '1rem', | ||
}); |
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. 감사합니다! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,31 @@ | ||
import { createBrowserRouter } from 'react-router-dom'; | ||
|
||
import Login from '@/components/Login'; | ||
import ContestPage from '@/pages/ContestPage'; | ||
import ProblemPage from '@/pages/ProblemPage'; | ||
|
||
import App from './App'; | ||
|
||
const router = createBrowserRouter([ | ||
{ | ||
path: '/', | ||
element: <App />, | ||
children: [ | ||
{ | ||
index: true, | ||
path: '/contest/:id', | ||
element: <ContestPage />, | ||
}, | ||
{ | ||
path: '/problem/:id', | ||
element: <ProblemPage />, | ||
}, | ||
], | ||
}, | ||
]); | ||
const router = createBrowserRouter( | ||
[ | ||
{ | ||
path: '/', | ||
element: <App />, | ||
children: [ | ||
{ | ||
index: true, | ||
path: '/contest/:id', | ||
element: <ContestPage />, | ||
}, | ||
{ | ||
path: '/problem/:id', | ||
element: <ProblemPage />, | ||
}, | ||
{ path: '/login', element: <Login /> }, | ||
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. LoginPage로 분리하지 않는 이유가 있나요? 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. 저도 일관성을 위해서 Login 컴포넌트를 pages/LoginPage로 만드는 게 좋다고 생각해서 전 pr에는 그렇게 했었는데, 다른 분들은 불편하신 거 같아서 바꿨는데 그대로 진행해도될까요. 저는 라우터에는 page가 붙는 컴포넌트가 있어야 한다고 생각합니다. 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. 또 다르게 생각하면 컴포넌트를 page로 import하고 그대로 export하는게 더 불편하단 생각도 드는데 어떻게 생각하시나요? 제가 질문에 제대로 이해한 게 맞을까요 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. 제 생각엔 LoginPage를 별도의 페이지로 분리하시려고 했고 그렇다면 pages/LoginPage를 두고 그 안에 Login과 관련된 로직을 둘줄 알았는데 Login컴포넌트를 그냥 넣어서 물어봤습니다. 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. LoginPage만들어서 Login 로직 분리했습니다! |
||
], | ||
}, | ||
], | ||
{ basename: process.env.NODE_ENV === 'production' ? '/web12-algo-with-me' : '' }, | ||
); | ||
|
||
export default router; |
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.
분리한 것 좋네요.