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

formが書き途中でページ遷移を行おうとしたときに確認メッセージを出す #903

Merged
merged 7 commits into from
Oct 27, 2023

Conversation

mathsuky
Copy link
Contributor

issue451について,とりあえず進捗部屋登録のフォームの部分で,もしフォームに何か書かれている状態でページ遷移をしようとしたときに確認メッセージを出す機能を実装してみました。
方向性がこれで良さそうであれば,他のフォームも含めて実装していこうと思いますが,どうでしょうか。

@mathsuky mathsuky changed the title ページ遷移時に書き途中のformがある場合に確認メッセージを出す 書き途中のformがある状態でページ遷移を行おうとしたときに確認メッセージを出す Sep 25, 2023
@mathsuky mathsuky changed the title 書き途中のformがある状態でページ遷移を行おうとしたときに確認メッセージを出す formが書き途中でページ遷移を行おうとしたときに確認メッセージを出す Sep 25, 2023
@itt828
Copy link
Member

itt828 commented Sep 29, 2023

手元で検証したのですが、動いている様子が確認できませんでした:shiku2uzaki:
入力中に

  • ブラウザバックしたとき
  • 別のページに遷移したとき
  • モーダルを閉じたとき

に対応するように実装してみてください:pray:

@mathsuky
Copy link
Contributor Author

申し訳ありません。できるだけ早く確認いたします🙏

@mathsuky
Copy link
Contributor Author

上二つに対応して満足して,最後のを実装できていませんでした。本当にすみません🙇
メッセージを表示するために上二つと下で使っている機能がちがうのですが,それは問題になりますでしょうか。統一した方がいいということなら,下のほうに合わせる方向でいこうと思うのですが,どうでしょうか。

@mathsuky
Copy link
Contributor Author

mathsuky commented Oct 2, 2023

すみません,上のやつが正しく動作しない時があるので,修正したものをあげなおします🙏

@mathsuky
Copy link
Contributor Author

mathsuky commented Oct 2, 2023

フォームに何も書かれていない場合の処理が抜けていました。確認不足でした。申し訳ないです。

Copy link
Member

@itt828 itt828 left a comment

Choose a reason for hiding this comment

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

返信遅くなってしまって申し訳ないです:bow:

イベント新規作成フォームや編集フォームについても同様に実装をはじめてみてほしいです

Comment on lines 78 to 87
mounted: function () {
window.onbeforeunload = () => {
if (this.inputData) {
return 'このページを離れると保存されていないデータは破棄されますが,よろしいですか。'
}
}
},
destroyed() {
window.onbeforeunload = null
},
Copy link
Member

Choose a reason for hiding this comment

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

少なくとも自分の手元では利用中にこの表示を見つけることができなかったので、ここの部分がどのような操作をした際にどのような表示をするのか教えてほしいです:pray:

Copy link
Contributor Author

@mathsuky mathsuky Oct 5, 2023

Choose a reason for hiding this comment

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

スクリーンショット 2023-10-05 15 46 54

この部分については,onbeforeunloadの仕様で,特定のブラウザでのみこのreturnの中身が表示され,それ以外ではブラウザのデフォルトの内容が表示されるそうです。returnの中身が空だとエラーが生じることがあるという記述も見かけたので,一応適当な文章を入れてあります。ただその問題は自分の手元では再現されていません。
説明が不十分で申し訳ありませんでした🙇

Copy link
Contributor Author

@mathsuky mathsuky Oct 5, 2023

Choose a reason for hiding this comment

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

ところで,このonbeforeunloadはiOS上のブラウザでのみうまく動作しないそうです。代替の機能として紹介されていたものがうまく動かなかったのでこれを用いているのですが,やはりこれを使うのは問題だろうと思いましたので,代替のものをうまく使えないか試してみようと思います。(動作は上の写真と似た感じになると思います。)

Copy link
Member

Choose a reason for hiding this comment

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

:naruhodo_UD:

Copy link
Member

Choose a reason for hiding this comment

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

ちなみにtraQではこんな感じ(大体おなじ)で書いてるみたいですね
https://github.com/traPtitech/traQ_S-UI/blob/01a24c96d1cf168736a21be5fc92da4e43bb0f04/src/views/MainPage.vue#L76

Copy link
Contributor Author

@mathsuky mathsuky Oct 7, 2023

Choose a reason for hiding this comment

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

ありがとうございます!
traQでは普通にbeforeunloadで書いているんですね。それならばあえてそれを使うのを避ける必要はないのかもしれません。考えてみます。

@mathsuky
Copy link
Contributor Author

遷移時警告を進捗部屋登録,イベント作成,グループ作成に追加しました。確認よろしくお願いいたします🙏

@itt828
Copy link
Member

itt828 commented Oct 16, 2023

ありがとうございます!
実際に試してみて気になった点は以下です

  • 破棄してもよい方に進むとポップアップが繰り返し表示される
  • 何も入力していない時からポップアップが表示される

@itt828
Copy link
Member

itt828 commented Oct 16, 2023

submitするときにもポップアップが出てしまっていそう

@mathsuky
Copy link
Contributor Author

submitした時にポップアップが表示される問題を修正しました。追加でイベントとグループの編集で警告が出ない問題を修正しました。ご確認よろしくお願いします🙇

@itt828
Copy link
Member

itt828 commented Oct 26, 2023

遅くなりました:bow:
よさそうなのでマージします!
一点だけ

@itt828 itt828 marked this pull request as ready for review October 26, 2023 11:34
Comment on lines 234 to 254
beforLeaveGuardinEventEdit = (to, from, next) => {
if (from.name === 'EventEdit') {
if (this.isChanged()) {
if (
confirm(
'入力されたデータは送信されないまま破棄されますが,よろしいですか。'
)
) {
removeDraftConfirmer()
this.cleanupContent()
next()
} else {
next(false)
}
} else {
next()
}
} else {
next()
}
}
Copy link
Member

Choose a reason for hiding this comment

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

こここんな感じでearly returnさせるとネストが浅くなって読みやすくなるかも

Suggested change
beforLeaveGuardinEventEdit = (to, from, next) => {
if (from.name === 'EventEdit') {
if (this.isChanged()) {
if (
confirm(
'入力されたデータは送信されないまま破棄されますが,よろしいですか。'
)
) {
removeDraftConfirmer()
this.cleanupContent()
next()
} else {
next(false)
}
} else {
next()
}
} else {
next()
}
}
beforLeaveGuardinEventEdit = (to, from, next) => {
if (from.name !== 'EventEdit' || this.isChanged()) {
next()
}
if (
confirm(
'入力されたデータは送信されないまま破棄されますが,よろしいですか。'
)
) {
removeDraftConfirmer()
this.cleanupContent()
next()
}
next(false)
}

@mathsuky
Copy link
Contributor Author

レビューありがとうございます!
いただいた指摘をもとに修正しましたので,確認お願いします🙏

@itt828
Copy link
Member

itt828 commented Oct 27, 2023

ありがとうございます!マージします!

@itt828 itt828 merged commit def3551 into master Oct 27, 2023
6 checks passed
@itt828 itt828 deleted the unsaved-warning branch October 27, 2023 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants