提出物タブでもマイプロフィール確認と日報作成がしたい #5760
Conversation
|
@OdenTakashi |
|
@ksmxxxxxx 月曜日以降に確認させていただいてもよろしいでしょうか。？🙇♂️ |
|
@OdenTakashi 大丈夫ですよ〜 🙆🏻 |
|
@ksmxxxxxx ただいま確認させていただきました！ |
|
@OdenTakashi コメントありがとうございます〜
|
| @@ -5,6 +5,17 @@ header.page-header | |||
| .page-header__inner | |||
| h2.page-header__title | |||
| | ダッシュボード | |||
| .page-header-actions | |||
| .page-header-actions__items | |||
There was a problem hiding this comment.
9行目の.page-header-actions__itemsにはulタグを付与するのが正しいみたいです！
[192] スクラム#3 | FBCこちらの日報でmachidaさんが仰っていました！
There was a problem hiding this comment.
@OdenTakashi
レビューありがとうございます。
こちら確かに町田さんのおっしゃるとおりで、ulとliでマークアップするのが良さそうというのは同意です。
更に言うとナビゲーションに該当すると思うので、.page-header-actionsにnavでマークアップするのが良いのでは？という気になりポイントがあります。
ただ、対応することにしたときに、一点気になったことがあります。
ここ以外にも同様のナビゲーションがダッシュボードのトップや日報タブにも実装されています。同時進行で出している #5758 にもあります。
もしここを ul,liにするのだとしたら、ここだけマークアップされているのも不揃になるので合わせてマークアップを修正するのが妥当だと考えますが、今回のIssueの範囲からはみ出てしまうので、このPRで対応するべきか？という疑問が出てきます。
自分の経験則だと、基本「Issueからはみ出る範囲のものは別Issueを作成してそちらで対応する」というのが筋の良いスプリントの進め方かなぁと考えています。（コミットもまとめられるので履歴もきれいになりそう）
提案としては、
- このPRではあくまで実装するにとどめて、マークアップの修正については別のIssueで対応することを開発MTGでチームリーダー（この場合komagataさんになると思います）に提案する
- このPRだけはマークアップの修正を行って、他のタブの同様のナビゲーションは別Issueを作成してそこで対応する
- どうするのがいいのかwakaranので、水曜日の開発MTGで共有してPO（町田さんになるのかな）に判断を仰ぐ
の3つになります。
OdenTakashiさんはどうおもいますか？
自分は1がいいかなぁって考えてます（多分、現場でもそう判断すると思います）。
There was a problem hiding this comment.
@ksmxxxxxx
ご連絡遅れてしまい申し訳ありません🙏
なるほどです！確かに今回のみマークアップの修正を行ってしまうと不揃いになってしまいますね💦
machidaさんもマークアップができていない箇所を見つけ次第issueを立て欲しいとおっしゃっていたので、マークアップの修正については別issueを立てて対応するという方が良いのかなと感じたので、私も１がいいかなと考えました！
このような視点を考えずコメントをしてしまっていました💦
申し訳ありません🙇♂️
There was a problem hiding this comment.
@OdenTakashi こちら開発MTGでmachidaさんに確認させてもらいました。方針については別途まとめてコメントに残しておきたいと思います〜
| @@ -5,6 +5,17 @@ header.page-header | |||
| .page-header__inner | |||
| h2.page-header__title | |||
| | ダッシュボード | |||
| .page-header-actions | |||
| .page-header-actions__items | |||
| .page-header-actions__item | |||
There was a problem hiding this comment.
10行目のpage-header-actions__itemにはliタグを付与するのが正しいみたいです！
| i.fa-solid.fa-user | ||
| | マイプロフィール | ||
| - unless current_user.adviser? | ||
| .page-header-actions__item |
There was a problem hiding this comment.
15行目のpage-header-actions__itemにはliタグを付与するのが正しいみたいです！
|
@ksmxxxxxx 大変申し訳ありません！ ご迷惑おかけして申し訳ありません🙇♂️ |
|
@OdenTakashi 改めましてレビューありがとうございました。 他に気になるポイントなどあればまたコメントいただければと思います！ よろしくおねがいします〜 🙏🏻 |
|
@ksmxxxxxx good first issueのためにわざと外していたのですね 🤔 私からはApproveとさせていただきます！ |
|
@komagata おつかれさまです。 |
| .page-header-actions | ||
| .page-header-actions__items | ||
| .page-header-actions__item | ||
| = link_to user_path(id: current_user.id), class: 'a-button is-md is-secondary is-block' do |
There was a problem hiding this comment.
| = link_to user_path(id: current_user.id), class: 'a-button is-md is-secondary is-block' do | |
| = link_to current_user, class: 'a-button is-md is-secondary is-block' do |
こちらでいいかもです〜
Issue
概要
ダッシュボードの提出物でも「マイプロフィール」と「日報作成」のリンクを、Watch中タブでも表示する
変更点確認方法
feature/add_daily_note_button_on_products_viewブランチをローカルに取り込むbin/rails sでローカルサーバーを起動変更前
変更後
kimura
アドバイザー