Merged
Conversation
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
GitHub Actions .github/workflows/ci.yml, .github/workflows/deploy-pages.yml |
Add CI workflow (runs on PRs and pushes to dev/main; Node.js 20; lint/format/build) and Pages deployment workflow (builds site with base path, uploads artifact, deploys via actions/deploy-pages@v4 with least-privilege permissions). |
Router / App src/app/App.tsx |
Compute routerBasename from import.meta.env.BASE_URL and pass it to <BrowserRouter basename={routerBasename}>. |
Manager pages & components src/pages/manager/home/index.tsx |
Flatten JSX nesting; add explicit props to WorkerImageCard and StoreWorkerListItem; reformat mapping output. |
User pages (layout/indent only) src/pages/user/home/index.tsx, src/pages/user/job-lookup-map/index.tsx, src/pages/user/schedule/index.tsx, src/pages/user/workspace-members/index.tsx |
Reindent and restructure JSX branches; workspace-members additionally tightens loading/error guards and consolidates empty/data branches. |
Auth / Login page src/pages/login/index.tsx |
Reorganize JSX wrapper nesting/indentation; no handler or logic changes. |
Shared UI tweaks src/shared/ui/MobileLayout.tsx, src/shared/ui/MobileLayoutWithDocbar.tsx |
Change className string quoting from single to double quotes; no behavior changes. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- [feat] 사장님 메인 페이지 #7 — Related to App routing and ManagerHomePage components; overlaps on manager UI and routing changes.
- [feat] 근무자 목록 페이지 구현 및 API 연동 #6 — Modifies
src/pages/user/workspace-members/index.tsx, related to JSX/layout adjustments in workspace members.
Suggested reviewers
- kim3360
- limtjdghks
Poem
🐰 I hopped through commits, tidy and neat,
New workflows hum, builds skip a beat.
Basenames set, routes find their way,
JSX trimmed for a brighter day —
🍃 small changes, big bunny cheer!
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | The description includes ID and implementation details but lacks a summary in the '변경 내용' section and omits visual demonstrations or referenced documentation. | Expand '변경 내용' with a clear summary of all changes (CI workflows, routing fix), and consider adding '참고 사항' with relevant links or setup instructions if applicable. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically summarizes the main changes: adding CI/CD configuration files (ci.yml, deploy-pages.yml) and related routing setup, which aligns with the PR objectives. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
chore/ALT-165
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/deploy-pages.yml (1)
32-37: Consider deduplicating quality/build steps shared with CI.
npm ci+ lint + format-check + build logic is now duplicated here and in.github/workflows/ci.yml. Extracting shared steps into a reusable workflow (or composite action) will reduce drift and maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-pages.yml around lines 32 - 37, The workflow duplicates the quality/build steps (the npm ci, npm run lint, npm run format:check and the Build for GitHub Pages step that runs npm run build -- --base=/${{ github.event.repository.name }}/); extract these shared steps into a reusable workflow or composite action (e.g., a new callable workflow that runs npm ci, lint, format:check and the build invocation) and then replace the duplicated block in deploy-pages.yml with a workflow_call/uses step that invokes that reusable workflow, ensuring inputs (like repository name or base path) are exposed and passed through so Build for GitHub Pages still receives the correct --base value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1-11: The workflow named "CI" omits an explicit permissions block
causing GITHUB_TOKEN to have broader privileges; add a top-level permissions
block under the "name: CI" header that explicitly grants only the minimal scopes
this workflow needs (for example, contents: read and checks: write or
pull-requests: write as applicable), matching the pattern used in
deploy-pages.yml, and place it above the concurrency/job definitions so the CI
workflow runs with least-privilege GITHUB_TOKEN.
In `@src/app/App.tsx`:
- Around line 28-33: The file fails Prettier checks; run the project's formatter
(e.g., npx prettier --write or yarn format) on the component containing App,
routerBasename and BrowserRouter, or run the repo's formatting script, then
stage and commit the formatted changes so Prettier --check in CI passes. Ensure
the declaration of routerBasename and the JSX return in App are reformatted
according to project Prettier settings before committing.
- Around line 28-29: The routerBasename computation in App.tsx can produce
relative values like '.' which React Router rejects; update the routerBasename
logic (the variable named routerBasename) to normalize and validate BASE_URL
after trimming the trailing slash, and only return a basename when the result is
an absolute non-root path (i.e., starts with '/' and is not '/'); otherwise set
routerBasename to undefined so react-router-dom receives a valid basename.
---
Nitpick comments:
In @.github/workflows/deploy-pages.yml:
- Around line 32-37: The workflow duplicates the quality/build steps (the npm
ci, npm run lint, npm run format:check and the Build for GitHub Pages step that
runs npm run build -- --base=/${{ github.event.repository.name }}/); extract
these shared steps into a reusable workflow or composite action (e.g., a new
callable workflow that runs npm ci, lint, format:check and the build invocation)
and then replace the duplicated block in deploy-pages.yml with a
workflow_call/uses step that invokes that reusable workflow, ensuring inputs
(like repository name or base path) are exposed and passed through so Build for
GitHub Pages still receives the correct --base value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e89fd44-da15-4a02-8b4f-3a0a4dceca9a
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/deploy-pages.ymlsrc/app/App.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/manager/home/index.tsx (1)
91-91: Replace hardcoded color with Tailwind token.
bg-[#EFEFEF]should bebg-bg-darkper the project's Tailwind configuration.♻️ Suggested fix
- <div className="flex min-h-[100dvh] flex-col box-border bg-[`#EFEFEF`]"> + <div className="flex min-h-[100dvh] flex-col box-border bg-bg-dark">Based on learnings: "use bg-bg-dark for '#efefef'. Replace literals like bg-[
#EFEFEF] with the appropriate tokens."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/manager/home/index.tsx` at line 91, Replace the hardcoded hex background color in the JSX div that currently uses className "flex min-h-[100dvh] flex-col box-border bg-[`#EFEFEF`]" and swap the literal bg-[`#EFEFEF`] with the Tailwind token bg-bg-dark (and update any similar hardcoded literals in the same component) so the component uses the project's configured design token instead of an inline hex value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/manager/home/index.tsx`:
- Line 94: The className on the div element in the Manager Home component
contains an unintended leading space (" h-50"); update the JSX in
src/pages/manager/home/index.tsx by removing the leading space so the className
reads "h-50" (locate the div with className=" h-50" and change it to
className="h-50").
In `@src/pages/user/workspace-members/index.tsx`:
- Around line 50-75: The UI currently uses the aggregate hasError to gate all
render branches which hides partial-success data; update the conditional logic
in the render blocks to use per-query status instead of the global OR: replace
usages of hasError in the error and empty-state checks with a conjunction of the
individual error flags (e.g. workersError && managersError) so that only when
both queries fail the full-page error shows, and similarly ensure loading is
based on both queries (e.g. workersLoading || managersLoading) where
appropriate; locate and change the conditions that reference hasError,
isLoading, workers, and managers in the component render to use
workersError/managersError and workersLoading/managersLoading.
---
Nitpick comments:
In `@src/pages/manager/home/index.tsx`:
- Line 91: Replace the hardcoded hex background color in the JSX div that
currently uses className "flex min-h-[100dvh] flex-col box-border bg-[`#EFEFEF`]"
and swap the literal bg-[`#EFEFEF`] with the Tailwind token bg-bg-dark (and update
any similar hardcoded literals in the same component) so the component uses the
project's configured design token instead of an inline hex value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e72b8f35-fcb2-459b-a5dd-f65b511988f8
📒 Files selected for processing (9)
src/app/App.tsxsrc/pages/login/index.tsxsrc/pages/manager/home/index.tsxsrc/pages/user/home/index.tsxsrc/pages/user/job-lookup-map/index.tsxsrc/pages/user/schedule/index.tsxsrc/pages/user/workspace-members/index.tsxsrc/shared/ui/MobileLayout.tsxsrc/shared/ui/MobileLayoutWithDocbar.tsx
✅ Files skipped from review due to trivial changes (6)
- src/shared/ui/MobileLayout.tsx
- src/pages/user/home/index.tsx
- src/shared/ui/MobileLayoutWithDocbar.tsx
- src/pages/user/job-lookup-map/index.tsx
- src/pages/login/index.tsx
- src/pages/user/schedule/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/App.tsx
| alt="logo" | ||
| className="w-full h-full object-cover" | ||
| /> | ||
| <div className=" h-50"> |
There was a problem hiding this comment.
Remove leading space in className.
There's an extra leading space in " h-50" that appears unintentional.
🧹 Suggested fix
- <div className=" h-50">
+ <div className="h-50">📝 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.
Suggested change
| <div className=" h-50"> | |
| <div className="h-50"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/manager/home/index.tsx` at line 94, The className on the div
element in the Manager Home component contains an unintended leading space ("
h-50"); update the JSX in src/pages/manager/home/index.tsx by removing the
leading space so the className reads "h-50" (locate the div with className="
h-50" and change it to className="h-50").
Comment on lines
+50
to
+75
| {!isWorkspaceIdInvalid && hasError && !isLoading && ( | ||
| <div className="bg-white rounded-2xl p-5 shadow-sm border border-red-100"> | ||
| <p className="font-pretendard text-3 text-red-500 m-0"> | ||
| 함께 일하는 사람들 목록을 불러오는 중 오류가 발생했습니다. 잠시 후 | ||
| 다시 시도해주세요. | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {!isWorkspaceIdInvalid && | ||
| !isLoading && | ||
| !hasError && | ||
| workers.length === 0 && | ||
| managers.length === 0 && ( | ||
| <div className="bg-white rounded-2xl p-5 shadow-sm border border-line-1"> | ||
| <p className="font-pretendard text-3 text-text-70 m-0"> | ||
| 워크스페이스 ID가 유효하지 않습니다. 올바른 경로로 다시 | ||
| 접근해주세요. | ||
| 현재 이 워크스페이스에 등록된 근무자 또는 점주/매니저가 | ||
| 없습니다. | ||
| </p> | ||
| </div> | ||
| )} | ||
|
|
||
| {!isWorkspaceIdInvalid && isLoading && ( | ||
| <div className="flex justify-center items-center min-h-[200px]"> | ||
| <Spinner /> | ||
| {!isWorkspaceIdInvalid && | ||
| !isLoading && | ||
| !hasError && | ||
| (workers.length > 0 || managers.length > 0) && ( |
There was a problem hiding this comment.
Partial-success data is currently blocked by global hasError gating.
Because hasError is composed with OR across two independent queries, one failed query can hide successfully loaded data from the other query. This turns partial success into a full-page failure UI.
💡 Proposed fix
@@
const {
@@
} = useWorkspaceMembers({
workspaceId,
initialPageSize: 3,
loadMorePageSize: 10,
})
+
+ const hasAnyMembers = workers.length > 0 || managers.length > 0
@@
- {!isWorkspaceIdInvalid && hasError && !isLoading && (
+ {!isWorkspaceIdInvalid && hasError && !isLoading && !hasAnyMembers && (
<div className="bg-white rounded-2xl p-5 shadow-sm border border-red-100">
<p className="font-pretendard text-3 text-red-500 m-0">
함께 일하는 사람들 목록을 불러오는 중 오류가 발생했습니다. 잠시 후
다시 시도해주세요.
</p>
</div>
)}
@@
- {!isWorkspaceIdInvalid &&
- !isLoading &&
- !hasError &&
- (workers.length > 0 || managers.length > 0) && (
+ {!isWorkspaceIdInvalid && !isLoading && hasAnyMembers && (
<div className="flex flex-col gap-4">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/user/workspace-members/index.tsx` around lines 50 - 75, The UI
currently uses the aggregate hasError to gate all render branches which hides
partial-success data; update the conditional logic in the render blocks to use
per-query status instead of the global OR: replace usages of hasError in the
error and empty-state checks with a conjunction of the individual error flags
(e.g. workersError && managersError) so that only when both queries fail the
full-page error shows, and similarly ensure loading is based on both queries
(e.g. workersLoading || managersLoading) where appropriate; locate and change
the conditions that reference hasError, isLoading, workers, and managers in the
component render to use workersError/managersError and
workersLoading/managersLoading.
limtjdghks
approved these changes
Mar 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ID
변경 내용
구현 사항
Summary by CodeRabbit
New Features
Chores