スマホでも他人の進捗を消す機能が使えるようにした。スマホでコメントにユーザーロールが表示されないバグも修正#9121
Conversation
WalkthroughSassでスイッチとカードアクションのブレークポイント/レイアウトを調整。コメントと回答のアバター画像を役割用のラッパー要素で包むマークアップ変更。プラクティス一覧ヘッダーの一部アクションの小画面での非表示制限を解除。 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
app/javascript/stylesheets/atoms/_a-switch.sass (3)
2-5: transition: all は対象を絞り、低モーション対応を入れてください不要なプロパティまでアニメーションしないようにし、prefers-reduced-motion を考慮しましょう。
- transition: all .2s ease-out + transition: color .2s ease-out, background-color .2s ease-out, transform .2s ease-out + @media (prefers-reduced-motion: reduce) + transition: none
6-7: 子孫セレクタ「*」での cursor: pointer は過剰適用無効化された要素やアクセシビリティ補助に影響する可能性。対象を限定しましょう。
- * - cursor: pointer + label, input[type='checkbox'], .a-help, .a-switch__label-text + cursor: pointer
19-24: ホバー時の装飾はキーボード操作でも反映させる:focus-visible / :focus-within でも同等の見た目にしてください。
&:hover .a-switch__label-text color: var(--main-text) text-decoration: underline .a-help background-color: var(--main) + &:focus-within, &:focus-visible + .a-switch__label-text + color: var(--main-text) + text-decoration: underline + .a-help + background-color: var(--main)app/views/courses/practices/index.html.slim (1)
27-32: モーダル起動に label[for] を使うのは非推奨(非フォーム要素)ボタンに置き換えるとセマンティクスとアクセシビリティが改善します。
- label.a-switch__label(for='modal-progress') + button.a-switch__label type='button' data-action='click->modal#open' data-target='modal-progress'app/views/comments/_comment.html.slim (2)
16-17: ロール用クラス付与を answer 側と統一(helper 経由)answer は user_icon_frame_class を使っています。comment も同一ヘルパを使うと分岐・nil対策が一元化できます。
- span class="a-user-role is-#{comment.user.primary_role}" + span class="#{comment.user.user_icon_frame_class}"
6-6: アバター画像に alt を追加(装飾なら空、代替が必要ならユーザー名)SR 対応のため alt を明示してください。
- img.thread-comment__user-icon.a-user-icon src="#{comment.user.avatar_url}" + img.thread-comment__user-icon.a-user-icon src="#{comment.user.avatar_url}" alt="#{comment.user.login_name}のアバター"(装飾目的なら alt="")
Also applies to: 17-17
app/views/questions/_answer.html.slim (1)
5-5: アバター画像の alt を追加コメント側と同様に alt を付けてください。
- img.thread-comment__user-icon.a-user-icon src="#{answer.user.avatar_url}" + img.thread-comment__user-icon.a-user-icon src="#{answer.user.avatar_url}" alt="#{answer.user.login_name}のアバター"(装飾目的なら alt="")
Also applies to: 22-22
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
app/javascript/stylesheets/atoms/_a-switch.sass(1 hunks)app/javascript/stylesheets/shared/blocks/card/_card-main-actions.sass(0 hunks)app/views/comments/_comment.html.slim(1 hunks)app/views/courses/practices/index.html.slim(1 hunks)app/views/questions/_answer.html.slim(2 hunks)
💤 Files with no reviewable changes (1)
- app/javascript/stylesheets/shared/blocks/card/_card-main-actions.sass
⏰ 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 (5)
app/views/courses/practices/index.html.slim (1)
25-35: スマホで「みんなの進捗」トグルを表示する変更は妥当要件達成のための最小変更で副作用リスクも低いです。
app/views/comments/_comment.html.slim (2)
16-17: モバイルでの役割表示(ラッパー追加)は良い修正ロール別スタイル適用のための構造が揃いました。
27-29:.adviserと.adviser?の混在は問題ありません
Rails の boolean カラムではfoo?が同義エイリアスとして自動定義されるため、user.adviserとuser.adviser?に未定義メソッドや真偽のずれは発生しません (karolgalanciak.com, stackoverflow.com)app/views/questions/_answer.html.slim (2)
4-4: user_icon_frame_class への置換は適切クラス値の引用も統一され、意図が明確です。
Also applies to: 21-22
29-31:adviser?メソッドの存在と意図を確認
現状answer.user.adviser(association取得)が使われています。boolean判定としてadviser?を呼ぶ意図がある場合は、Userモデルにadviser?が定義されているか・boolean属性かを確認し、必要に応じて呼び出しを切り替えてください。
#9118
#9119
Summary by CodeRabbit