Conversation
Walkthrough複数のビューとSass、フロントエンドコンポーネントでプレゼンテーショナルなDOM/スタイル調整を実施。Products/Events のリストを ul→div 構造に変更、いくつかの外側 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Products.jsx
participant Data as props/data
participant P as Pagination
participant U as UnconfirmedLink
Note over UI: Products の描画順序（変更後）
UI->>P: data.total_pages > 1 ? render Pagination (top)
UI->>UI: render div.card-list > div.card-list__items (product items)
UI->>P: data.total_pages > 1 ? render Pagination (bottom)
UI->>U: render UnconfirmedLink
sequenceDiagram
autonumber
participant View as _movie_list.html.slim
participant Movie as movie
Note over View,Movie: タグ描画の条件分岐
View->>Movie: movie.tag_list.present?
alt タグあり
View->>View: タグ用コンテナを出力
View->>Movie: movie.tag_list をループしてタグを出力
else タグなし
View->>View: タグ関連マークアップを出力しない
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 warnings, 1 inconclusive)❌ Failed checks (2 warnings, 1 inconclusive)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.76.1)test/models/company_test.rbrubocop-minitest extension supports plugin, specify ✨ Finishing Touches
🧪 Generate unit tests
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)
app/views/movies/_movie_list.html.slim (1)
34-44: 空hrefのリンクは避けましょう。クリックでページ先頭に飛びます。未実装ならリンクではなくspan等にしておくのが無難です。
- = link_to tag, '', class: 'tag-links__item-link' + = content_tag :span, tag, class: 'tag-links__item-link is-disabled', aria_disabled: true
🧹 Nitpick comments (5)
app/javascript/stylesheets/atoms/_a-block-check.sass (1)
9-9: flexのgap導入とアイコン縮小は良い。旧Safariをサポートするならフォールバック追加を。Safari 13系以前で「flex + gap」が未対応です。サポート対象ならフォールバックを併記しておくと安全です。
追加（選択式・ファイル末尾でも可）:
@supports not (gap: 1rem) .a-block-check__label > * + * margin-left: .25remAlso applies to: 73-73
app/javascript/stylesheets/shared/blocks/card/_thumbnail-card.sass (1)
60-60: font-sizeを直接指定したので、line-heightも明示を。周辺のテキストと行間がズレる可能性があります。1.4を合わせておくと安全。
.thumbnail-card__author display: flex align-items: center - font-size: .875rem + font-size: .875rem + line-height: 1.4app/views/pages/index.html.slim (3)
50-59: ページネーション併用時の空表示判定を修正検討（out-of-range対策）
@pages.empty?だと、総件数はあるが範囲外ページ（例: 最終ページ超過）でも「Docはまだありません」が表示されます。total_count基準にし、範囲外ページ時は別メッセージ（または先頭/最終ページへ誘導）に分岐するのが安全です。- - if @pages.empty? + - if @pages.respond_to?(:total_count) && @pages.total_count.zero? .o-empty-message .o-empty-message__icon - i.fa-regular.fa-smile + i.fa-regular.fa-smile p.o-empty-message__text | Docはまだありません - - else + - elsif @pages.blank? + .o-empty-message + p.o-empty-message__text + = t('shared.pagination.out_of_range', default: 'このページには項目がありません') + = link_to pages_path, class: 'a-button is-md is-secondary is-block' do + = t('shared.pagination.go_to_first', default: '先頭ページへ') + - else .card-list.a-card .card-list__items = render partial: 'page', collection: @pages補足: 下部のページネーション（このブロックの外）との位置関係がモバイルで崩れないか、念のためスクリーンショットでご確認ください。
52-53: 装飾アイコンに aria-hidden を付与装飾目的のアイコンはスクリーンリーダーから隠すとより親切です。
- i.fa-regular.fa-smile + i.fa-regular.fa-smile aria-hidden="true"
55-55: 文言のI18n化固定文言はI18nに寄せておくと再利用・翻訳が容易です。
- | Docはまだありません + = t('pages.index.empty', default: 'Docはまだありません')
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
app/javascript/components/Products.jsx(1 hunks)app/javascript/stylesheets/_common-imports.sass(0 hunks)app/javascript/stylesheets/application/blocks/page/_page-header-actions.sass(0 hunks)app/javascript/stylesheets/application/blocks/page/_page-header.sass(1 hunks)app/javascript/stylesheets/application/blocks/page/_page-main-header.sass(0 hunks)app/javascript/stylesheets/application/blocks/page/_page-tabs.sass(1 hunks)app/javascript/stylesheets/application/blocks/user/_following.sass(1 hunks)app/javascript/stylesheets/application/blocks/user/_users-item.sass(0 hunks)app/javascript/stylesheets/atoms/_a-block-check.sass(2 hunks)app/javascript/stylesheets/shared/blocks/card-list/_products.sass(0 hunks)app/javascript/stylesheets/shared/blocks/card/_thumbnail-card.sass(2 hunks)app/views/external_entries/index.html.slim(0 hunks)app/views/movies/_movie_list.html.slim(1 hunks)app/views/pages/index.html.slim(1 hunks)app/views/reports/index.html.slim(1 hunks)
💤 Files with no reviewable changes (6)
- app/javascript/stylesheets/_common-imports.sass
- app/views/external_entries/index.html.slim
- app/javascript/stylesheets/shared/blocks/card-list/_products.sass
- app/javascript/stylesheets/application/blocks/user/_users-item.sass
- app/javascript/stylesheets/application/blocks/page/_page-header-actions.sass
- app/javascript/stylesheets/application/blocks/page/_page-main-header.sass
🧰 Additional context used
🧬 Code graph analysis (1)
app/javascript/components/Products.jsx (1)
app/javascript/components/Product.jsx (1)
Product(5-113)
🔇 Additional comments (5)
app/javascript/stylesheets/application/blocks/user/_following.sass (1)
11-12: z-indexの明示化はOK。重なり順の実機確認だけお願いします。小要素（ヘルプ、ツールチップ、モーダル等）との重なりで埋もれないか、ヘッダー配下やカード上での表示をスマホ/PCで一度確認ください（特に .users-item 周辺の変更と併せて）。
app/javascript/stylesheets/application/blocks/page/_page-header.sass (1)
17-17: 小画面時のgap追加は妥当です。ヘッダー内要素の縦積み時の密集を解消できています。
app/javascript/stylesheets/shared/blocks/card/_thumbnail-card.sass (1)
13-16: 内側のgapをブレークポイントで切り替える変更は👍小画面の詰まりと中画面以上の余白感の両立が取れています。
app/views/reports/index.html.slim (1)
14-14: ラッパー除去に伴う配置はOK。レイアウト崩れがないかだけ確認を。.page-header-actions系のラッパーを外して直置きに変更。Sass側の新レイアウト（ヘッダー内gap等）でボタンの位置・折返し・余白が期待通りか実機でご確認ください。
app/javascript/stylesheets/application/blocks/page/_page-tabs.sass (1)
4-6: 小画面でのタブ位置調整と幅指定は適切です。ヘッダー直下での重なり/余白問題の解消に寄与しています。
Also applies to: 21-22
| {data.total_pages > 1 && ( | ||
| <Pagination | ||
| sum={data.total_pages * per} | ||
| per={per} | ||
| page={page} | ||
| setPage={setPage} | ||
| /> | ||
| )} | ||
| <ul className="card-list a-card"> | ||
| {data.products.map((product) => { | ||
| return ( | ||
| <Product | ||
| product={product} | ||
| key={product.id} | ||
| isMentor={isMentor} | ||
| isAdmin={isAdmin} | ||
| currentUserId={currentUserId} | ||
| /> | ||
| ) | ||
| })} | ||
| </ul> | ||
| {data.total_pages > 1 && ( | ||
| <Pagination | ||
| sum={data.total_pages * per} | ||
| per={per} | ||
| page={page} | ||
| setPage={setPage} | ||
| /> | ||
| )} | ||
| <UnconfirmedLink label={unconfirmedLinksName()} /> | ||
| </div> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ul直下にdivを並べるのは不正なHTML。div構造に統一してください。
このブロックは
- 配下に
- 以外（がdiv）を直接置いており、アクセシビリティとスタイル崩れの温床です。下記のように.card-list/.card-list__itemsのdiv構造へ統一を。
- <ul className="card-list a-card"> - {data.products.map((product) => { - return ( - <Product - product={product} - key={product.id} - isMentor={isMentor} - isAdmin={isAdmin} - currentUserId={currentUserId} - /> - ) - })} - </ul> + <div className="card-list a-card"> + <div className="card-list__items"> + {data.products.map((product) => { + return ( + <Product + product={product} + key={product.id} + isMentor={isMentor} + isAdmin={isAdmin} + currentUserId={currentUserId} + /> + ) + })} + </div> + </div>
他所でも同様の誤用がないか確認するワンライナー:
🏁 Script executed:
#!/bin/bash # ul.card-list 直下に li 以外が混在していないかの目視トリガーになる候補箇所検索 rg -nP -C2 '<ul\s+className=["\'][^"\']*\bcard-list\b' --type=js --type=jsx --type=tsx
Length of output: 68
🏁 Script executed:
#!/bin/bash # ul.card-list 直下に li 以外が混在していないかの候補箇所検索 rg -nP -C2 "<ul\s+className=[\"'][^\"']*\bcard-list\b" -g "*.js" -g "*.jsx" -g "*.tsx" # Product.jsx が <li> 要素をレンダリングしているか確認 file=$(fd --hidden --no-ignore --type f 'Product.jsx') echo "Product.jsx location: $file" rg -nP "<li\b" -n "$file"
Length of output: 1262
ul直下にdivを並べるのは不正なHTML。card-list構造をdivに統一する
- Products.jsx（app/javascript/components/Products.jsx｜約172行目）と Events.jsx（app/javascript/components/Events.jsx｜約30行目）で
<ul className="card-list a-card">を
<div className="card-list a-card">に置き換え、内部を<div className="card-list__items">でラップしてカードコンポーネントを配置してください。 - 例（Products.jsx／Events.jsx 共通）：
-<ul className="card-list a-card"> - {data.xxx.map(item => ( - <CardComponent key={item.id} .../> - ))} -</ul> +<div className="card-list a-card"> + <div className="card-list__items"> + {data.xxx.map(item => ( + <CardComponent key={item.id} .../> + ))} + </div> +</div>
- 他の同様箇所は以下コマンドで検索・置換を検討してください。
rg -nP "<ul\s+className=[\"'][^\"']*\bcard-list\b" -g "*.js" -g "*.jsx" -g "*.tsx"
🤖 Prompt for AI Agents
In app/javascript/components/Products.jsx around lines 164 to 194, the markup
currently renders a <ul className="card-list a-card"> with direct div children
(invalid HTML); replace the <ul> with a <div className="card-list a-card"> and
wrap the product card elements inside a new container <div
className="card-list__items"> so the card components are children of that inner
div (leave the Pagination and UnconfirmedLink placement unchanged); apply the
same change in app/javascript/components/Events.jsx around line 30 and run the
provided ripgrep command to find and update any other occurrences of <ul
className="...card-list..."> across .js/.jsx/.tsx files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/system/mail_notification_test.rb (2)
10-10: タイトルの厳密一致はフレークになりがち—部分一致/正規表現にして耐性を上げましょう
レイアウトやブランディング変更でサイト名サフィックス等が付与されると落ちやすいです。部分一致（正規表現 or exact: false）にするのを推奨します。- assert_title 'メール通知解除の確認' + assert_title(/メール通知解除の確認/)
7-7: URL直書きではなくパスヘルパーを使用してください
ルーティング変更やホスト切替に強く、可読性も上がります。例:visit edit_user_mail_notification_path(users(:kimura), token: users(:kimura).unsubscribe_email_token)のように、実際のヘルパー名に置き換えてください。Also applies to: 20-20
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/system/mail_notification_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/system/mail_notification_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/system/mail_notification_test.rb
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/models/company_test.rb (1)
8-9: URL拡張子への素朴な部分一致は環境依存になりやすい—境界を考慮した正規表現に。S3等では拡張子がクエリ文字列に入る場合があります。
assert_includesより、クエリの有無を吸収した境界マッチの方が堅牢です。ついでに「デフォルト画像ではない」ことも明示すると意図がよりクリアになります。- assert_includes logo_url, '.webp' + assert_match(/\.(webp)(?:\?|$)/, logo_url)この直後に以下の1行を追加することも検討してください（任意）:
refute_equal '/images/companies/logos/default.png', logo_url
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/models/company_test.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
⚙️ CodeRabbit configuration file
**/*.rb: # refactoring
- まずFat Controllerを避け、次にFat Modelを避ける。
- Serviceクラスの乱用を避ける。
- controller concernを作ろうとしたらPORO(Plain Old Ruby Object)やActiveRecordモデルでの実装で代替できないか検討する。
Rails Patterns
- ViewHelperにメソッドを実装する時にはまずDecoratorパターンを使うことを検討する。（active_decorator gemを導入しているのでそれを使う）
- 複雑なActiveRecordクエリがあり、再利用できそうな場合はQueryObjectパターンを検討する。（rails-patterns gemを導入しているのでそれのQuery機能を使う）
- Viewにpartialを作る場合はViewComponentを使うことを検討する。
- 複数のActiveRecordモデルを操作する１つの責務がある時や外部APIとやりとりする処理がある場合にはInteractorオブジェクトパターンを検討する。（interactor gemを導入しているのでそれを使う）
- 複数のInteractorを実行するような処理がある場合Organizerオブジェクトパターンを検討する。（interactor gemを導入しており、その中にOrganizerの機能があるので使う）
Files:
test/models/company_test.rb
test/**/*
⚙️ CodeRabbit configuration file
test/**/*: # Test
- TestCase名は英語で書く。
- どうしても避けられない時以外にsystem testでsleepは使わない。
Unit Test
model, helper, decorator, view_componentについてはメソッドを追加した場合は必ず対応したUnit TestのTestCaseを1つは書く。
Files:
test/models/company_test.rb
🧠 Learnings (2)
📚 Learning: 2025-09-08T04:55:46.615Z
Learnt from: sharoa119
PR: fjordllc/bootcamp#8711
File: db/schema.rb:0-0
Timestamp: 2025-09-08T04:55:46.615Z
Learning: In this PR, the removal of length limits from companies.name and companies.website columns in db/schema.rb was unintentional - it was a case of accidentally lost constraints that needed to be restored to their original limits, not a deliberate design change.
Applied to files:
test/models/company_test.rb
📚 Learning: 2025-08-25T07:23:54.802Z
Learnt from: sjabcdefin
PR: fjordllc/bootcamp#9090
File: app/javascript/user_sns.js:0-0
Timestamp: 2025-08-25T07:23:54.802Z
Learning: app/javascript/user_sns.js の会社ロゴ表示において、_list_user.json.builder の構造により user.company.logo_url が存在する場合は必ず user.company.url も存在することが API レスポンス構造で保証されている
Applied to files:
test/models/company_test.rb
🧬 Code graph analysis (1)
test/models/company_test.rb (1)
app/models/company.rb (1)
logo_url(13-21)
⏰ 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
1 participant
Summary by CodeRabbit