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.

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

[FE] モーダル UX 改善 #442

Merged
merged 7 commits into from
Nov 27, 2024
Merged

[FE] モーダル UX 改善 #442

merged 7 commits into from
Nov 27, 2024

Conversation

popunbom
Copy link
Contributor

@popunbom popunbom commented Nov 27, 2024

relates: #241

トップの配車ボタン押下時と到着画面で評価を押したあとにAPI完了を待たずに、すぐ画面を閉じる処理に変更

上記の対応を実施しました。

加えてモーダルクローズ時のアニメーション関連処理を修正しました。

トップの配車ボタン押下時に、API完了を待たずにモーダルをすぐ表示する

improve-modal-01.mov

到着画面で評価を押した後に、API完了を待たずにモーダルをすぐ閉じる

improve-modal-02.mov

@popunbom popunbom force-pushed the feat/frontend/improve-modal-ux branch from 8f78117 to da27db2 Compare November 27, 2024 01:01
@popunbom
Copy link
Contributor Author

@imamiya-masaki @narirou レビューお願いいたします。

selectorModalRef.current.close();
}
}, []);
// internalState: AppRequestContext.status をもとに決定する内部 status
Copy link
Contributor

Choose a reason for hiding this comment

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

コメントは書かなくて大丈夫です! 変数名を明示的にしたり、コードをシンプルにすることで担保してもらえたらと
それでもわからないWhyがあるときだけ記載をお願いします 🙇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@narirou 不要なコメントを削除しました (8a39055)。

return;
}
setInternalStatus("MATCHING");
await fetchAppPostRides({
Copy link
Contributor

Choose a reason for hiding this comment

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

try-catchにしてエラーハンドリングしてもらえると助かります!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@narirou エラーを catch するようにしました (9ea52bf)。

});
}, [currentLocation, destLocation]);
// 少し時間をおいた後、Modal 要素を DOM から外す
setTimeout(() => setStatusModalOpen(false), 300);
Copy link
Contributor

Choose a reason for hiding this comment

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

(IMO) animationendイベントがあるのでこちらを見ても良いです
https://developer.mozilla.org/en-US/docs/Web/API/Element/animationend_event

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@narirou こちら修正しました (4d25cae)。
Modal の実装をちゃんと理解できてませんでした。改めて読み直すと setTimeout() もイベントハンドラも不要そうでした。

  • ref.current.close() でクローズアニメーションがトリガー
  • Modal 内部でアニメーション完了時に props.onClose() を呼び出す
    • なので、Modal の onClose に Modal 自体のレンダリングを制御するフラグを更新する処理を入れれば良い

Copy link
Contributor

Choose a reason for hiding this comment

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

なるほど、ありがとうございます!

Copy link
Contributor

@narirou narirou left a comment

Choose a reason for hiding this comment

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

LGTM

@popunbom popunbom merged commit 184eb76 into main Nov 27, 2024
2 checks passed
@popunbom popunbom deleted the feat/frontend/improve-modal-ux branch November 27, 2024 02:44
@popunbom popunbom mentioned this pull request Nov 27, 2024
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants