Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

提出物の新規作成/編集画面にキャンセルボタンを追加 #7864

Merged
merged 5 commits into from
Jul 2, 2024
Merged

提出物の新規作成/編集画面にキャンセルボタンを追加 #7864

merged 5 commits into from
Jul 2, 2024

Conversation

motohiro-mm
Copy link
Contributor

@motohiro-mm motohiro-mm commented Jun 17, 2024 •
edited

Issue

概要

提出物の新規作成・編集画面にキャンセルボタンを追加しました。

  • 修正前：キャンセルボタンなし
  • 修正後：キャンセルボタンあり
    • 新規作成：元のプラクティス詳細画面に遷移
    • 編集：編集前の提出物の詳細画面に遷移

変更確認方法

  1. feature/add-cancel-button-to-product-edit-pageをローカルに取り込む
  2. foreman start -f Procfile.devでサーバーを立ち上げる
  3. hatsunoでログイン
    （提出物が未作成のプラクティス、既に作成済みのプラクティスの両方があるユーザーなら誰でもOKですが、以下リンクはhatsunoでログインした場合のみ有効です）
  4. 新規作成画面のキャンセルボタンを確認する
  1. 編集画面のキャンセルボタンを確認する

Screenshot

変更前

スクリーンショット 2024-06-17 16 22 00

変更後

  • 新規作成画面
2024-06-17.18.03.39.mov
  • 編集画面
2024-06-17.18.06.31.mov

@motohiro-mm
Copy link
Contributor Author

@machida

お疲れ様です！
デザインのことで確認していただきたいところがあります。

app/views/products/_form.html.slimにキャンセルボタンのコードを追加すると、キャンセルボタンが下記画像の位置に表示されました。
他の既にあるキャンセルボタン（日報、イベント）と位置が異なるため、ul.form-actions__items.is-ais-flex-startからクラス名is-ais-flex-startを削除し、既にあるキャンセルボタンと同じ位置に表示されるようにしました。

提出物のキャンセルボタン位置修正前
スクリーンショット 2024-06-17 18 16 03

他の既にあるキャンセルボタン（日報）
スクリーンショット 2024-06-17 18 15 10

確認していただきたいことは2点です。

  • is-ais-flex-startを削除して問題ないか
    他に影響ないことを確認して削除しましたが、確証を得られなかったので確認していただきたいです。

  • app/javascript/stylesheets/application/blocks/form/_form-actions.sassはそのままで良いのか
    is-ais-flex-startが他に使われている箇所がなさそうだったので、.form-actions__itemsの中のis-ais-flex-startは必要ないのかなと考えました。そちらも確認していただきたいです。

また、デザイン確認の依頼の仕方の間違い（PRではなくissueで、assigneeではなくreviewerで、など）がありましたらそちらもご指摘いただければと思います🙇‍♀️

お忙しいところ恐縮ですが、よろしくお願いします！🙏

@machida
Copy link
Member

machida commented Jun 17, 2024

@motohiro-mm
報告ありがとうございます！！
キャンセルリンクのいち調整の対応方法バッチリです:+1:
is-ais-flex-start は不要なので、削除しておきましたー。

@motohiro-mm motohiro-mm marked this pull request as ready for review June 18, 2024 04:33
@motohiro-mm motohiro-mm requested a review from reckyy June 18, 2024 04:33
@motohiro-mm
Copy link
Contributor Author

motohiro-mm commented Jun 18, 2024 •
edited

@machida
ご対応いただきありがとうございました！🙏✨
今後もデザインに関するところはmachidaさんに確認させていただきますので、よろしくお願いいたします！

@reckyy
お疲れさまです！
お手隙の際にこちらのPRのレビューをお願いできますでしょうか🙏
ご都合がつかなければ遠慮なく仰ってください🙇‍♀️
よろしくお願いいたします！

@machida machida removed their assignment Jun 18, 2024
@reckyy
Copy link
Contributor

reckyy commented Jun 18, 2024

@motohiro-mm
承知しました！
一週間以内にはレビューさせて頂きます。
よろしくお願いいたします。

reckyy
reckyy approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@reckyy reckyy left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@motohiro-mm
お待たせしました〜。
動作、コード共に確認させていただきました。
自分からはOKです🙆

@motohiro-mm
Copy link
Contributor Author

@reckyy
おつかれさまです！
お忙しいところご対応いただきありがとうございました！🙌

@motohiro-mm
Copy link
Contributor Author

@komagata
おつかれさまです！
チームメンバーからApproveされましたので、お手隙の際にレビューをお願いいたします🙏

komagata
komagata reviewed Jun 24, 2024
View reviewed changes
app/views/products/_form.html.slim Outdated
Comment on lines 44 to 46
= link_to 'キャンセル', practice_path(params[:practice_id]), class: 'a-button is-sm is-text'
- when 'edit', 'update'
= link_to 'キャンセル', product_path, class: 'a-button is-sm is-text'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コードの重複が多くなってしまうので、違うURL部分だけを分岐させるのがいいと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case文での条件分岐をpath部分だけにして、path部分以外をまとめる形に修正しました！

@motohiro-mm motohiro-mm force-pushed the feature/add-cancel-button-to-product-edit-page branch from 6945f2c to bbed6a6 Compare June 25, 2024 05:30
motohiro-mm
motohiro-mm commented Jun 25, 2024
View reviewed changes
Copy link
Contributor Author

@motohiro-mm motohiro-mm left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@komagata
レビューありがとうございました！
条件分岐内の重複を修正しました。
確認をお願いいたします。

app/views/products/_form.html.slim Outdated
Comment on lines 44 to 46
= link_to 'キャンセル', practice_path(params[:practice_id]), class: 'a-button is-sm is-text'
- when 'edit', 'update'
= link_to 'キャンセル', product_path, class: 'a-button is-sm is-text'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case文での条件分岐をpath部分だけにして、path部分以外をまとめる形に修正しました！

@motohiro-mm motohiro-mm force-pushed the feature/add-cancel-button-to-product-edit-page branch from bbed6a6 to d5b2f00 Compare June 29, 2024 09:04
komagata
komagata approved these changes Jul 2, 2024
View reviewed changes
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確認させていただきました。OKです～👌

@komagata komagata merged commit 53d69ed into main Jul 2, 2024
2 checks passed
@komagata komagata deleted the feature/add-cancel-button-to-product-edit-page branch July 2, 2024 23:37
@github-actions github-actions bot mentioned this pull request Jul 2, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@komagata komagata komagata approved these changes

@reckyy reckyy reckyy approved these changes

Assignees

@motohiro-mm motohiro-mm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@motohiro-mm @machida @reckyy @komagata