-
Notifications
You must be signed in to change notification settings - Fork 1
Feat/login page #17
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
Feat/login page #17
Conversation
Summary of ChangesHello @dasosann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays crucial groundwork for the application's data management and service health monitoring. It establishes a robust data fetching layer using React Query and introduces a dedicated provider for handling service status, such as maintenance mode. Additionally, it incorporates essential SVG assets for branding and social login options, which are vital components for the upcoming login page feature. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
전반적으로 React Query와 서비스 상태 관리를 위한 Provider 패턴을 잘 적용하셨습니다. 전역 상태 관리를 위한 구조가 명확하게 잡혀있어 유지보수성이 높아 보입니다. 제공된 규칙(인라인 스타일 사용)은 현재 코드 리뷰 댓글에 직접적으로 적용되는 내용이 없으므로, 모든 기존 댓글을 유지합니다. 몇 가지 개선점을 제안드리며, 특히 환경 변수 사용과 타입 안정성 강화, 그리고 Next.js 서버 컴포넌트의 이점을 최대한 활용하는 방향으로 코드를 개선하면 더욱 견고한 애플리케이션이 될 것입니다. 이번 리뷰에서는 하드코딩된 API URL, 타입 안정성(스타일 가이드 44번 규칙), 서버 컴포넌트에서의 데이터 페칭 최적화 등에 대한 의견을 드렸습니다.
|
@gemini-code-assist review |
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.
Code Review
This pull request establishes the foundation for global state management, integrating React Query and a service status provider to efficiently manage data fetching and maintenance mode. The ServiceStatusProvider effectively prevents client-side flickering by hydrating initial state from server components. However, a critical security vulnerability exists where the client-side status provider uses a hardcoded placeholder URL instead of an environment variable. This could allow an attacker to manipulate the application's maintenance state. General improvements for code stability and maintainability, particularly addressing the hardcoded API address, have also been identified.
app/layout.tsx
Outdated
| async function getInitialMaintenanceStatus() { | ||
| try { | ||
| const res = await fetch(`${process.env.NEXT_PUBLIC_API_URL}/status`, { | ||
| cache: "no-store", | ||
| }); | ||
| if (!res.ok) return false; | ||
| const data = await res.json(); | ||
| return data.maintenance ?? false; | ||
| } catch (error) { | ||
| console.error("Failed to fetch maintenance status:", error); | ||
| return false; | ||
| } | ||
| } |
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.
코드 구조 개선을 위해 getInitialMaintenanceStatus 함수를 별도의 파일로 분리하는 것을 고려해 보세요. 예를 들어 lib/api/service.ts 또는 services/status.ts와 같은 파일에 이 함수를 위치시키면, layout.tsx는 UI 렌더링에만 집중할 수 있고 데이터 fetching 로직의 재사용성과 테스트 용이성이 향상됩니다. 현재는 작은 함수이지만, 앞으로 관련 API 호출이 늘어날 것을 대비하여 미리 구조를 잡아두는 것이 장기적으로 유지보수에 도움이 됩니다.
References
- 컴포넌트와 로직을 분리하여 파일 구조를 더 직관적으로 만들고, 서버와 클라이언트 코드를 명확히 분리하는 것을 권장합니다. 데이터 fetching 로직을 UI 컴포넌트 파일에서 분리하면 코드의 재사용성과 유지보수성이 향상됩니다. (link)
|
@gemini-code-assist review |
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.
Code Review
이번 PR은 React Query와 서비스 상태 관리를 위한 Provider를 도입하여 애플리케이션의 데이터 관리 기반을 마련하는 중요한 변경사항을 담고 있습니다. 전반적인 구조는 훌륭하지만, 몇 가지 치명적인 설정 오류와 개선점이 발견되었습니다. 특히, layout.tsx에서 Provider들이 주석 처리되어 기능이 비활성화된 상태이며, service-status-provider.tsx에는 API 주소가 하드코딩되어 있어 즉각적인 수정이 필요합니다. 또한, 컴포넌트의 재사용성과 유지보수성을 높이기 위해 하드코딩된 값들을 동적으로 처리하거나 디자인 토큰으로 관리하는 것을 권장합니다.
app/layout.tsx
Outdated
| async function getInitialMaintenanceStatus() { | ||
| try { | ||
| const res = await fetch(`${process.env.NEXT_PUBLIC_API_URL}/status`, { | ||
| cache: "no-store", | ||
| }); | ||
| if (!res.ok) return false; | ||
|
|
||
| // Check if response is JSON before parsing | ||
| const contentType = res.headers.get("content-type"); | ||
| if (!contentType || !contentType.includes("application/json")) { | ||
| console.warn("Status endpoint returned non-JSON response"); | ||
| return false; | ||
| } | ||
|
|
||
| const data = await res.json(); | ||
| return data.maintenance ?? false; | ||
| } catch (error) { | ||
| console.error("Failed to fetch maintenance status:", error); | ||
| return false; | ||
| } | ||
| } |
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.
getInitialMaintenanceStatus 함수가 RootLayout 컴포넌트 파일 내에 직접 정의되어 있습니다. 이 함수는 데이터 fetching 로직을 담고 있으므로, lib/api.ts 또는 services/status.ts와 같은 별도의 유틸리티 파일로 분리하면 코드의 관심사 분리(Separation of Concerns) 원칙을 더 잘 지킬 수 있고, 다른 곳에서도 재사용하기 용이해집니다.
References
- 서버와 클라이언트 코드가 명확히 분리되어 있는지, 파일 구조가 직관적인지 확인해야 합니다. 데이터 fetching 로직을 레이아웃 컴포넌트에서 분리하여 파일 구조를 개선할 수 있습니다. (link)
|
@gemini-code-assist review |
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.
Code Review
PR의 주된 내용은 로그인 페이지 UI를 구성하고, React Query와 서비스 상태 관리를 위한 Provider를 추가하는 것입니다. 전반적으로 컴포넌트 구조화가 잘 되어 있고, Next.js의 App Router 패턴을 잘 활용하고 있습니다. 다만, 몇 가지 중요한 수정이 필요합니다. RootLayout에 추가된 Provider들이 주석 처리되어 있어 실제 기능이 동작하지 않는 critical한 문제가 있습니다. 또한, service-status-provider에 API URL이 하드코딩되어 있어 환경 변수로 대체해야 합니다. 그 외 vh 단위 사용으로 인한 잠재적인 레이아웃 시프트 문제, cn 유틸리티 미사용 등 스타일 가이드 및 유지보수성 측면에서 개선할 점들이 있습니다.
|
/describe |
|
PR Description updated to latest commit (bbf3320)
|
|
/describe |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
@gemini-code-assist review |
|
PR Description updated to latest commit (71d878c)
|
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.
Code Review
로그인 페이지 UI 및 관련 컴포넌트 구현이 잘 이루어졌습니다. vaul 라이브러리를 사용한 드로어, QueryProvider, ServiceStatusProvider 등 새로운 기능과 상태 관리 구조가 추가되었습니다. 디자인 토큰 생성 스크립트가 개선되어 유지보수성이 향상되었습니다. 전반적으로 코드 변경 사항이 명확하고 PR 설명이 잘 작성되어 있습니다. 다만, 몇 가지 중요한 수정이 필요합니다. layout.tsx에서 핵심 Provider들이 주석 처리되어 기능이 비활성화된 상태이며, service-status-provider.tsx에서는 API URL이 하드코딩되어 있어 환경에 따라 문제가 발생할 수 있습니다. 또한, 접근성 및 시맨틱 마크업 개선을 위한 제안 사항과 함께, 참여자 수 하드코딩 문제도 해결해야 합니다.
PR Type
Enhancement
Description
로그인 페이지 UI 완성:
ScreenLoginPage,LoginLogoSection,LoginActionSection등 로그인 관련 컴포넌트 구현Drawer UI 컴포넌트 새로 추가:
vaul라이브러리 기반으로 반응형 Drawer 컴포넌트 구현소셜 로그인 통합: 카카오, 구글 로그인 버튼 컴포넌트 구현
서비스 상태 관리 시스템 구축:
ServiceStatusProvider와QueryProvider추가로 유지보수 상태 모니터링 기능 구현디자인 토큰 확장: 로그인 페이지 UI를 위해 144개로 증가 (기존 126개), 타이포그래피 유틸리티 클래스 추가
의존성 업데이트:
@radix-ui/react-dialog,vaul등 필요한 라이브러리 추가Button컴포넌트 prop 정규화:backgroundColor→bgColor로 변경Diagram Walkthrough
File Walkthrough
1 files
tokens.css
로그인 페이지 UI를 위한 디자인 토큰 확장app/tokens.css
13 files
status.ts
서비스 유지보수 상태 확인 함수 구현lib/status.ts
false반환generate-tokens.js
토큰 생성 스크립트 폰트 사이즈 확장scripts/generate-tokens.js
drawer.tsx
Drawer UI 컴포넌트 새로 구현components/ui/drawer.tsx
vaul라이브러리 기반 Drawer 컴포넌트 구현LoginActionSection.tsx
로그인 액션 섹션 컴포넌트 구현app/_components/LoginActionSection.tsx
layout.tsx
루트 레이아웃에 서비스 상태 관리 추가app/layout.tsx
getInitialMaintenanceStatus함수로 초기 유지보수 상태 조회SocialButtonList.tsx
소셜 로그인 버튼 컴포넌트 구현app/_components/SocialButtonList.tsx
service-status-provider.tsx
서비스 상태 관리 프로바이더 구현providers/service-status-provider.tsx
useServiceStatus커스텀 훅 제공Button.tsx
Button 컴포넌트 prop 이름 정규화components/ui/Button.tsx
backgroundColorprop을bgColor로 이름 변경query-provider.tsx
React Query 프로바이더 구현providers/query-provider.tsx
BubbleDiv.tsx
참여 인원 표시 버블 컴포넌트 구현app/_components/BubbleDiv.tsx
LoginLogoSection.tsx
로그인 페이지 로고 섹션 컴포넌트 구현app/_components/LoginLogoSection.tsx
ScreenLoginPage.tsx
로그인 페이지 메인 컴포넌트 구현app/_components/ScreenLoginPage.tsx
page.tsx
홈 페이지를 로그인 페이지로 변경app/page.tsx
ScreenLoginPage컴포넌트로 교체2 files
pnpm-lock.yaml
Drawer UI 컴포넌트 관련 의존성 추가 및 업데이트pnpm-lock.yaml
@radix-ui/react-dialog1.1.15 버전 추가vaul1.1.2 버전 추가 (drawer 컴포넌트용)package.json
로그인 페이지 구현을 위한 의존성 추가 및 업데이트package.json
@radix-ui/react-dialog의존성 추가vaul의존성 추가✨ Describe tool usage guide:
Overview:
The
describetool scans the PR code changes, and generates a description for the PR - title, type, summary, walkthrough and labels. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.When commenting, to edit configurations related to the describe tool (
pr_descriptionsection), use the following template:With a configuration file, use the following template:
Enabling\disabling automation
meaning the
describetool will run automatically on every PR.the tool will replace every marker of the form
pr_agent:marker_namein the PR description with the relevant content, wheremarker_nameis one of the following:type: the PR type.summary: the PR summary.walkthrough: the PR walkthrough.diagram: the PR sequence diagram (if enabled).Note that when markers are enabled, if the original PR description does not contain any markers, the tool will not alter the description at all.
Custom labels
The default labels of the
describetool are quite generic: [Bug fix,Tests,Enhancement,Documentation,Other].If you specify custom labels in the repo's labels page or via configuration file, you can get tailored labels for your use cases.
Examples for custom labels:
Main topic:performance- pr_agent:The main topic of this PR is performanceNew endpoint- pr_agent:A new endpoint was added in this PRSQL query- pr_agent:A new SQL query was added in this PRDockerfile changes- pr_agent:The PR contains changes in the DockerfileThe list above is eclectic, and aims to give an idea of different possibilities. Define custom labels that are relevant for your repo and use cases.
Note that Labels are not mutually exclusive, so you can add multiple label categories.
Make sure to provide proper title, and a detailed and well-phrased description for each label, so the tool will know when to suggest it.
Inline File Walkthrough 💎
For enhanced user experience, the
describetool can add file summaries directly to the "Files changed" tab in the PR page.This will enable you to quickly understand the changes in each file, while reviewing the code changes (diffs).
To enable inline file summary, set
pr_description.inline_file_summaryin the configuration file, possible values are:'table': File changes walkthrough table will be displayed on the top of the "Files changed" tab, in addition to the "Conversation" tab.true: A collapsable file comment with changes title and a changes summary for each file in the PR.false(default): File changes walkthrough will be added only to the "Conversation" tab.Utilizing extra instructions
The
describetool can be configured with extra instructions, to guide the model to a feedback tailored to the needs of your project.Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Notice that the general structure of the description is fixed, and cannot be changed. Extra instructions can change the content or style of each sub-section of the PR description.
Examples for extra instructions:
Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.
More PR-Agent commands
See the describe usage page for a comprehensive guide on using this tool.