88-99
88-99
@machida デザインをお願いいたします🙏
@machida > デザイン入れましたー ありがとうございます!
@reckyy お疲れ様です。 お手隙の際で結構ですので、こちらのレビューをお願いできませんでしょうか?🙏
ありがとうございます!
@reckyy ご確認いただきまして、ありがとうございます。 > a-placeholderの適用を止めるとなくなりました。こちらの実装が意図されたものかどうかお伺いしたいです。 こちらの実装というのは、メモの内容が点滅する動作のことで合っていますでしょうか?念の為。 もしそうでしたら、@machida さんにはお聞きしなかったのですが、私の見解ですと意図されたものであると判断しました。 - 管理者として企業ページを開くと管理者向けメモが点滅しますので、デザインの仕上がり確認時などにも目にとまる - 管理者向けメモだということを目立たせる仕様にしている のではないかと考えました。 デザインしていただいたときに、おもしろいと思ったので一応私もコードを確認いたしました。 以下のファイルの9行目で町田さんがanimationを定義されています。 https://github.com/fjordllc/bootcamp/blame/main/app/javascript/stylesheets/atoms/_a-placeholder.sass 【参考】 https://developer.mozilla.org/ja/docs/Web/CSS/animation PRでは、管理者向けメモが点滅することに触れていませんでしたので、GIFを追加しました🙏 いかがでしょうか?
@machida > a-placeholder は読み込み待ちの時に表示するもので、読み込みが完了した後は消える必要があります。もしそのように使われていない場合は、dawaさんが a-placeholder の意図や存在理由を勘違いしている可能性があります。 勘違いしてました。こちらお手隙の時に修正していただけませんでしょうか🙏
@reckyy > おっしゃる通りです。曖昧な言及で申し訳ございません。 一応確認させていただいただけですので。 仕様に関して確認が大事でした😓ご質問くださり、ありがとうございました🙇♂️
@machida かしこまりました!ご確認いただき、ありがとうございます🙇♂
@machida a-placeholder を削除しました。 https://github.com/fjordllc/bootcamp/pull/7401/commits/8f5d6320fe58e9f99f144be2d371f8e7b6f2e0ae @reckyy お手数ですが、レビューお願いいたします🙏
ご確認いただき、ありがとうございます! コメント:https://github.com/fjordllc/bootcamp/pull/7401#pullrequestreview-1965264220 > 5394a65 は「i18n追加」などに改めた方がいいかもと思いました。このコミットで、メモを実装したと勘違いするかもしれないので。 そうですね。修正します。 > こちらはdawaさんが実装したわけではないですが、管理者として情報変更した後に企業一覧に遷移するのはUI的に微妙かな〜と感じました。 メモなら尚更、メモ記入後も企業の情報を開いたままのほうがいいはずです。このPRで変更するのもありかなと思いましたが。。(もしくは別Issue?good first issueにはよさそう。) dawaさんのご意見をお伺いしたいです。 ご意見いただき、ありがとうございます。 私としても、一覧画面に行くのは少し違和感がありました。更新後に企業の詳細(個別)ページに遷移する方が、更新内容が確認しやすい気はしています。同じようなケースで、日報の内容を更新するとどうなるかを確認したところ、詳細ページに遷移しました。 - 一覧画面に遷移した場合は、同じ企業の個別ページをまた開きたいと思った時、探して選択する必要がある - 企業の個別ページに遷移した場合は、一覧画面のボタンを押せば一覧画面に行ける 駒形さんにご意見をお伺いしたいです。 @komagata 上記についてご意見を伺いたいです。 1. 企業情報更新後の遷移先を、一覧画面と詳細画面のどちらが良いのか。 2. もし詳細画面に遷移するよう変更する場合、別Issueを立てた方がよろしいでしょうか。 お願いいたします。