Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded bureau "産学局" to DB seeds/migration and app constants; introduced exported helper Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying finansu with
|
| Latest commit: |
8a9d3ec
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96d29ce3.finansu.pages.dev |
| Branch Preview URL: | https://feat-yama-refactor-sponsor-u.finansu.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds the '産学局' bureau to the database schema, seed data, and constants. It also updates the sponsor activities UI by adjusting card widths, refining table layouts, and implementing tooltips for truncated sponsor names. Feedback recommends using the Tooltip component consistently for user names in the desktop view and removing it from the mobile view to address layout inconsistencies and touch-device interaction limitations.
| <Tooltip text={activity.sponsor?.name || '-'}> | ||
| <div className='truncate'>{activity.sponsor?.name}</div> | ||
| </Tooltip> |
There was a problem hiding this comment.
モバイル表示において Tooltip コンポーネントを使用することには、以下の懸念点があります：
- レイアウトの不整合:
Tooltipコンポーネントは内部でitems-centerを使用しているため、企業名が中央揃えになります。カード内の他の項目（担当者、ステータス等）が左揃えであるため、視覚的な一貫性が損なわれます。 - 操作性の制限:
Tooltipはgroup-hoverに依存していますが、タッチデバイスであるモバイル環境ではホバー操作が一般的ではないため、ツールチップが表示されない可能性があります。 - 表示の不備: 59行目の表示テキストにフォールバック（
|| '-'）が設定されていないため、データがない場合に表示が不自然になります。
モバイル環境では Tooltip を使用せず、直接テキストを表示し、必要に応じてフォールバックを設定することを推奨します。これに伴い、使用しなくなる Tooltip のインポートも削除を検討してください。
<div className='truncate'>{activity.sponsor?.name || '-'}</div>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesDesktopSection.tsx`:
- Line 39: The div in SponsorActivitiesDesktopSection.tsx uses a non-existent
Tailwind class "min-w-245" which will be ignored; fix it by either adding a
custom spacing token (e.g., --spacing-245: 245px) to your `@theme` block in
globals.css and mapping it into Tailwind so "min-w-245" becomes valid, or
replace the class on the element with a valid Tailwind utility such as an
existing min-w-* (e.g., min-w-64) or an arbitrary value like min-w-[245px];
update the class on the div in SponsorActivitiesDesktopSection.tsx accordingly.
In
`@view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsx`:
- Around line 57-60: The visible sponsor name should not rely on the Tooltip or
be left blank on touch devices: change the inner div so it renders
activity.sponsor?.name || '-' (not just activity.sponsor?.name) so the fallback
is visible; keep the surrounding <Tooltip text={activity.sponsor?.name || '-'}>
but ensure the truncated div uses the same fallback (the element with className
'truncate' in SponsorActivitiesMobileSection.tsx) so mobile users always see a
readable placeholder when sponsor.name is missing.
In `@view/next-project/src/constants/bureaus.ts`:
- Around line 28-31: The code currently indexes BUREAUS with bureauID (e.g.,
BUREAUS[bureauID]) in createPurchaseReportPdf and createPurchaseOrderPdf which
misaligns IDs (1–7) with array indices (0–6) and crashes for bureauID=7; change
both lookups to use a search: call BUREAUS.find(b => b.id === bureauID) (using
the existing bureauID variable) and then read bureau?.name with a safe fallback
like '不明' to avoid undefined and incorrect names.
🪄 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: fce2fd59-7fc3-4c53-aede-80266ce6d4b7
📒 Files selected for processing (6)
mysql/prdDb/17_bureau.sqlmysql/seed/000001_initial_schema_seed.sqlview/next-project/src/components/sponsor-activities/page/SponsorActivitiesLayout.tsxview/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesDesktopSection.tsxview/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsxview/next-project/src/constants/bureaus.ts
| { | ||
| id: 7, | ||
| name: '産学局', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find all instances of direct array indexing on BUREAUS constant
# Search for BUREAUS[...] pattern that indicates array indexing
rg -n -C3 --type=ts --type=tsx 'BUREAUS\['
# Also search for similar patterns with BUREAUS_WITH_EN
rg -n -C3 --type=ts --type=tsx 'BUREAUS_WITH_EN\['Repository: NUTFes/FinanSu
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for BUREAUS[ pattern with correct file types
rg -n -C3 'BUREAUS\[' --type ts
# Also search BUREAUS_WITH_EN if it exists
rg -n -C3 'BUREAUS_WITH_EN\[' --type tsRepository: NUTFes/FinanSu
Length of output: 1229
🏁 Script executed:
#!/bin/bash
# Find and examine the bureaus.ts constant definition
find . -type f -path '*/constants/bureaus.ts' | head -5Repository: NUTFes/FinanSu
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Find and examine createPurchaseReportPdf.ts around the claimed buggy lines
find . -type f -name 'createPurchaseReportPdf.ts' | head -5Repository: NUTFes/FinanSu
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Examine the bureaus.ts constant definition
cat view/next-project/src/constants/bureaus.tsRepository: NUTFes/FinanSu
Length of output: 883
🏁 Script executed:
#!/bin/bash
# Look at createPurchaseReportPdf.ts around line 101 for more context
sed -n '90,110p' view/next-project/src/utils/createPurchaseReportPdf.tsRepository: NUTFes/FinanSu
Length of output: 526
🏁 Script executed:
#!/bin/bash
# Look at createPurchaseOrderPdf.ts around line 135 for more context
sed -n '125,145p' view/next-project/src/utils/createPurchaseOrderPdf.tsRepository: NUTFes/FinanSu
Length of output: 522
🏁 Script executed:
#!/bin/bash
# Search for any other array indexing patterns on BUREAUS that might be missed
rg -n 'BUREAUS\[|BUREAUS_WITH_EN\[' --type tsRepository: NUTFes/FinanSu
Length of output: 314
Critical: Array indexing bug will cause crashes and incorrect bureau names in PDFs.
Direct array indexing on BUREAUS[bureauID] is used in two locations:
view/next-project/src/utils/createPurchaseReportPdf.ts:101view/next-project/src/utils/createPurchaseOrderPdf.ts:135
Since bureauID values are 1-7 but array indices are 0-6, this causes two problems:
- Runtime crash: When
bureauID=7(産学局), accessingBUREAUS[7]returnsundefined, crashing PDF generation - Pre-existing bug: All bureaus display incorrect names (e.g.,
bureauID=1showsBUREAUS[1]withid: 2instead of the correct bureau)
Both files must use .find() to lookup bureaus by ID instead of direct array indexing:
Recommended fix pattern
// ❌ Wrong: Direct array indexing
const bureauName = BUREAUS[bureauID].name;
// ✓ Correct: Lookup by ID
const bureau = BUREAUS.find(b => b.id === bureauID);
const bureauName = bureau?.name ?? '不明';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@view/next-project/src/constants/bureaus.ts` around lines 28 - 31, The code
currently indexes BUREAUS with bureauID (e.g., BUREAUS[bureauID]) in
createPurchaseReportPdf and createPurchaseOrderPdf which misaligns IDs (1–7)
with array indices (0–6) and crashes for bureauID=7; change both lookups to use
a search: call BUREAUS.find(b => b.id === bureauID) (using the existing bureauID
variable) and then read bureau?.name with a safe fallback like '不明' to avoid
undefined and incorrect names.
…ons/SponsorActivitiesDesktopSection.tsx Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- SponsorActivitiesDesktopSection.tsx: リファクタリングの残骸により壊れていたJSXを削除 - globals.css: 産学局用の --color-industry-university テーマ変数を追加 - createPurchaseOrderPdf.ts / createPurchaseReportPdf.ts: BUREAUS を id で検索するよう修正し、bureauID=7 でのクラッシュとオフバイワンを解消 Co-Authored-By: TkymHrt <23.h.takayama.nutfes@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
view/next-project/src/utils/createPurchaseReportPdf.ts (1)
101-109: Consider centralizing bureau-name resolution to avoid drift.This logic is now duplicated in both PDF generators. Optional, but extracting a shared helper will keep fallback behavior consistent.
♻️ Optional refactor example
--- a/view/next-project/src/constants/bureaus.ts +++ b/view/next-project/src/constants/bureaus.ts @@ export const BUREAUS = [ @@ ] as Bureau[]; + +export const getBureauNameById = (bureauID: number) => + BUREAUS.find((b) => b.id === bureauID)?.name ?? '不明';--- a/view/next-project/src/utils/createPurchaseReportPdf.ts +++ b/view/next-project/src/utils/createPurchaseReportPdf.ts @@ -import { BUREAUS } from '@/constants/bureaus'; +import { getBureauNameById } from '@/constants/bureaus'; @@ - page.drawText( - BUREAUS.find((b) => b.id === purchaseReportViews.reportUser.bureauID)?.name ?? '不明', + page.drawText( + getBureauNameById(purchaseReportViews.reportUser.bureauID), {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@view/next-project/src/utils/createPurchaseReportPdf.ts` around lines 101 - 109, The bureau-name lookup used in the PDF drawing (currently: BUREAUS.find((b) => b.id === purchaseReportViews.reportUser.bureauID)?.name ?? '不明') should be extracted into a shared helper (e.g., getBureauName(bureauID)) and consumed by both PDF generators so the fallback behavior stays consistent; locate the lookup around the page.drawText call and replace it with a call to the new helper, implement getBureauName to accept a bureauID, search BUREAUS for matching b.id, return b.name or '不明', and export/import this helper where the other PDF generator needs it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@view/next-project/src/utils/createPurchaseReportPdf.ts`:
- Around line 101-109: The bureau-name lookup used in the PDF drawing
(currently: BUREAUS.find((b) => b.id ===
purchaseReportViews.reportUser.bureauID)?.name ?? '不明') should be extracted into
a shared helper (e.g., getBureauName(bureauID)) and consumed by both PDF
generators so the fallback behavior stays consistent; locate the lookup around
the page.drawText call and replace it with a call to the new helper, implement
getBureauName to accept a bureauID, search BUREAUS for matching b.id, return
b.name or '不明', and export/import this helper where the other PDF generator
needs it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d2b3aee-b431-4846-9687-2fa179fce7f9
📒 Files selected for processing (4)
view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesDesktopSection.tsxview/next-project/src/styles/globals.cssview/next-project/src/utils/createPurchaseOrderPdf.tsview/next-project/src/utils/createPurchaseReportPdf.ts
✅ Files skipped from review due to trivial changes (1)
- view/next-project/src/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (1)
- view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesDesktopSection.tsx
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ネイティブのtitle属性に戻すことで、オーバーフロー境界で切り取られる 問題を回避 Co-Authored-By: TkymHrt <23.h.takayama.nutfes@gmail.com>
| page.drawText(getBureauName(purchaseOrdersViews.user.bureauID), { | ||
| x: 100, | ||
| y: height - 95, | ||
| y: height - 125, |
There was a problem hiding this comment.
🔴 Bureau name y-coordinate incorrectly changed, misaligning PDF layout
The bureau name's y-coordinate in the purchase order PDF was changed from height - 95 to height - 125, placing it 30 pixels below the "提出者" label (line 131: y: height - 95) and the "(担当:...)" text (line 143: y: height - 95). These three text elements should all share the same y-coordinate to appear on the same line. The equivalent code in view/next-project/src/utils/createPurchaseReportPdf.ts:103 correctly keeps y: height - 95.
| y: height - 125, | |
| y: height - 95, |
Was this helpful? React with 👍 or 👎 to provide feedback.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsx (1)
7-7:⚠️ Potential issue | 🟠 MajorHover-only tooltip still makes full sponsor names hard to access on mobile.
Line 58 relies on
Tooltip, but the current tooltip implementation is hover-triggered (group-hover), so truncated names from Line 59 are not reliably expandable on touch devices. This is inconsistent with the desktop approach usingtitle.💡 Proposed fix
-import { Card, Tooltip } from '@components/common'; +import { Card } from '@components/common'; ... <div className='text-black-300 text-lg font-medium'> - <Tooltip text={activity.sponsor?.name || '-'}> - <div className='truncate'>{activity.sponsor?.name || '-'}</div> - </Tooltip> + <div className='truncate' title={activity.sponsor?.name || '-'}> + {activity.sponsor?.name || '-'} + </div> </div>Also applies to: 57-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsx` at line 7, The mobile truncated sponsor names are wrapped with the hover-only Tooltip component (Tooltip) which relies on group-hover and doesn't work on touch devices; update the SponsorActivitiesMobileSection rendering so the sponsor name uses a touch-friendly tooltip behavior—either switch to the element's native title attribute for the truncated name or configure Tooltip to use a click/focus trigger (e.g., trigger="click" or onClick toggle) so tapping reveals the full name; adjust the JSX where the truncated name is rendered (the element using className "truncate" / the Tooltip wrapper) to use the chosen approach and ensure accessibility (keyboard focus) parity with the desktop title behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@view/next-project/src/utils/createPurchaseOrderPdf.ts`:
- Around line 135-137: The bureau name's y-coordinate in the page.drawText call
for getBureauName is misaligned (uses height - 125) compared to the "提出者" label
and 担当名 which are drawn at height - 95; update the draw call that references
getBureauName(purchaseOrdersViews.user.bureauID) to use the same y value (height
- 95) so the bureau name lines up with the other row, ensuring you adjust only
the y property in that page.drawText invocation.
---
Duplicate comments:
In
`@view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsx`:
- Line 7: The mobile truncated sponsor names are wrapped with the hover-only
Tooltip component (Tooltip) which relies on group-hover and doesn't work on
touch devices; update the SponsorActivitiesMobileSection rendering so the
sponsor name uses a touch-friendly tooltip behavior—either switch to the
element's native title attribute for the truncated name or configure Tooltip to
use a click/focus trigger (e.g., trigger="click" or onClick toggle) so tapping
reveals the full name; adjust the JSX where the truncated name is rendered (the
element using className "truncate" / the Tooltip wrapper) to use the chosen
approach and ensure accessibility (keyboard focus) parity with the desktop title
behavior.
🪄 Autofix (Beta)
🔄 Running autofix...
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e06b607a-0546-47cb-87e9-43192246373f
📒 Files selected for processing (6)
view/next-project/src/components/sponsor-activities/page/sections/SponsorActivitiesMobileSection.tsxview/next-project/src/constants/bureaus.tsview/next-project/src/utils/createActivityCsv.tsview/next-project/src/utils/createPurchaseOrderPdf.tsview/next-project/src/utils/createPurchaseReportPdf.tsview/next-project/src/utils/createSponsorshipActivityCsv.ts
✅ Files skipped from review due to trivial changes (2)
- view/next-project/src/utils/createActivityCsv.ts
- view/next-project/src/utils/createSponsorshipActivityCsv.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- view/next-project/src/constants/bureaus.ts
| page.drawText(getBureauName(purchaseOrdersViews.user.bureauID), { | ||
| x: 100, | ||
| y: height - 95, | ||
| y: height - 125, |
There was a problem hiding this comment.
Bureau name Y-position is misaligned with the “提出者” row.
Line 137 moves only the bureau name to height - 125 while the label and担当名 remain at height - 95, causing a visible layout break.
🛠️ Proposed fix
page.drawText(getBureauName(purchaseOrdersViews.user.bureauID), {
x: 100,
- y: height - 125,
+ y: height - 95,
size: fontSizes[0],
font: fontData,
});📝 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.
| page.drawText(getBureauName(purchaseOrdersViews.user.bureauID), { | |
| x: 100, | |
| y: height - 95, | |
| y: height - 125, | |
| page.drawText(getBureauName(purchaseOrdersViews.user.bureauID), { | |
| x: 100, | |
| y: height - 95, | |
| size: fontSizes[0], | |
| font: fontData, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@view/next-project/src/utils/createPurchaseOrderPdf.ts` around lines 135 -
137, The bureau name's y-coordinate in the page.drawText call for getBureauName
is misaligned (uses height - 125) compared to the "提出者" label and 担当名 which are
drawn at height - 95; update the draw call that references
getBureauName(purchaseOrdersViews.user.bureauID) to use the same y value (height
- 95) so the bureau name lines up with the other row, ensuring you adjust only
the y property in that page.drawText invocation.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating fixes for 1 unresolved review comment... This may take a few minutes. The auto-fix coding agent will autonomously read files and make edits. |
1 participant
対応Issue
resolve #0
概要
スポンサー活動の管理において、新たに「産学局」を追加し、UI上での表示を改善しました。
主な変更点：
画面スクリーンショット等
URLスクリーンショット
テスト項目
備考
特になし
Summary by CodeRabbit
New Features
Style & Layout
Bug Fixes