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

Option for "Leave site?" warning #20347

Closed
Ryan-pelo opened this issue Jul 12, 2022 · 10 comments
Closed

Option for "Leave site?" warning #20347

Ryan-pelo opened this issue Jul 12, 2022 · 10 comments
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@Ryan-pelo
Copy link

Feature Description

A few times I have been typing a long comment or post and accidently clicked a link or refreshed the page. Then all my work is gone and non-recoverable. I would appreciate having a "Leave Site?" dialogue box if I have unsaved content in a text box.

Screenshots

Screenshot 2022-07-12 140337
@Ryan-pelo Ryan-pelo added type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. labels Jul 12, 2022
@wxiaoguang
Copy link
Contributor

Related:

@silverwind
Copy link
Member

silverwind commented Jul 14, 2022 •
edited
Loading

There is already a warning via jquery.are-you-sure, but I'm not sure whether it will catch all changes, I think it only looks at stuff inside <form>.

Also, we really need to eliminate all cases of window.reload in JS. Might require new APIs to be added thought.

@wxiaoguang wxiaoguang mentioned this issue Feb 22, 2023
Merged
lunny added a commit that referenced this issue Feb 24, 2023
@wxiaoguang @brechtvl @lunny
Make issue meta dropdown support Enter, confirm before reloading (#23014
0bc8bb3 
)

As the title. Label/assignee share the same code.

* Close #22607
* Close #20727

Also:

* partially fix for #21742, now the comment reaction and menu work with
keyboard.
* partially fix for #17705, in most cases the comment won't be lost.
* partially fix for #21539
* partially fix for #20347
* partially fix for #7329

### The `Enter` support

Before, if user presses Enter, the dropdown just disappears and nothing
happens or the window reloads.

After, Enter can be used to select/deselect labels, and press Esc to
hide the dropdown to update the labels (still no way to cancel ....
maybe you can do a Cmd+R or F5 to refresh the window to discard the
changes .....)


This is only a quick patch, the UX is still not perfect, but it's much
better than before.


### The `confirm` before reloading

And more fixes for the `reload` problem, the new behaviors:

* If nothing changes (just show/hide the dropdown), then the page won't
be reloaded.
* If there are draft comments, show a confirm dialog before reloading,
to avoid losing comments.

That's the best effect can be done at the moment, unless completely
refactor these dropdown related code.

Screenshot of the confirm dialog:

<details>


![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png)

</details>

---------

Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@yardenshoham yardenshoham mentioned this issue Feb 24, 2023
Merged
yardenshoham pushed a commit to yardenshoham/gitea that referenced this issue Feb 24, 2023
@wxiaoguang @brechtvl
@lunny @yardenshoham
Make issue meta dropdown support Enter, confirm before reloading (go-…
ad252a7 
…gitea#23014)

As the title. Label/assignee share the same code.

* Close go-gitea#22607
* Close go-gitea#20727

Also:

* partially fix for go-gitea#21742, now the comment reaction and menu work with
keyboard.
* partially fix for go-gitea#17705, in most cases the comment won't be lost.
* partially fix for go-gitea#21539
* partially fix for go-gitea#20347
* partially fix for go-gitea#7329

### The `Enter` support

Before, if user presses Enter, the dropdown just disappears and nothing
happens or the window reloads.

After, Enter can be used to select/deselect labels, and press Esc to
hide the dropdown to update the labels (still no way to cancel ....
maybe you can do a Cmd+R or F5 to refresh the window to discard the
changes .....)


This is only a quick patch, the UX is still not perfect, but it's much
better than before.


### The `confirm` before reloading

And more fixes for the `reload` problem, the new behaviors:

* If nothing changes (just show/hide the dropdown), then the page won't
be reloaded.
* If there are draft comments, show a confirm dialog before reloading,
to avoid losing comments.

That's the best effect can be done at the moment, unless completely
refactor these dropdown related code.

Screenshot of the confirm dialog:

<details>


![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png)

</details>

---------

Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
lunny added a commit that referenced this issue Feb 24, 2023
@yardenshoham @wxiaoguang
@brechtvl @lunny
Make issue meta dropdown support Enter, confirm before reloading (#23014
5ddf67a 
) (#23102)

Backport #23014

As the title. Label/assignee share the same code.

* Close #22607
* Close #20727

Also:

* partially fix for #21742, now the comment reaction and menu work with
keyboard.
* partially fix for #17705, in most cases the comment won't be lost.
* partially fix for #21539
* partially fix for #20347
* partially fix for #7329

### The `Enter` support

Before, if user presses Enter, the dropdown just disappears and nothing
happens or the window reloads.

After, Enter can be used to select/deselect labels, and press Esc to
hide the dropdown to update the labels (still no way to cancel ....
maybe you can do a Cmd+R or F5 to refresh the window to discard the
changes .....)


This is only a quick patch, the UX is still not perfect, but it's much
better than before.


### The `confirm` before reloading

And more fixes for the `reload` problem, the new behaviors:

* If nothing changes (just show/hide the dropdown), then the page won't
be reloaded.
* If there are draft comments, show a confirm dialog before reloading,
to avoid losing comments.

That's the best effect can be done at the moment, unless completely
refactor these dropdown related code.

Screenshot of the confirm dialog:

<details>


![image](https://user-images.githubusercontent.com/2114189/220538288-e2da8459-6a4e-43cb-8596-74057f8a03a2.png)

</details>

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Brecht Van Lommel <brecht@blender.org>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@wxiaoguang
Copy link
Contributor

Some details & background:

Before SimpleMDE/EasyMDE, there is a global $.areYourSure confirm (using onbeforeunload) for comment textarea. Everything was just "fine".

However, after introducing SimpleMDE/EasyMDE, $.areYourSure doesn't work well with SimpleMDE/EasyMDE editor, so the situation became what you see now.

@silverwind
Copy link
Member

silverwind commented Mar 2, 2023 •
edited
Loading

Another reason to add to the list of like 30+ other issues on why we should remove EasyMDE and replace it with a textarea.

@wxiaoguang
Copy link
Contributor

I could also agree to remove the EasyMDE if there is no strong objection from user side.

@silverwind
Copy link
Member

Yes I think the majority of users is for it, see #10729.

@wxiaoguang
Copy link
Contributor

Then let's do it?

@silverwind
Copy link
Member

Yes, I think the approach in #15394 with the github toolbar is still ideal. If you have the capacity to do it, please go ahead, I'm happy to review.

@lunny
Copy link
Member

lunny commented Apr 3, 2023

Should be fixed by #23895

@silverwind
Copy link
Member

silverwind commented Apr 3, 2023 •
edited
Loading

Works now with #23876

Screenshot 2023-04-03 at 22 34 48

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@lunny @silverwind @wxiaoguang @Ryan-pelo