Conversation
Walkthrough管理画面のキャンペーン関連ビューで、新規作成ボタンのDOM構造と配置を変更。 Changes
Sequence Diagram(s)（この変更はビュー上のDOM構造・クラス調整であり、コントロールフローや新しい機能は含まれないため、シーケンス図は省略します。） Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
|
@machida 相談したいこと
詳細お試し延長一覧画面 (http://localhost:3000/admin/campaigns) の「+ お試し延長作成」ボタンを 一方で、同じ位置に配置されている「< お試し延長一覧」ボタンについては、
また、faq や companies などキャンペーン以外の画面でも 考え見た目や配置は現状どちらのクラスでも問題なさそうなのですが、配置箇所が このPRでキャンペーン系（index/new/edit）まで統一してしまうのがよいか、それとも index のみで区切って別issueを立てて整理した方がよいかご意見いただけると助かります🙇♀️ |
![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 (3)
app/views/admin/campaigns/index.html.slim (3)
23-26: アイコンのa11yとFAスタイルを微調整しませんか
- アイコンは装飾用途なので aria-hidden を付与推奨。
- Font Awesome の plus は通常 solid系です。現状の fa-regular で描画できるかご確認ください（テーマ依存）。
適用案（FAのスタイルはご利用環境に合わせて調整）:
- i.fa-regular.fa-plus + i.fa-solid.fa-plus(aria-hidden="true")
23-26: is-block によりヘッダ内で横幅を取りすぎる可能性を確認ヘッダのアクション領域では横並びが前提のことが多く、is-block がフル幅になってしまうレイアウト崩れの原因になる場合があります。スクリーンショット上問題なければ維持でOKですが、もし気になるようなら is-block の除去も検討ください。
適用例:
- = link_to new_admin_campaign_path, class: 'a-button is-md is-secondary is-block' do + = link_to new_admin_campaign_path, class: 'a-button is-md is-secondary' do
20-22: 確認結果：.page-main-header-actions は定義済みで、該当ビューは旧クラスを参照していません。
- 定義: app/javascript/stylesheets/application/blocks/page/_page-main-header-actions.sass（app/javascript/stylesheets/application.sass で @import されています）。
- 対象ビュー: app/views/admin/campaigns/index.html.slim は .page-main-header-actions を使用しており、.page-header-actions は含まれていません。
- 注意: リポジトリ内に旧クラス .page-header-actions を参照するテンプレートが多数残っています（例: app/views/notifications/index.html.slim、app/javascript/stylesheets/application/blocks/page/_page-header-actions.sass）。統一が必要なら横断的な置換／古いスタイルの削除を検討してください。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/views/admin/campaigns/index.html.slim(1 hunks)
🔇 Additional comments (1)
app/views/admin/campaigns/index.html.slim (1)
19-26: 配置変更は目的どおりでOKボタンを .page-main-header__end 側へ移動しており、PR目的に合致。リンク先・クラスも維持されています。
|
上記の件は9/17のMTGで町田さんに確認し、このPRではお試し延長ページのコンテンツタブ内のボタンのスタイルを |
- 管理ページヘッダーにあったボタンをお試し期間ページ内のヘッダー(`header.page-main-header`)に移動した - 移動先のクラスに合わせてボタンのCSSを`page-main-header`に変更した
- 他のお試し延長画面(edit/index)に合わせて、`header.page-main-header`内に配置されているボタンに適用するCSSは`page-main-header-actions`に統一する
yokomaru
force-pushed
the
chore/move-campaign-create-button
branch
from
e36e16d to
ca87ad9
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
app/views/admin/campaigns/new.html.slim (3)
20-24: 軽いリファクタ: アクション群の共通ラッパをパーシャル化同一BEM構造を new/index/edit で繰り返すなら、レイアウトパーシャルでラップすると保守が楽です。
該当箇所を以下のように置換（本ファイル内差分）:
- .page-main-header-actions - .page-main-header-actions__items - .page-main-header-actions__item - = link_to admin_campaigns_path, class: 'a-button is-md is-secondary is-block is-back' do - | お試し延長一覧 + = render layout: 'admin/shared/page_main_header_actions' do + .page-main-header-actions__item + = link_to admin_campaigns_path, class: 'a-button is-md is-secondary is-block is-back' do + | お試し延長一覧新規パーシャル（ファイル追加）:
/ app/views/admin/shared/_page_main_header_actions.html.slim .page-main-header-actions .page-main-header-actions__items = yield
23-24: i18n化（任意）文言を翻訳キーに寄せると将来の文言統一が容易です。
差分例:
- | お試し延長一覧 + = t('admin.campaigns.index.back_to_list', default: 'お試し延長一覧')ロケール例:
# config/locales/ja.yml ja: admin: campaigns: index: back_to_list: お試し延長一覧
20-24: UI確認ポイント（レスポンシブ）
.is-blockは狭幅での全幅化を誘発します。ヘッダー右寄せのボタン群で崩れがないか、主要ブレークポイント（<=480px, 768px, 1024px）での折返しと余白を目視確認してください。問題があれば.is-blockの条件付与やコンテナ側のフレックス設定で調整を。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/views/admin/campaigns/index.html.slim(1 hunks)app/views/admin/campaigns/new.html.slim(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/views/admin/campaigns/index.html.slim
⏰ 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
🔇 Additional comments (2)
app/views/admin/campaigns/new.html.slim (2)
20-22: DOMの移設とBEMブロックの統一は妥当（page-main-header-actions へ集約）
.page-main-header__end配下にアクションを置く方針と合致しており、命名も既存パターンと整合しています。
20-24: 確認結果 — CSS定義あり、ビューは統一済み、テスト参照要確認
- CSS: app/javascript/stylesheets/application/blocks/page/_page-main-header-actions.sass に .page-main-header-actions__items と .page-main-header-actions__item の定義あり。
- ビュー: app/views/admin/campaigns/{new.html.slim, edit.html.slim, index.html.slim, _form.html.slim} に .page-main-header-actions / __items / __item が出現（new/edit/index は 20–22 行で確認）。
- 旧クラス残存: app/views/admin/campaigns 配下に 'page-header-actions' の残存は無し。
- テスト: spec ディレクトリが存在しないため、テスト内で旧クラス参照の有無を確認できていない。テストが別ディレクトリ（例: test/）にある場合は同様に検索するか、CI/ローカルでテストを実行して旧セレクタ参照を確認すること。
|
リサさん、お疲れ様です！ |
|
@yokomaru |
|
@yokomaru |
|
@smallmonkeykey 丁寧な確認をありがとうございますっ！ レビュー&Approveありがとうございましたー！ |
|
@komagata |
3 participants
Issue
概要
header.page-headerからheader.page-main-headerに移動するので、ボタンのCSSのをpage-header-actionsからpage-main-header-actionsに変更page-main-header-actionsに統一(編集画面はpage-main-header-actionsが適用されているため非修正対象)変更確認方法
確認前準備
chore/move-campaign-create-buttonをローカルに取り込むforeman start -f Procfile.devでローカルサーバーを起動machidaまたはkomagata/ pass:testtest）でログインするお試し延長作成 ボタン
お試し延長一覧 ボタン
Screenshot
お試し延長作成 ボタン
表示
表示(念の為)
お試し延長一覧 ボタン
表示
表示(念の為)
Summary by CodeRabbit