分報のページはviewのlayoutを分報独自のlayoutファイルを使うようにした #9372
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:
📝 WalkthroughWalkthroughUsers::MicroReportsControllerに Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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 unit tests (beta)
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 |
yokomaru
force-pushed
the
chore/create_micro_report_layout_file
branch
2 times, most recently
from
446bcbe to
afe025a
Compare
|
@machida
ご確認とデザイン等お願いします🙏 |
|
@yokomaru メンションありがとうございます！デザイン入れますー💪 |
|
@machida |
|
@yokomaru すいません！！リマインドありがとうございます🙏 |
|
分報のデザインが遅れてすいません🙇♂️もうすぐSassのCSS化が終わるタイミングなので、それが終わってから着手をする予定です。分報の機能は限られた人しか見れない部分なので、一旦ここまででレビューを進めてマージしてしまおうと思います。その後にデザインを入れるようにします🙇♂️ |
machida
force-pushed
the
chore/create_micro_report_layout_file
branch
from
afe025a to
c66510d
Compare
|
@yokomaru 長くお待たせしてごめんなさい🙇♂️イメージがなんとなくわくくらいまでのざっくりデザインを入れました。一旦これでマージに進めようと思います。 |
|
最新のmainをrebaseしました〜 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.css (3)
102-114: ホバー効果とcursor: defaultの組み合わせについて
.mr-sidebar__userには:hoverでバックグラウンドが変わるスタイルがありますが、cursor: defaultが設定されています。ホバー効果は通常インタラクティブな要素を示唆するため、クリック可能でない場合はユーザーの混乱を招く可能性があります。意図的な設計であれば問題ありませんが、将来的にクリック可能にする予定がある場合は
cursor: pointerに変更することを検討してください。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.css` around lines 102 - 114, `.mr-sidebar__user` currently has a hover background change while `cursor: default` is set, which can mislead users about interactivity; update the rule for `.mr-sidebar__user` to either (A) treat it as interactive by changing `cursor: default` to `cursor: pointer` and ensure any click handler (e.g., the element's click listener) is wired up, or (B) if it is non-interactive, remove the hover background rule `.mr-sidebar__user:hover` (or replace it with a non-interactive visual treatment) so the hover does not imply clickability—apply the chosen change to the `.mr-sidebar__user` selector and its `:hover` rule.
71-87: キーボードナビゲーションのためのフォーカススタイルが不足しています
.mr-sidebar__nav-linkには:hoverスタイルがありますが、:focusや:focus-visibleスタイルがありません。キーボードユーザーのアクセシビリティを確保するため、フォーカス状態のスタイルを追加することを推奨します。♿ フォーカススタイルの追加案
.mr-sidebar__nav-link:hover { background-color: var(--main-tint); color: var(--main); } + +.mr-sidebar__nav-link:focus-visible { + background-color: var(--main-tint); + color: var(--main); + outline: 2px solid var(--main); + outline-offset: -2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.css` around lines 71 - 87, .mr-sidebar__nav-link に :focus および :focus-visible のスタイルが欠けているので、キーボードユーザー向けにフォーカス時の視覚フィードバックを追加してください（既存の :hover を残す）。具体的には .mr-sidebar__nav-link:focus と .mr-sidebar__nav-link:focus-visible を追加して背景色・文字色を :hover と同等にするか、代わりに視認性の高いアウトライン（例: 2px のリングや box-shadow）を適用し、フォーカスリングが色やコントラストの要件を満たすように調整してください；対象は .mr-sidebar__nav-link クラスです。
184-197: モバイルナビゲーションにもフォーカススタイルが必要ですサイドバーと同様に、
.mr-bottom-nav__linkにも:focus-visibleスタイルを追加することを推奨します。♿ フォーカススタイルの追加案
.mr-bottom-nav__link:hover { color: var(--main); } + +.mr-bottom-nav__link:focus-visible { + color: var(--main); + outline: 2px solid var(--main); + outline-offset: -2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.css` around lines 184 - 197, Add an accessible focus style for keyboard users by adding a .mr-bottom-nav__link:focus-visible rule (matching the visual treatment used for the sidebar links) that provides a clear visible indicator (e.g., outline or box-shadow and/or change to var(--main)) while preserving existing transitions; update the .mr-bottom-nav__link selectors so :focus-visible uses the same color/contrast as .mr-bottom-nav__link:hover and includes an accessible focus ring rather than relying on :focus alone.app/assets/stylesheets/application/blocks/micro-report/_micro-reports.css (1)
2-2: マジックナンバー52pxの使用について
calc(100vh - 52px)の52pxが何を表しているのか（例：ヘッダーの高さなど）が明確ではありません。将来のメンテナンス性のため、CSS変数として定義するか、コメントで意図を説明することを検討してください。また、他の箇所では
rem単位が使われているため、単位を統一することでフォントサイズ設定の変更に対する一貫性が保たれます。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/assets/stylesheets/application/blocks/micro-report/_micro-reports.css` at line 2, CSS の宣言 "height: calc(100vh - 52px);" にあるマジックナンバー 52px を説明または変数化してください; 具体的には :root または該当コンポーネントで --header-height（例）を定義し、該当箇所の高さを "height: calc(100vh - var(--header-height));" に置き換えるか、プロジェクトで使われている単位（rem）に合わせて rem に変換してから CSS 変数として定義し、さらに意図を示す短いコメントを追加してください（参照箇所は "height: calc(100vh - 52px);" の行）。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@app/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.css`:
- Around line 102-114: `.mr-sidebar__user` currently has a hover background
change while `cursor: default` is set, which can mislead users about
interactivity; update the rule for `.mr-sidebar__user` to either (A) treat it as
interactive by changing `cursor: default` to `cursor: pointer` and ensure any
click handler (e.g., the element's click listener) is wired up, or (B) if it is
non-interactive, remove the hover background rule `.mr-sidebar__user:hover` (or
replace it with a non-interactive visual treatment) so the hover does not imply
clickability—apply the chosen change to the `.mr-sidebar__user` selector and its
`:hover` rule.
- Around line 71-87: .mr-sidebar__nav-link に :focus および :focus-visible
のスタイルが欠けているので、キーボードユーザー向けにフォーカス時の視覚フィードバックを追加してください（既存の :hover を残す）。具体的には
.mr-sidebar__nav-link:focus と .mr-sidebar__nav-link:focus-visible を追加して背景色・文字色を
:hover と同等にするか、代わりに視認性の高いアウトライン（例: 2px のリングや
box-shadow）を適用し、フォーカスリングが色やコントラストの要件を満たすように調整してください；対象は .mr-sidebar__nav-link
クラスです。
- Around line 184-197: Add an accessible focus style for keyboard users by
adding a .mr-bottom-nav__link:focus-visible rule (matching the visual treatment
used for the sidebar links) that provides a clear visible indicator (e.g.,
outline or box-shadow and/or change to var(--main)) while preserving existing
transitions; update the .mr-bottom-nav__link selectors so :focus-visible uses
the same color/contrast as .mr-bottom-nav__link:hover and includes an accessible
focus ring rather than relying on :focus alone.
In `@app/assets/stylesheets/application/blocks/micro-report/_micro-reports.css`:
- Line 2: CSS の宣言 "height: calc(100vh - 52px);" にあるマジックナンバー 52px
を説明または変数化してください; 具体的には :root または該当コンポーネントで --header-height（例）を定義し、該当箇所の高さを
"height: calc(100vh - var(--header-height));" に置き換えるか、プロジェクトで使われている単位（rem）に合わせて
rem に変換してから CSS 変数として定義し、さらに意図を示す短いコメントを追加してください（参照箇所は "height: calc(100vh -
52px);" の行）。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 16031542-f503-4a38-a372-fe2d96a052a1
📒 Files selected for processing (5)
app/assets/stylesheets/application.cssapp/assets/stylesheets/application/blocks/micro-report/_micro-report-form-tabs.cssapp/assets/stylesheets/application/blocks/micro-report/_micro-report-form.cssapp/assets/stylesheets/application/blocks/micro-report/_micro-report-layout.cssapp/assets/stylesheets/application/blocks/micro-report/_micro-reports.css
💤 Files with no reviewable changes (2)
- app/assets/stylesheets/application/blocks/micro-report/_micro-report-form-tabs.css
- app/assets/stylesheets/application/blocks/micro-report/_micro-report-form.css
|
お疲れ様です、デザイン対応諸々ありがとうございます！(めちゃくちゃ分報っぽいデザインになっていて感動しました！すごい。。！👀) レイアウト変更によって落ちているテストを修正しているのですが、仕様について2つ確認させてください！ 自分の分報へ飛ぶリンクについて旧リンク(タブのヘッダー、現在の件数を表示)
新リンク(サイドバー)
確認事項
|
|
@yokomaru 質問ありがとうございます！！
すいません、僕のミスです🙇♂️
分報へのリンクの仕方も変更しようと思っています。 |
yokomaru
force-pushed
the
chore/create_micro_report_layout_file
branch
from
398e285 to
9c32c71
Compare
|
お疲れ様です！ |
|
@yokomaru |
|
色的に「内容修正」と「削除」のボタンがdisabledになっているのかと思いました。 |
|
今回の修正とは関係ないのですが、感じたことを置いておきます キー押下で投稿ができたら便利だな～と思いました。（個人的には |
|
リアクションボタンを押したときに出現するメニューが、下のリアクションボタンと被って押しにくいな、と思いました。メニューを最前面にした方が良いかな～と思います。 |
| = link_to root_path, class: 'mr-sidebar__nav-link' do | ||
| i.fa-solid.fa-house | ||
| span.mr-sidebar__nav-label ホーム | ||
| - if current_user |
There was a problem hiding this comment.
if current_userは必要無いように思えたのですがどうでしょうか？
ここの処理は「ログインしているか、していないか」をチェックして、ログイン状態の時のみ「自分の分報」と「通知」という項目を表示するものだと解釈しています。
しかし分報ページにはそもそも未ログインのユーザーはアクセスできない（私の知る限り）ので必要ないかな、と思いました。
この条件があることで「未ログインのユーザーもアクセスできるページだ」という勘違いを生みそうだな、と思ったので必要無ければ消した方が良いと思いました。
（別の目的や自分の解釈の間違いがあれば教えていただきたいです🙏🙏）
There was a problem hiding this comment.
ご確認いただきありがとうございます！ここの分岐を見落としておりました🙏
以下確認し、分岐が不要なことを確認いたしました！
- 分報ページにはそもそも未ログインのユーザーはアクセスできない
- ログイン中にcurrent_userが空になることはないのでここでの分岐や以降の処理でも不要(エラーにならないことを確認)
@machida
こちらmachidaさんの方で追加していただいたと思うのですが、不要であればこちら も併せて削除で問題ないでしょうか？(他に意図や目的がある分岐かどうか念の為確認できたら嬉しいです)
また、このif current_userの分岐を全て削除して動作確認してみたところ、
今まではslimのインデントのズレなどで見えていなかったサイドバーのフッターが表示されたのですが
こちらはマイページへのリンクなどが必要でしょうか？(現状はカーソルを当てると色が変わりますがリンクなどはないためクリックしても何も起こらない状態になります)
2026-03-12.15.05.39.mov
お手数おかけいたしますが、ご確認お願いいたします。
There was a problem hiding this comment.
@yokomaru すいません！！メッセージを見逃してました🙇♂️
不要です！！なんとなくの雰囲気だけを雑に作ってた段階なので、細かい意図はありませんでした🙇♂️
不要な部分はガンガン削除してしまって大丈夫ですー🙏
マイページへのリンクもなくて大丈夫です。ダッシュボードへのリンクは作ろうと思いますが、それもおいおいでやっていこうと思ってます。
There was a problem hiding this comment.
ありがとうございます！
こちらで対応いたしました。
| = link_to notifications_path, class: 'mr-sidebar__nav-link' do | ||
| i.fa-solid.fa-bell | ||
| span.mr-sidebar__nav-label 通知 | ||
| - if current_user |
There was a problem hiding this comment.
|
@yokomaru デザインや機能への以下のコメントは今PRの対応範囲を超えているな、と感じるので、要修正の場合は別Issueを立て対応がした良いかなというのが個人的な感想です。そのあたりはmachidaさんと相談していただきたいです🙇♂️🙇♂️🙇♂️ |
|
お疲れ様です。分報のレイアウトのデザインの部分につきまして、 @s-tone-gs さんにコメントいただいた箇所がいくつかあり、一旦優先度などの自分なりに整理してみました！ ステージングの分報画面でも再現したので以前のレイアウトでも起きていたようです。こちらはすぐに修正いただけそうであればこのPRで修正をお願いしたいですが可能でしょうか🙏 
2026-03-12.15.31.38.mov※ 参考までに誰もリアクションしてない時のリアクションした人を見るボタンのdisable時のデザインは以下なので、似ているようにも感じるな〜とも思いました。
|
|
@yokomaru 連絡ありがとうございます！！ 3は別Issueの作成をお願いできると助かります🙇♂️ |
|
@machida @s-tone-gs |
|
対応しました〜 
|
|
@machida また、お手隙で以下もご確認お願い致します🙏 |
|
@yokomaru 確認ありがとうございますー🙏 |
|
@yokomaru |
- 分報一覧画面への遷移は分報タブではなくサイドバーの自分の分報リンクから飛ぶ - 自分の分報リンクには件数は不要のため件数関連のテストを削除
yokomaru
force-pushed
the
chore/create_micro_report_layout_file
branch
from
232b477 to
89c2a76
Compare
|
@s-tone-gs |
🚀 Review AppURL: https://bootcamp-pr-9372-fvlfu45apq-an.a.run.app
|
|
@s-tone-gs @okuramasafumi |
|
@okuramasafumi @komagata |
5 participants
Issue
概要
/users/:user_id/micro_reportsを開いた場合、独自の layout ファイルが適用されるようにした変更確認方法(レイアウトの変更だが、念の為機能を一通り確認)
chore/create_micro_report_layout_fileをローカルに取り込むbin/setupでサーバーを起動hatsunoでログイン分報の投稿はまだありません。と表示されるhttp://localhost:3000/users/655153192/micro_reports?page=1#latest-micro-reportに飛ぶ@kimuraと書き込む)をした後、その投稿を削除する自分の分報をクリック?page=2#latest-micro-reportになっていること)Screenshot
分報一覧
変更前
変更後
該当の分報がない時の表示
変更前
変更後
Summary by CodeRabbit