Merge develop → main: UI 메뉴 활성화 로직 개선 및 버그 수정 #545
Conversation
This reverts commit eec03af.
fix: 다른 메뉴로 이동 시 메뉴 닫히게 처리 & 주문 메뉴 클릭시 활성화 처리
WalkthroughAdds storeId-aware navigation for admin mobile menu and closes the drawer on item click. Gates the Booth image crop modal to wide viewports and standardizes its title and padding; improves zoom input handling. Adds cross-browser styling for range inputs. Updates multiple user-facing API endpoints to new /v1/... routes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25–30 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/nowait-admin/src/pages/AdminBooth/components/BoothSection.tsx (1)
411-423: Thewidth > 1279condition unnecessarily restricts image cropping to large desktop screens.The ImageCropModal component itself has responsive design with no technical constraints preventing mobile use (
w-full max-w-[520px]layout). However, the condition on line 411 gates the modal to only render when viewport width exceeds 1279px (the tailwindxlbreakpoint), which completely disables image cropping for users on tablets, small laptops, and mobile devices.This appears intentional given the precise threshold alignment with tailwind's breakpoint, but it eliminates the feature entirely rather than adapting it. Consider either:
- Show the modal on all screen sizes (the component supports responsive sizing)
- Display a message informing users to use a larger screen if desktop-only cropping is required
- Implement a mobile-optimized version of the cropping interface
🧹 Nitpick comments (1)
apps/nowait-admin/src/global.css (1)
294-306: Remove or complete the commented-out Firefox range styling.The commented-out Firefox-specific range input styling is incomplete and uses a different color scheme (#3b82f6 vs. #222). Either uncomment and align it with the WebKit styling for cross-browser consistency, or remove it entirely to reduce code clutter.
Apply this diff to remove the commented code:
-/* Firefox */ -/* input[type="range"]::-moz-range-thumb { - width: 14px; - height: 14px; - background: #3b82f6; - border-radius: 50%; - cursor: pointer; -} -input[type="range"]::-moz-range-track { - background: #d1d5db; - height: 6px; - border-radius: 4px; -} */Or, to enable Firefox support with consistent styling:
-/* Firefox */ -/* input[type="range"]::-moz-range-thumb { +input[type="range"]::-moz-range-thumb { width: 14px; height: 14px; - background: #3b82f6; + background: #222; border-radius: 50%; cursor: pointer; + border: none; } input[type="range"]::-moz-range-track { - background: #d1d5db; + background: transparent; height: 6px; border-radius: 4px; -} */ +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
.yarn/cache/@esbuild-win32-x64-npm-0.25.12-2425a2e173-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@parcel-watcher-darwin-arm64-npm-2.5.1-12be747bca-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@parcel-watcher-win32-x64-npm-2.5.1-6e3012ad80-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@rollup-rollup-darwin-arm64-npm-4.53.2-5fce7146cd-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@rollup-rollup-win32-x64-gnu-npm-4.53.2-e204c95015-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@rollup-rollup-win32-x64-msvc-npm-4.53.2-b69273fe45-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@sentry-cli-win32-x64-npm-2.58.0-6714cdeb2b-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@tailwindcss-oxide-darwin-arm64-npm-4.1.17-d42dccd811-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@tailwindcss-oxide-win32-x64-msvc-npm-4.1.17-1eb35761b5-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/fsevents-patch-6b67494872-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/lightningcss-darwin-arm64-npm-1.30.2-57ebf00548-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/lightningcss-win32-x64-msvc-npm-1.30.2-868de13b8a-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/turbo-darwin-arm64-npm-2.6.0-f639398cc6-10c0.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/turbo-windows-64-npm-2.6.0-08a058737d-10c0.zipis excluded by!**/.yarn/**,!**/*.zip
📒 Files selected for processing (4)
apps/nowait-admin/src/components/MobileAdminNav.tsx(1 hunks)apps/nowait-admin/src/global.css(1 hunks)apps/nowait-admin/src/pages/AdminBooth/components/BoothSection.tsx(4 hunks)apps/nowait-admin/src/pages/AdminBooth/components/Modal/ImageCropModal.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/nowait-admin/src/pages/AdminBooth/components/BoothSection.tsx (1)
apps/nowait-admin/src/hooks/useWindowWidth.tsx (1)
useWindowWidth(3-13)
🔇 Additional comments (2)
apps/nowait-admin/src/components/MobileAdminNav.tsx (1)
71-74: LGTM! Menu close behavior correctly implemented.The onClick handler properly navigates to the selected menu item and then closes the mobile navigation menu, implementing the PR objective of closing the menu when navigating to a different menu item.
apps/nowait-admin/src/pages/AdminBooth/components/Modal/ImageCropModal.tsx (1)
98-103: LGTM! Zoom slider enhancement correctly implemented.The enhanced onChange handler correctly calculates the zoom percentage and updates the CSS variable
--valueto drive the gradient track styling defined in global.css. The percentage calculation((v - 1) / (6 - 1)) * 100properly maps the zoom range [1, 6] to [0%, 100%].
| {menuItems.map(({ label, icon, activeIcon, path }) => { | ||
| const isActive = pathname === path; | ||
| //주문의 경우 클릭시 경로에 storeId가 붙기 때문에 추가 검증 | ||
| if (label === "주문") { | ||
| path = `/admin/orders/${storeId}`; | ||
| } | ||
| const isActive = pathname === path; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid mutating the destructured path variable.
Reassigning the path variable that comes from destructuring is confusing and violates clean code principles. Instead, compute the actual path in a separate variable to maintain clarity.
Apply this diff to use a separate variable:
{menuItems.map(({ label, icon, activeIcon, path }) => {
- //주문의 경우 클릭시 경로에 storeId가 붙기 때문에 추가 검증
+ // 주문의 경우 클릭시 경로에 storeId가 붙기 때문에 추가 검증
+ const actualPath = label === "주문" ? `/admin/orders/${storeId}` : path;
- if (label === "주문") {
- path = `/admin/orders/${storeId}`;
- }
- const isActive = pathname === path;
+ const isActive = pathname === actualPath;
return (
<li
key={label}
className={`flex items-center gap-3 px-3 py-2 rounded-md text-title-18-semibold ${
isActive ? "bg-[#f5f5f5] text-black" : "text-black-50"
}`}
- onClick={() => {
- navigate(path);
- onClose();
- }}
+ onClick={() => {
+ navigate(actualPath);
+ onClose();
+ }}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {menuItems.map(({ label, icon, activeIcon, path }) => { | |
| const isActive = pathname === path; | |
| //주문의 경우 클릭시 경로에 storeId가 붙기 때문에 추가 검증 | |
| if (label === "주문") { | |
| path = `/admin/orders/${storeId}`; | |
| } | |
| const isActive = pathname === path; | |
| {menuItems.map(({ label, icon, activeIcon, path }) => { | |
| // 주문의 경우 클릭시 경로에 storeId가 붙기 때문에 추가 검증 | |
| const actualPath = label === "주문" ? `/admin/orders/${storeId}` : path; | |
| const isActive = pathname === actualPath; | |
| return ( | |
| <li | |
| key={label} | |
| className={`flex items-center gap-3 px-3 py-2 rounded-md text-title-18-semibold ${ | |
| isActive ? "bg-[#f5f5f5] text-black" : "text-black-50" | |
| }`} | |
| onClick={() => { | |
| navigate(actualPath); | |
| onClose(); | |
| }} | |
| > |
🤖 Prompt for AI Agents
In apps/nowait-admin/src/components/MobileAdminNav.tsx around lines 59 to 64,
the code mutates the destructured path variable for the "주문" menu item which is
confusing; instead compute a new variable (e.g., resolvedPath or itemPath) that
sets `/admin/orders/${storeId}` when label === "주문" otherwise uses the original
path, then use that new variable for active-checking and any navigation so the
destructured props remain immutable and intent is clear.
feat: api 주소 네임 변경
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/nowait-user/src/api/menu.tsapps/nowait-user/src/api/order.tsapps/nowait-user/src/api/reservation.tsapps/nowait-user/src/hooks/useInfiniteStores.tsapps/nowait-user/src/hooks/useMyWaitingList.tsapps/nowait-user/src/hooks/useWaitingStores.ts
🔇 Additional comments (9)
apps/nowait-user/src/hooks/useWaitingStores.ts (1)
29-29: LGTM: Endpoint updated to versioned path.The endpoint change from
/v1/stores/waiting-listto/v1/stores/waiting-countaligns with the PR's API versioning updates.apps/nowait-user/src/hooks/useInfiniteStores.ts (1)
48-48: LGTM: Endpoint simplified to base stores path.The endpoint change to
/v1/storessimplifies the API structure while maintaining pagination parameters.apps/nowait-user/src/hooks/useMyWaitingList.ts (1)
32-32: LGTM: Endpoint migrated to user-centric v1 path.The endpoint change to
/v1/users/me/waitingsfollows RESTful conventions and aligns with the PR's API versioning strategy.apps/nowait-user/src/api/menu.ts (2)
26-26: LGTM: Menu endpoints restructured under store namespace.The endpoint change to
/v1/stores/${publicCode}/menusimproves API organization by grouping menus under their parent store resource.
39-39: LGTM: Individual menu endpoint follows new pattern.The endpoint change to
/v1/stores/${publicCode}/menus/${menuId}maintains consistency with the getStoreMenus endpoint structure.apps/nowait-user/src/api/order.ts (1)
43-43: LGTM: Payments endpoint updated correctly.The endpoint
/v1/stores/${publicCode}/paymentsis properly structured with a leading slash.apps/nowait-user/src/api/reservation.ts (3)
17-23: LGTM: Store list endpoint updated correctly.The endpoint
/v1/storesis properly structured and includes pagination parameters.
30-36: LGTM: Infinite scroll endpoint consistent with getAllStores.The endpoint maintains consistency with other store-fetching functions.
62-63: LGTM: User-centric endpoints properly structured.All the updated endpoints (
/v1/users/me/waitings,/v1/users/me/bookmarks) follow RESTful conventions with correct leading slashes and align with the PR's API versioning strategy.Also applies to: 68-69, 77-78, 85-86
| ): Promise<CreateOrderServerResponse> => { | ||
| const res = await api.post( | ||
| `/orders/create/${publicCode}/${tableId}`, | ||
| `v1/stores/${publicCode}/tables/${tableId}/orders`, |
There was a problem hiding this comment.
Critical: Missing leading slash in URL path.
The path v1/stores/${publicCode}/tables/${tableId}/orders is missing a leading slash. When combined with the baseURL, this will create a malformed URL. Always include a leading slash when using axios instances with baseURL.
🔎 Proposed fix
- `v1/stores/${publicCode}/tables/${tableId}/orders`,
+ `/v1/stores/${publicCode}/tables/${tableId}/orders`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `v1/stores/${publicCode}/tables/${tableId}/orders`, | |
| `/v1/stores/${publicCode}/tables/${tableId}/orders`, |
🤖 Prompt for AI Agents
In apps/nowait-user/src/api/order.ts around line 24 the request path string is
missing a leading slash so concatenation with the axios instance baseURL
produces an incorrect URL; update the path to include a leading slash (e.g.
change `v1/...` to `/v1/...`) wherever this endpoint is used so axios resolves
the URL correctly.
| tableId: number | ||
| ): Promise<OrderDetailsServerResponse> => { | ||
| const res = await api.get(`/orders/items/${publicCode}/${tableId}`); | ||
| const res = await api.get(`v1/stores/${publicCode}/tables/${tableId}/orders`); |
There was a problem hiding this comment.
Critical: Missing leading slash in URL path.
The path v1/stores/${publicCode}/tables/${tableId}/orders is missing a leading slash. This will create a malformed URL when combined with the axios instance's baseURL.
🔎 Proposed fix
- const res = await api.get(`v1/stores/${publicCode}/tables/${tableId}/orders`);
+ const res = await api.get(`/v1/stores/${publicCode}/tables/${tableId}/orders`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const res = await api.get(`v1/stores/${publicCode}/tables/${tableId}/orders`); | |
| const res = await api.get(`/v1/stores/${publicCode}/tables/${tableId}/orders`); |
🤖 Prompt for AI Agents
In apps/nowait-user/src/api/order.ts around line 35 the GET path lacks a leading
slash which can produce a malformed URL when concatenated with the axios
instance baseURL; update the request to use an absolute path by prefixing the
route with a leading slash (i.e. change to "/v1/stores/…/orders") so axios
correctly resolves the endpoint, and run a quick test to confirm the constructed
URL is valid.
| ) => { | ||
| const res = await UserApi.post( | ||
| `/reservations/create/redis/${storeId}`, | ||
| `v1/users/me/waitings/${storeId}`, |
There was a problem hiding this comment.
Critical: Missing leading slash in URL path.
The path v1/users/me/waitings/${storeId} is missing a leading slash. When used with UserApi (which likely has a configured baseURL), this will create a malformed URL and break reservation creation.
🔎 Proposed fix
- `v1/users/me/waitings/${storeId}`,
+ `/v1/users/me/waitings/${storeId}`,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `v1/users/me/waitings/${storeId}`, | |
| `/v1/users/me/waitings/${storeId}`, |
🤖 Prompt for AI Agents
In apps/nowait-user/src/api/reservation.ts around line 55 the request path uses
`v1/users/me/waitings/${storeId}` which lacks a leading slash and will produce a
malformed URL when combined with the API client's baseURL; update the string to
include a leading slash (i.e. `/v1/users/me/waitings/${storeId}`) so the request
path is absolute and compose correctly with the configured baseURL.
작업 내용
다른 메뉴로 이동 시 메뉴 닫히게 처리 & 주문 메뉴 클릭시 활성화 처리
문제점 및 어려움
해결 방안
공유 사항
Summary by CodeRabbit
Bug Fixes
Improvements
Style
Chores
✏️ Tip: You can customize this high-level summary in your review settings.