Merged
Conversation
DrawControlPanelに集中していた描画モード非依存のボタンを適切な場所に再配置した。 - ヘルプ: モーダルから左下タブ+左側サイドバー（画面幅50%）に変更 - インポート(CSV/GeoJSON): GeoJSONパネルのボタンエリアに移動 - シェア(URLコピー): GeoJSONパネルに移動 - リセット(GeoJSON初期化): GeoJSONパネルに移動 - DrawControlPanelには描画関連（モード選択、Undo/Redo、確定、削除）のみ残存 closes #51
|
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:
📝 WalkthroughWalkthroughヘルプをモーダルから左下タブ＋スライドインのサイドバーへ変更し、インポート／シェア／リセット機能を DrawControlPanel から GeoJSONPanel に移設。E2E セレクタとステップを新 UI（アイコン＋ポップアップ／サイドバー）へ更新。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant GeoPanel as GeoJSONPanel
participant App as MapView
participant Reader as FileReader
User->>GeoPanel: インポートアイコンをホバー/クリック
GeoPanel->>User: インポートポップアップ表示 (CSV / GeoJSON)
User->>GeoPanel: ファイルを選択
GeoPanel->>Reader: readAsText(file)
Reader-->>GeoPanel: ファイルテキスト / parsed features
GeoPanel->>User: (GeoJSON) 置換 or マージ を確認
User->>GeoPanel: 選択 (replace|merge)
GeoPanel->>App: onImportCSV(text) / onImportGeoJSON(features, mode)
App->>App: マップ状態を更新
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (3)
src/components/HelpSidebar.css (1)
41-54: キーボードフォーカスの見え方を hover と同等にしてください。
.help-tabと.help-sidebar__closeに:focus-visibleがないため、キーボード利用時の操作性が落ちます。♿ 追加例
.help-tab:hover { ... } + +.help-tab:focus-visible { + outline: 2px solid var(--color-blue); + outline-offset: 2px; + border-color: rgba(26, 115, 232, 0.25); + box-shadow: + 0 6px 20px rgba(0, 0, 0, 0.1), + 0 0 12px rgba(26, 115, 232, 0.12); +} .help-sidebar__close:hover { ... } + +.help-sidebar__close:focus-visible { + outline: 2px solid var(--color-purple); + outline-offset: 2px; + background: rgba(124, 58, 237, 0.1); + color: var(--color-purple); +}Also applies to: 148-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpSidebar.css` around lines 41 - 54, Add matching :focus-visible rules for keyboard focus so .help-tab and .help-sidebar__close get the same visual treatment as :hover; specifically create selectors .help-tab:focus-visible and .help-sidebar__close:focus-visible that apply the same background linear-gradient, border-color, color (var(--color-blue)), box-shadow, and transform (translateY(-1px)) as the .help-tab:hover rule, and ensure any default outline is managed (e.g., keep or replace with a visible focus indicator) so keyboard users see the same state.e2e/support/helpers/selectors.ts (1)
33-34: コメントのみで対応するセレクタが定義されていません。「インポート（GeoJSONパネル内）」というコメントがありますが、その下にセレクタ定義がありません。旧セレクタ（
IMPORT_ACTION_BUTTON、IMPORT_POPUP_BTN）を削除した後の残骸と思われます。E2Eテストでインポートボタンを操作する際に
GEOJSON_PANEL配下のボタンを直接参照する方針であればコメントは不要です。削除するか、実際に使用するセレクタを定義することを検討してください。🔧 提案: 不要なコメントを削除
-/** インポート（GeoJSONパネル内） */ - /** 住所検索バー */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/support/helpers/selectors.ts` around lines 33 - 34, The comment "インポート（GeoJSONパネル内）" is left without any selector definitions (likely after removing IMPORT_ACTION_BUTTON and IMPORT_POPUP_BTN); either remove this orphaned comment or restore/define the actual selectors you intend to use under that heading (e.g., add definitions for IMPORT_ACTION_BUTTON, IMPORT_POPUP_BTN, or document that GEOJSON_PANEL's child buttons will be referenced directly), and ensure the selectors referenced by tests match the names used in selectors.ts (GEOJSON_PANEL, IMPORT_ACTION_BUTTON, IMPORT_POPUP_BTN).src/components/GeoJSONPanel.tsx (1)
97-112: FileReader のエラーハンドリングを追加することを検討してください。JSON パースエラーは catch されていますが、
reader.onerrorが設定されていないためファイル読み込み自体の失敗は検知できません。また、Line 107 の
console.errorのみではユーザーに失敗が伝わりません。トースト通知などでユーザーへフィードバックを提供することを検討してください。🛡️ 提案: エラーハンドリングの強化
const handleGeoJSONFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0] if (!file) return const reader = new FileReader() reader.onload = () => { try { const features = parseGeoJSONImport(reader.result as string) const replace = window.confirm('既存のフィーチャを置き換えますか？\n「OK」→ 置換\n「キャンセル」→ マージ（追加）') onImportGeoJSON(features, replace ? 'replace' : 'merge') } catch (err) { console.error('GeoJSON のインポートに失敗しました', err) + window.alert('GeoJSON のインポートに失敗しました。ファイル形式を確認してください。') } } + reader.onerror = () => { + console.error('GeoJSON ファイルの読み込みに失敗しました', reader.error) + } reader.readAsText(file) e.target.value = '' }, [onImportGeoJSON])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 97 - 112, The file-read branch in handleGeoJSONFileChange lacks reader.onerror and user-facing feedback: add a reader.onerror handler that logs the error (including the error event) and reports it to the user (e.g., via existing toast/notification system or fallback alert), ensure the same user-facing message is used as in the JSON parse catch, include error details when calling parseGeoJSONImport failures, and guarantee the input is reset (e.target.value = '') even when a read error occurs so the file input remains usable; reference handleGeoJSONFileChange, reader.onerror, parseGeoJSONImport, and onImportGeoJSON when making changes.
🤖 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/components/GeoJSONPanel.tsx`:
- Around line 86-95: The file lacks FileReader error handling in
handleCSVFileChange: add reader.onerror to catch read failures and surface a
user-facing error (e.g., call an existing or new callback like onImportError or
set an error state) with the FileReader.error details; also wrap the
onImportCSV(reader.result as string) call in a try/catch so CSV parse errors
from onImportCSV are caught and reported the same way (use the same
onImportError or state update), and ensure e.target.value is still reset in both
success and error paths so the input clears consistently.
In `@src/components/HelpSidebar.css`:
- Around line 3-39: The Stylelint declaration-empty-line-before errors come from
inconsistent blank lines inside the .help-tab rule (and the similar block around
lines 63-93); open the .help-tab CSS rule and any other affected rule, then
normalize empty lines so they follow your project's
declaration-empty-line-before configuration by either removing stray blank lines
between declarations or adding a single required blank line before declaration
groups—ensure all property declarations (e.g., position, bottom, left, z-index,
display, align-items, gap, padding, border-radius, background, backdrop-filter,
border, box-shadow, font-size, font-weight, color, cursor, transition) follow
the same empty-line pattern and apply the same fix to the other block referenced
(the rule in the 63-93 range).
In `@src/components/HelpSidebar.tsx`:
- Around line 55-67: The dialog lacks aria-modal/aria-labelledby and initial
focus handling: add aria-modal="true" to the div with role="dialog" and set
aria-labelledby to the h2 by giving the title an id (e.g., helpSidebarTitle) so
assistive tech reads the label; implement initial focus in the HelpSidebar
component by creating a ref for the close button (e.g., closeButtonRef) and in a
useEffect that runs when isOpen becomes true call closeButtonRef.current.focus()
(also ensure onClose remains attached to that button) so keyboard users land in
the dialog when it opens.
---
Nitpick comments:
In `@e2e/support/helpers/selectors.ts`:
- Around line 33-34: The comment "インポート（GeoJSONパネル内）" is left without any
selector definitions (likely after removing IMPORT_ACTION_BUTTON and
IMPORT_POPUP_BTN); either remove this orphaned comment or restore/define the
actual selectors you intend to use under that heading (e.g., add definitions for
IMPORT_ACTION_BUTTON, IMPORT_POPUP_BTN, or document that GEOJSON_PANEL's child
buttons will be referenced directly), and ensure the selectors referenced by
tests match the names used in selectors.ts (GEOJSON_PANEL, IMPORT_ACTION_BUTTON,
IMPORT_POPUP_BTN).
In `@src/components/GeoJSONPanel.tsx`:
- Around line 97-112: The file-read branch in handleGeoJSONFileChange lacks
reader.onerror and user-facing feedback: add a reader.onerror handler that logs
the error (including the error event) and reports it to the user (e.g., via
existing toast/notification system or fallback alert), ensure the same
user-facing message is used as in the JSON parse catch, include error details
when calling parseGeoJSONImport failures, and guarantee the input is reset
(e.target.value = '') even when a read error occurs so the file input remains
usable; reference handleGeoJSONFileChange, reader.onerror, parseGeoJSONImport,
and onImportGeoJSON when making changes.
In `@src/components/HelpSidebar.css`:
- Around line 41-54: Add matching :focus-visible rules for keyboard focus so
.help-tab and .help-sidebar__close get the same visual treatment as :hover;
specifically create selectors .help-tab:focus-visible and
.help-sidebar__close:focus-visible that apply the same background
linear-gradient, border-color, color (var(--color-blue)), box-shadow, and
transform (translateY(-1px)) as the .help-tab:hover rule, and ensure any default
outline is managed (e.g., keep or replace with a visible focus indicator) so
keyboard users see the same state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
e2e/features/csv-import.featuree2e/features/help-modal.featuree2e/step-definitions/csv-import-steps.tse2e/step-definitions/help-modal-steps.tse2e/support/helpers/selectors.tssrc/components/DrawControlPanel.csssrc/components/DrawControlPanel.tsxsrc/components/GeoJSONPanel.csssrc/components/GeoJSONPanel.tsxsrc/components/HelpModal.csssrc/components/HelpModal.tsxsrc/components/HelpSidebar.csssrc/components/HelpSidebar.tsxsrc/components/MapView.tsx
💤 Files with no reviewable changes (4)
- src/components/HelpModal.tsx
- src/components/DrawControlPanel.tsx
- src/components/HelpModal.css
- src/components/DrawControlPanel.css
Comment on lines
+86
to
+95
| const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0] | ||
| if (!file) return | ||
| const reader = new FileReader() | ||
| reader.onload = () => { | ||
| onImportCSV(reader.result as string) | ||
| } | ||
| reader.readAsText(file) | ||
| e.target.value = '' | ||
| }, [onImportCSV]) |
There was a problem hiding this comment.
FileReader のエラーハンドリングが不足しています。
reader.onerror が設定されていないため、ファイル読み込み失敗時（権限エラー、ファイル破損など）にユーザーへのフィードバックがありません。また、CSV パースエラー（onImportCSV 内部で発生する可能性）時のユーザー通知もこのコンポーネントでは行われていません。
🛡️ 提案: エラーハンドリングの追加
const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0]
if (!file) return
const reader = new FileReader()
reader.onload = () => {
onImportCSV(reader.result as string)
}
+ reader.onerror = () => {
+ console.error('CSV ファイルの読み込みに失敗しました', reader.error)
+ }
reader.readAsText(file)
e.target.value = ''
}, [onImportCSV])📝 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
| const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { | |
| const file = e.target.files?.[0] | |
| if (!file) return | |
| const reader = new FileReader() | |
| reader.onload = () => { | |
| onImportCSV(reader.result as string) | |
| } | |
| reader.readAsText(file) | |
| e.target.value = '' | |
| }, [onImportCSV]) | |
| const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { | |
| const file = e.target.files?.[0] | |
| if (!file) return | |
| const reader = new FileReader() | |
| reader.onload = () => { | |
| onImportCSV(reader.result as string) | |
| } | |
| reader.onerror = () => { | |
| console.error('CSV ファイルの読み込みに失敗しました', reader.error) | |
| } | |
| reader.readAsText(file) | |
| e.target.value = '' | |
| }, [onImportCSV]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/GeoJSONPanel.tsx` around lines 86 - 95, The file lacks
FileReader error handling in handleCSVFileChange: add reader.onerror to catch
read failures and surface a user-facing error (e.g., call an existing or new
callback like onImportError or set an error state) with the FileReader.error
details; also wrap the onImportCSV(reader.result as string) call in a try/catch
so CSV parse errors from onImportCSV are caught and reported the same way (use
the same onImportError or state update), and ensure e.target.value is still
reset in both success and error paths so the input clears consistently.
- .help-tab: left: 0, bottom: 72px, 左側border-radius: 0（画面端にぴったり配置） - リセットボタン: GeoJSONパネル → DrawControlPanelに戻す - インポートボタン(CSV/GeoJSON): GeoJSONパネルのボタンエリア → ヘッダー部に移動
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/components/HelpSidebar.css (1)
3-39:⚠️ Potential issue | 🟡 MinorStylelintエラー（
declaration-empty-line-before）が未解消です。Line 3-39 と Line 63-93 の宣言前空行が、報告されている lint ルール違反に該当します。空行を整理して宣言を連続させてください。
修正イメージ（抜粋）
.help-tab { position: fixed; bottom: 72px; left: 0; z-index: 10; - display: flex; align-items: center; gap: 6px; padding: 8px 14px; border-radius: 0 10px 10px 0; - background: linear-gradient( 160deg, @@ ); backdrop-filter: blur(var(--glass-blur-strong)); -webkit-backdrop-filter: blur(var(--glass-blur-strong)); - border: 1px solid var(--glass-border); box-shadow: 0 4px 16px rgba(0, 0, 0, 0.08), 0 2px 6px rgba(26, 115, 232, 0.06); - font-size: 13px; font-weight: 600; color: `#374151`; cursor: pointer; - transition: background var(--transition-normal), border-color var(--transition-normal), @@ .help-sidebar { position: fixed; top: 0; left: 0; bottom: 0; width: 50vw; max-width: 600px; min-width: 320px; z-index: 100; - display: flex; flex-direction: column; box-sizing: border-box; - background: linear-gradient( @@ ); backdrop-filter: blur(var(--glass-blur-strong)); -webkit-backdrop-filter: blur(var(--glass-blur-strong)); - border-right: 1px solid rgba(255, 255, 255, 0.5); box-shadow: 8px 0 32px rgba(0, 0, 0, 0.12), 2px 0 8px rgba(124, 58, 237, 0.04); - animation: help-sidebar-slide-in 0.25s ease-out; }Also applies to: 63-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpSidebar.css` around lines 3 - 39, The CSS for the .help-tab block violates stylelint's declaration-empty-line-before rule because there are unnecessary blank lines between property declarations; remove the empty lines so all declarations are consecutive (e.g., group display, align-items, gap, padding, border-radius, background, backdrop-filter, border, box-shadow, font-size, font-weight, color, cursor, transition without blank lines between them) and apply the same cleanup to the other affected block (lines referenced in the review) to satisfy declaration-empty-line-before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/HelpSidebar.css`:
- Around line 3-39: The CSS for the .help-tab block violates stylelint's
declaration-empty-line-before rule because there are unnecessary blank lines
between property declarations; remove the empty lines so all declarations are
consecutive (e.g., group display, align-items, gap, padding, border-radius,
background, backdrop-filter, border, box-shadow, font-size, font-weight, color,
cursor, transition without blank lines between them) and apply the same cleanup
to the other affected block (lines referenced in the review) to satisfy
declaration-empty-line-before.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
e2e/features/csv-import.featuree2e/step-definitions/csv-import-steps.tssrc/components/DrawControlPanel.csssrc/components/DrawControlPanel.tsxsrc/components/GeoJSONPanel.csssrc/components/GeoJSONPanel.tsxsrc/components/HelpSidebar.csssrc/components/MapView.tsx
💤 Files with no reviewable changes (1)
- src/components/DrawControlPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/GeoJSONPanel.tsx
- e2e/step-definitions/csv-import-steps.ts
- src/components/GeoJSONPanel.css
GeoJSONパネルヘッダーのCSV/GeoJSONインポートボタンを、 単一のインポートアイコンにホバーするとポップアップで表示される形式に変更。 E2Eテストも合わせて更新。
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/GeoJSONPanel.tsx (1)
101-127:⚠️ Potential issue | 🟠 Majorインポート処理で読込失敗がサイレントになります。
FileReader.onerrorが未設定のため、ファイル読込失敗時のハンドリングが欠落しています。加えて CSV 側はonImportCSV(...)例外が未捕捉です。既出の指摘内容と同系統で、未解消に見えます。修正例
const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0] if (!file) return const reader = new FileReader() reader.onload = () => { - onImportCSV(reader.result as string) + try { + onImportCSV(reader.result as string) + } catch (err) { + console.error('CSV のインポートに失敗しました', err) + } finally { + e.target.value = '' + } } + reader.onerror = () => { + console.error('CSV ファイルの読み込みに失敗しました', reader.error) + e.target.value = '' + } reader.readAsText(file) - e.target.value = '' }, [onImportCSV]) const handleGeoJSONFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0] if (!file) return const reader = new FileReader() reader.onload = () => { try { const features = parseGeoJSONImport(reader.result as string) const replace = window.confirm('既存のフィーチャを置き換えますか？\n「OK」→ 置換\n「キャンセル」→ マージ（追加）') onImportGeoJSON(features, replace ? 'replace' : 'merge') } catch (err) { console.error('GeoJSON のインポートに失敗しました', err) + } finally { + e.target.value = '' } } + reader.onerror = () => { + console.error('GeoJSON ファイルの読み込みに失敗しました', reader.error) + e.target.value = '' + } reader.readAsText(file) - e.target.value = '' }, [onImportGeoJSON])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 101 - 127, Both handlers lack FileReader.onerror and CSV import lacks try/catch: update handleCSVFileChange and handleGeoJSONFileChange to set reader.onerror to handle and log read errors (and show user feedback), wrap the onImportCSV(reader.result as string) call in try/catch to catch/ log/handle exceptions, and ensure reader.onerror clears the input value and surfaces an appropriate error message; for handleGeoJSONFileChange keep the existing try/catch around parseGeoJSONImport but also add reader.onerror to catch file-read failures (and clear e.target.value) before/after calling onImportGeoJSON.
🤖 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/components/GeoJSONPanel.css`:
- Around line 117-128: Remove the extra blank lines that trigger stylelint's
declaration-empty-line-before rule by collapsing the spacing so declarations are
contiguous: in the shown block containing the background, backdrop-filter,
-webkit-backdrop-filter, border, box-shadow and white-space properties, remove
the empty lines between the background gradient lines (around the end of the
background declaration) and between the box-shadow and white-space declarations
so there are no stray blank lines before declarations.
In `@src/components/GeoJSONPanel.tsx`:
- Around line 141-176: The import popup is only opened via mouse events; update
the geojson-panel import controls to support keyboard and touch by adding an
onClick handler to the import button that toggles the same state used by
isImportHovered (reuse or rename handlers like
handleImportMouseEnter/handleImportMouseLeave into a toggle function), add
onFocus/onBlur to the wrapper div to show/hide the popup on keyboard focus, add
aria-haspopup="true" and aria-expanded={isImportHovered} to the import button,
and add role="menu" to the popup div (keep existing
csvFileInputRef.current?.click() and geojsonFileInputRef.current?.click()
handlers intact).
---
Duplicate comments:
In `@src/components/GeoJSONPanel.tsx`:
- Around line 101-127: Both handlers lack FileReader.onerror and CSV import
lacks try/catch: update handleCSVFileChange and handleGeoJSONFileChange to set
reader.onerror to handle and log read errors (and show user feedback), wrap the
onImportCSV(reader.result as string) call in try/catch to catch/ log/handle
exceptions, and ensure reader.onerror clears the input value and surfaces an
appropriate error message; for handleGeoJSONFileChange keep the existing
try/catch around parseGeoJSONImport but also add reader.onerror to catch
file-read failures (and clear e.target.value) before/after calling
onImportGeoJSON.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
e2e/features/csv-import.featuree2e/step-definitions/csv-import-steps.tssrc/components/GeoJSONPanel.csssrc/components/GeoJSONPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/step-definitions/csv-import-steps.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/GeoJSONPanel.tsx (2)
101-110:⚠️ Potential issue | 🟡 MinorCSVインポートで読み込み失敗時の分岐が不足しています。
Line 105-107 は
onImportCSV例外、Line 104-108 はFileReader読み込み失敗が未処理です。失敗時に無反応になるので、reader.onerrorとtry/catchを入れて通知経路を確保してください。修正例
const handleCSVFileChange = useCallback((e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0] if (!file) return const reader = new FileReader() reader.onload = () => { - onImportCSV(reader.result as string) + try { + onImportCSV(reader.result as string) + } catch (err) { + console.error('CSV のインポートに失敗しました', err) + } finally { + e.target.value = '' + } } + reader.onerror = () => { + console.error('CSV ファイルの読み込みに失敗しました', reader.error) + e.target.value = '' + } reader.readAsText(file) - e.target.value = '' }, [onImportCSV])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 101 - 110, The CSV import handler handleCSVFileChange lacks error handling for FileReader and for onImportCSV; add a reader.onerror callback to handle read failures (log/notify and clear e.target.value) and wrap the onImportCSV invocation inside a try/catch so parsing exceptions are caught and reported (use an existing error callback prop if available, otherwise console.error or processLogger), and ensure e.target.value is cleared in both success and error paths; update references: handleCSVFileChange, reader.onload, reader.onerror, and onImportCSV.
141-176:⚠️ Potential issue | 🟠 Majorインポートメニューがマウス操作前提で、キーボード/タッチで到達しづらいです。
Line 143-144 の hover 制御のみだと、操作経路が限定されます。
onClick/onFocus/onBlurと ARIA 属性を追加して、入力手段に依存しない導線にしてください。修正例
<div className='geojson-panel__import-wrapper' onMouseEnter={handleImportMouseEnter} onMouseLeave={handleImportMouseLeave} + onFocus={handleImportMouseEnter} + onBlur={handleImportMouseLeave} > <button type='button' className='geojson-panel__header-button' title='インポート' aria-label='インポート' + aria-haspopup='menu' + aria-expanded={isImportHovered} + onClick={() => setIsImportHovered((v) => !v)} > @@ - {isImportHovered && ( - <div className='geojson-panel__import-popup'> + {isImportHovered && ( + <div className='geojson-panel__import-popup' role='menu'>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 141 - 176, The import menu currently relies only on mouse hover (handleImportMouseEnter/handleImportMouseLeave and isImportHovered) which blocks keyboard/touch users; update the import wrapper and the header button to also handle onClick, onFocus and onBlur to toggle the popup, and add appropriate ARIA attributes (aria-haspopup, aria-expanded on the button, and role="menu" / role="menuitem" on the popup and its buttons) so csvFileInputRef.current?.click() and geojsonFileInputRef.current?.click() remain usable via keyboard and touch; ensure focus/blur logic mirrors hover behavior and that the popup closes on blur or when an import button is activated.
🧹 Nitpick comments (1)
src/components/GeoJSONPanel.tsx (1)
39-54: hoverタイマーのクリーンアップを追加すると安全です。
importHoverTimerRefを使っていますが、アンマウント時のclearTimeoutがないため、後続変更で副作用が混入しやすいです。cleanup を入れておくのを推奨します。修正例
const importHoverTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) +useEffect(() => { + return () => { + if (importHoverTimerRef.current) { + clearTimeout(importHoverTimerRef.current) + importHoverTimerRef.current = null + } + } +}, []) + const handleImportMouseEnter = useCallback(() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 39 - 54, The hover timer isn't cleared on unmount; add a useEffect cleanup that checks importHoverTimerRef.current and calls clearTimeout if present to avoid lingering timers. Locate importHoverTimerRef and the handlers handleImportMouseEnter/handleImportMouseLeave in GeoJSONPanel and add a useEffect(() => () => { if (importHoverTimerRef.current) { clearTimeout(importHoverTimerRef.current); importHoverTimerRef.current = null; } }) so the timeout is cleared when the component unmounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/GeoJSONPanel.tsx`:
- Around line 101-110: The CSV import handler handleCSVFileChange lacks error
handling for FileReader and for onImportCSV; add a reader.onerror callback to
handle read failures (log/notify and clear e.target.value) and wrap the
onImportCSV invocation inside a try/catch so parsing exceptions are caught and
reported (use an existing error callback prop if available, otherwise
console.error or processLogger), and ensure e.target.value is cleared in both
success and error paths; update references: handleCSVFileChange, reader.onload,
reader.onerror, and onImportCSV.
- Around line 141-176: The import menu currently relies only on mouse hover
(handleImportMouseEnter/handleImportMouseLeave and isImportHovered) which blocks
keyboard/touch users; update the import wrapper and the header button to also
handle onClick, onFocus and onBlur to toggle the popup, and add appropriate ARIA
attributes (aria-haspopup, aria-expanded on the button, and role="menu" /
role="menuitem" on the popup and its buttons) so
csvFileInputRef.current?.click() and geojsonFileInputRef.current?.click() remain
usable via keyboard and touch; ensure focus/blur logic mirrors hover behavior
and that the popup closes on blur or when an import button is activated.
---
Nitpick comments:
In `@src/components/GeoJSONPanel.tsx`:
- Around line 39-54: The hover timer isn't cleared on unmount; add a useEffect
cleanup that checks importHoverTimerRef.current and calls clearTimeout if
present to avoid lingering timers. Locate importHoverTimerRef and the handlers
handleImportMouseEnter/handleImportMouseLeave in GeoJSONPanel and add a
useEffect(() => () => { if (importHoverTimerRef.current) {
clearTimeout(importHoverTimerRef.current); importHoverTimerRef.current = null; }
}) so the timeout is cleared when the component unmounts.
- GeoJSONPanel.tsx: FileReaderにonerrorハンドラーを追加（CSV/GeoJSON両方） - GeoJSONPanel.tsx: インポートポップアップにキーボード/タッチ対応（onClick, onFocus/onBlur）とARIA属性（aria-haspopup, aria-expanded, role=menu/menuitem）を追加 - GeoJSONPanel.css: declaration-empty-line-before違反の空行を削除 - HelpSidebar.tsx: aria-modal, aria-labelledby属性と初期フォーカス（closeButtonRef）を追加 - HelpSidebar.css: declaration-empty-line-before違反の空行を削除
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/GeoJSONPanel.tsx (1)
105-107:⚠️ Potential issue | 🟡 MinorCSVインポート成功時コールバックの例外が未捕捉です。
onImportCSV(...)が例外を投げた場合、このハンドラ内で失敗を扱えません。reader.onerrorだけでなくonload側も保護してください。修正例
reader.onload = () => { - onImportCSV(reader.result as string) + try { + onImportCSV(reader.result as string) + } catch (err) { + console.error('CSV のインポートに失敗しました', err) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 105 - 107, Wrap the onImportCSV invocation inside reader.onload with a try/catch to prevent uncaught exceptions from bubbling out; specifically, inside the reader.onload handler where it currently calls onImportCSV(reader.result as string), catch any thrown error and route it to the existing error handling path (e.g., call the same logic used by reader.onerror or an existing error callback prop), or at minimum log the error (console.error) and update component error state so the UI can reflect the failure; ensure you reference reader.onload, reader.onerror and onImportCSV when making the change so the error is handled consistently with the file-reading error flow.
🧹 Nitpick comments (2)
src/components/HelpSidebar.css (1)
83-95:prefers-reduced-motion対応を追加してください。サイドバーのスライドインが常時有効なので、動きに敏感なユーザー向けに
reduce時は無効化しておくのが安全です。💡 提案差分
.help-sidebar { @@ animation: help-sidebar-slide-in 0.25s ease-out; } `@keyframes` help-sidebar-slide-in { @@ } + +@media (prefers-reduced-motion: reduce) { + .help-sidebar { + animation: none; + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpSidebar.css` around lines 83 - 95, Add prefers-reduced-motion handling by disabling the slide animation when users request reduced motion: add a `@media` (prefers-reduced-motion: reduce) block that targets the element using the animation declaration (the rule that sets animation: help-sidebar-slide-in 0.25s ease-out) and override it to animation: none (and set transform: translateX(0) and opacity: 1 to ensure final state); you can keep the `@keyframes` help-sidebar-slide-in for normal users but ensure the media query prevents the keyframes from running for reduced-motion users.src/components/GeoJSONPanel.tsx (1)
39-54: ホバー用タイマーをアンマウント時に破棄してください。
importHoverTimerRefのタイマーが残ると、アンマウント後に state 更新が走る余地が残ります。cleanup を追加するのが安全です。修正例
const handleImportMouseLeave = useCallback(() => { importHoverTimerRef.current = setTimeout(() => { setIsImportHovered(false) importHoverTimerRef.current = null }, 200) }, []) + + useEffect(() => { + return () => { + if (importHoverTimerRef.current) { + clearTimeout(importHoverTimerRef.current) + importHoverTimerRef.current = null + } + } + }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GeoJSONPanel.tsx` around lines 39 - 54, The hover timer referenced by importHoverTimerRef can fire after the component unmounts and cause a state update; add a cleanup that clears the timeout on unmount. Implement a useEffect with no dependencies that returns a cleanup function which checks importHoverTimerRef.current, calls clearTimeout on it if set, and sets importHoverTimerRef.current to null so handleImportMouseEnter/handleImportMouseLeave cannot trigger a stale timer after unmount.
🤖 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/components/GeoJSONPanel.tsx`:
- Around line 149-162: The hover-driven open state
(handleImportMouseEnter/handleImportMouseLeave) conflicts with the click toggle
stored in isImportHovered via setIsImportHovered; replace the click toggle
behavior with an explicit open action and centralize closing (e.g., keep mouse
leave / blur to close). Concretely, change the onClick handler on the import
button to call setIsImportHovered(true) (or a named openImport() function)
instead of toggling, and ensure closing only happens via handleImportMouseLeave
or handleImportBlur/closeImport() so a click immediately after hover does not
invert the state.
---
Duplicate comments:
In `@src/components/GeoJSONPanel.tsx`:
- Around line 105-107: Wrap the onImportCSV invocation inside reader.onload with
a try/catch to prevent uncaught exceptions from bubbling out; specifically,
inside the reader.onload handler where it currently calls
onImportCSV(reader.result as string), catch any thrown error and route it to the
existing error handling path (e.g., call the same logic used by reader.onerror
or an existing error callback prop), or at minimum log the error (console.error)
and update component error state so the UI can reflect the failure; ensure you
reference reader.onload, reader.onerror and onImportCSV when making the change
so the error is handled consistently with the file-reading error flow.
---
Nitpick comments:
In `@src/components/GeoJSONPanel.tsx`:
- Around line 39-54: The hover timer referenced by importHoverTimerRef can fire
after the component unmounts and cause a state update; add a cleanup that clears
the timeout on unmount. Implement a useEffect with no dependencies that returns
a cleanup function which checks importHoverTimerRef.current, calls clearTimeout
on it if set, and sets importHoverTimerRef.current to null so
handleImportMouseEnter/handleImportMouseLeave cannot trigger a stale timer after
unmount.
In `@src/components/HelpSidebar.css`:
- Around line 83-95: Add prefers-reduced-motion handling by disabling the slide
animation when users request reduced motion: add a `@media`
(prefers-reduced-motion: reduce) block that targets the element using the
animation declaration (the rule that sets animation: help-sidebar-slide-in 0.25s
ease-out) and override it to animation: none (and set transform: translateX(0)
and opacity: 1 to ensure final state); you can keep the `@keyframes`
help-sidebar-slide-in for normal users but ensure the media query prevents the
keyframes from running for reduced-motion users.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/components/GeoJSONPanel.csssrc/components/GeoJSONPanel.tsxsrc/components/HelpSidebar.csssrc/components/HelpSidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/HelpSidebar.tsx
Comment on lines
+149
to
+162
| onMouseEnter={handleImportMouseEnter} | ||
| onMouseLeave={handleImportMouseLeave} | ||
| onFocus={handleImportMouseEnter} | ||
| onBlur={handleImportMouseLeave} | ||
| > | ||
| <button | ||
| type='button' | ||
| className='geojson-panel__header-button' | ||
| title='インポート' | ||
| aria-label='インポート' | ||
| aria-haspopup='true' | ||
| aria-expanded={isImportHovered} | ||
| onClick={() => setIsImportHovered((v) => !v)} | ||
| > |
There was a problem hiding this comment.
ホバー表示とクリックトグルが競合して、インポートメニューが閉じることがあります。
hover で開いた直後にボタンをクリックすると、onClick の toggle で閉じる挙動になります。表示状態の制御を一本化したほうが安全です。
修正例（最小）
- onClick={() => setIsImportHovered((v) => !v)}
+ onClick={() => setIsImportHovered(true)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/GeoJSONPanel.tsx` around lines 149 - 162, The hover-driven
open state (handleImportMouseEnter/handleImportMouseLeave) conflicts with the
click toggle stored in isImportHovered via setIsImportHovered; replace the click
toggle behavior with an explicit open action and centralize closing (e.g., keep
mouse leave / blur to close). Concretely, change the onClick handler on the
import button to call setIsImportHovered(true) (or a named openImport()
function) instead of toggling, and ensure closing only happens via
handleImportMouseLeave or handleImportBlur/closeImport() so a click immediately
after hover does not invert the state.
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
1 participant
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.
Summary
DrawControlPanel に集中していた描画モード非依存のボタンを適切な場所に再配置し、UIを整理した。
変更ファイル
HelpSidebar.tsx/.cssGeoJSONPanel.tsx/.cssDrawControlPanel.tsx/.cssMapView.tsxHelpModal.tsx/.css影響範囲
closes #51
Test plan
Summary by CodeRabbit
New Features
Improvements