-
Notifications
You must be signed in to change notification settings - Fork 3
Develop #384
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
Conversation
refactor: 모달 크기 수정
|
Caution Review failedThe pull request is closed. WalkthroughReplaced numeric store identifiers with string public codes across API functions and their callers; added Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Frontend Page Components
participant API as local API wrappers (menu.ts / order.ts / reservation.ts)
participant BE as Backend (nowait services)
Note over UI,API: User-triggered flows now pass publicCode (string)
UI->>API: getStoreMenus(publicCode)
API->>BE: GET /v1/menus/{publicCode}
BE-->>API: 200 menus
API-->>UI: menus
UI->>API: createOrder(publicCode, tableId, payload)
API->>BE: POST /orders/create/{publicCode}/{tableId}
BE-->>API: 201 { response: { storeId, publicCode, storeName, ... } }
API-->>UI: createOrder response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (13)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/nowait-user/src/pages/home/components/CancelWaitingModal.tsx (3)
36-41: Centering classes are ineffective without display:flex; add type and loading-safe attrsitems-center/justify-center won’t apply unless the button is a flex container. Also, default type is "submit" which can accidentally submit a surrounding form. While we’re here, disabling the button during isLoading prevents double-interactions.
Apply this diff within the button tag:
- <button - onClick={onClose} - className="flex-1 py-2.5 rounded-[10px] bg-black-20 text-[16px] font-semibold leading-[136%] tracking-normal text-black-65 items-center justify-center" - > + <button + type="button" + onClick={onClose} + disabled={isLoading} + className="flex flex-1 py-2.5 rounded-[10px] bg-black-20 text-[16px] font-semibold leading-[136%] tracking-normal text-black-65 items-center justify-center disabled:opacity-50 disabled:cursor-not-allowed" + >
19-20: Optional: guard against modal dismissal while loadingIf a cancel request is in-flight, consider preventing accidental close via the overlay. A simple pattern is to no-op the onClick when isLoading is true.
Example:
<div className="fixed inset-0 bg-black/60 z-50" onClick={isLoading ? undefined : onClose} />
44-49: Mirror loading-safety and semantics on the Confirm buttonFor consistency, add type="button", disabled while loading, and optionally aria-busy for a11y. Consider a small spinner if available in your design system.
Example:
<button type="button" onClick={onConfirm} disabled={isLoading} aria-busy={isLoading} className="flex flex-1 py-2.5 rounded-[10px] bg-primary text-[16px] font-semibold leading-[136%] tracking-normal text-white-100 items-center justify-center disabled:opacity-50 disabled:cursor-not-allowed" > {isLoading ? "취소 중..." : "확인"} </button>apps/nowait-user/src/pages/login/LoginPage.tsx (2)
14-19: Replace non-standard element with semantic HTML for accessibilityisn’t a valid HTML element. Use a semantic heading (h1/h2) or a span/p with appropriate roles for better a11y and SEO.
Example:
<h1 className="text-headline-28-bold text-black-100"> 기다림 없는 <br /> 우리의 즐거운 축제 </h1>
9-9: Consider 100svh for more reliable mobile viewport heighth-[100dvh] can cause layout jumps under mobile browser UI chrome. 100svh is often a safer fit for full-height sections on mobile.
Example:
<div className="flex flex-col px-6 overflow-x-hidden min-h-[100svh]">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/nowait-user/src/pages/home/components/CancelWaitingModal.tsx(1 hunks)apps/nowait-user/src/pages/login/LoginPage.tsx(2 hunks)
🔇 Additional comments (2)
apps/nowait-user/src/pages/login/LoginPage.tsx (2)
10-12: LGTM: reduced top margin for header blockmt-15 → mt-10 tightens the top whitespace; looks consistent with the overall spacing scale.
20-24: LGTM: tightened spacing around the hero imagemt-16 → mt-12.5 should bring the visual above-the-fold content higher without affecting layout logic.
feat: 주점 publicCode 추가
작업 내용
문제점 및 어려움
해결 방안
공유 사항
Summary by CodeRabbit
Style
Chores