ドラッグ&ドロップで画像選択できるように修正 #9311
ドラッグ&ドロップで画像選択できるように修正 #9311
Conversation
Walkthroughファイル入力要素にドラッグ＆ドロップを追加。ドロップゾーンで Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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: 0
🧹 Nitpick comments (1)
app/javascript/fileinput.js (1)
32-38: ドラッグ&ドロップの実装が正しく動作します実装は正常に機能しますが、より堅牢にするために以下の改善を検討してください:
- ドロップされたファイルの存在確認を追加
- 複数ファイルがドロップされた場合の動作を明示(現在は1つ目のファイルのみ処理されます)
以下のようにバリデーションを追加できます:
dropZone.addEventListener('drop', (e) => { e.preventDefault() + if (!e.dataTransfer.files || e.dataTransfer.files.length === 0) return input.files = e.dataTransfer.files input.dispatchEvent(new Event('change')) })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/javascript/fileinput.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/fileinput.js
🧠 Learnings (1)
📓 Common learnings
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
|
@yokomaru |
|
@karlley |
承知しました！ |
app/javascript/fileinput.js
Outdated
| if (!inputs) return null | ||
|
|
||
| inputs.forEach((input) => { | ||
| const dropZone = input.closest('.js-file-input') |
There was a problem hiding this comment.
app/javascript/fileinput.jsは共通モジュールだと思うので、今回の変更によって他の画面の画像選択フィールド（file_field）にもドラッグ＆ドロップ機能が適用されているかと思います！
↑の部分は画像選択フィールド（file_field）が必ず.js-file-inputの内側にあることが前提になっていると思うので、念のためgit grep "js-file-input" app/viewsなどで、すべての画面で同じDOM構造になっているか確認しておいていただけると安心かなと思いました！（すでに確認済みでしたら申し訳ありません🙏）
DOM構造が統一されていればこの書き方でOKだと思います！🙆♂️
There was a problem hiding this comment.
念のためgit grep "js-file-input" app/viewsなどで、すべての画面で同じDOM構造になっているか確認しておいていただけると安心かなと思いました！
ご指摘の通り他画面への影響の調査が不十分でした。
grepで絞込んで確認したところ全て同一のDOM構造でしたので問題が無いことが確認できました！
ご指摘ありがとうございました😄
app/javascript/fileinput.js
Outdated
| if (!inputs) return null | ||
|
|
||
| inputs.forEach((input) => { | ||
| const dropZone = input.closest('.js-file-input') |
There was a problem hiding this comment.
念のためgit grep "js-file-input" app/viewsなどで、すべての画面で同じDOM構造になっているか確認しておいていただけると安心かなと思いました！
ご指摘の通り他画面への影響の調査が不十分でした。
grepで絞込んで確認したところ全て同一のDOM構造でしたので問題が無いことが確認できました！
ご指摘ありがとうございました😄
yokomaru
left a comment
yokomaru
left a comment
There was a problem hiding this comment.
ご確認ありがとうございます！Approveさせていただきます！🙌
引き続きどうぞよろしくお願いいたします！
|
@yokomaru @okuramasafumi |
okuramasafumi
left a comment
okuramasafumi
left a comment
There was a problem hiding this comment.
LGTM!
偶然かもしれませんが、dropzoneという名前のライブラリがあり、機能としてはまさしくドラッグ&ドロップだったりします。
今のブートキャンプアプリには使われていないライブラリですが、今後入ったときに名前が同じで混乱を招く可能性がゼロとは言えません（動作には影響ないはず）。
今のうちに変数名を変更してもよいとは思うのですが、そこまで重要ではないとも思うので、approveしております。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/javascript/fileinput.js (1)
32-38: ドラッグ＆ドロップ実装は概ねOKですが、changeイベントのバブリングをネイティブ挙動に合わせたいです
.js-file-inputをclosestで辿って drop ハンドラを足す構成は、既存の DOM 構造前提で自然だと思います 👍- 一方で、
input.dispatchEvent(new Event('change'))はデフォルトだとbubbles: falseなので、もし他のコードで「イベントデリゲーション（例:document/ ラッパ要素にchangeをバインド）」している箇所があれば、ドラッグ＆ドロップ時だけそのハンドラが動かない可能性があります。ネイティブのchangeに近づける意味でも、下記のようにしておくとより安全そうです。- input.dispatchEvent(new Event('change')) + input.dispatchEvent(new Event('change', { bubbles: true }))
- ついでに、将来
.js-file-input inputの DOM 構造を変えたときに備えて、dropAreaが取得できなかった場合を防御的にケアしておくのもアリかなと思いました（必須ではないですが）。- const dropArea = input.closest('.js-file-input') - dropArea.addEventListener('dragover', (e) => e.preventDefault()) - dropArea.addEventListener('drop', (e) => { + const dropArea = input.closest('.js-file-input') + if (!dropArea) return + dropArea.addEventListener('dragover', (e) => e.preventDefault()) + dropArea.addEventListener('drop', (e) => { e.preventDefault() input.files = e.dataTransfer.files input.dispatchEvent(new Event('change')) })上記2点はどちらも「将来の拡張・他コードとの連携を考えたときに安全側に寄せる」ための提案なので、今の要件だけであれば現状のままでも動作は問題なさそうです。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/javascript/fileinput.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
app/**/*.{rb,js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Rails app code should be organized in
app/directory with subdirectories:models/,controllers/,views/,jobs/,helpers/, and frontend code underjavascript/(Shakapacker)
Files:
app/javascript/fileinput.js
app/javascript/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
JavaScript/TypeScript code in
app/javascript/should be linted with ESLint and formatted with Prettier viayarn lintscripts; use React 17 and Shakapacker/Webpack 5
Files:
app/javascript/fileinput.js
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: - どうしても避けられない時以外に新規にVue.js, Reactのコードを追加しない。
- fetchメソッドの代わりにrequest.jsを使う。
Files:
app/javascript/fileinput.js
🧠 Learnings (2)
📓 Common learnings
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/notifiers/discord_notifier.rb:131-135
Timestamp: 2025-09-12T21:18:00.834Z
Learning: Rails アップグレードPRにおいて、product_review_not_completed メソッドの webhook URL 設定の shared フォールバック追加も、設定システム全体の変更として別PRで対応すべきである。
📚 Learning: 2025-08-25T08:00:11.369Z
Learnt from: sjabcdefin
Repo: fjordllc/bootcamp PR: 9090
File: app/javascript/user-sns.js:2-25
Timestamp: 2025-08-25T08:00:11.369Z
Learning: app/javascript/user-sns.js において、アクセシビリティ改善（aria-label属性の追加など）は他の箇所との整合性を保つため、別途統一して対応する方針である
Applied to files:
app/javascript/fileinput.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
karlley
force-pushed
the
bug/portfolio-thumbnail-drag-drop
branch
from
54e468e to
3986db2
Compare
|
@okuramasafumi
そうなんですね！ @komagata |
Issue
概要
ドラッグ&ドロップで画像選択できるように修正
変更確認方法
bug/portfolio-thumbnail-drag-dropブランチをローカルに取り込むScreenshot
見た目の変更はありません
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.