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

Autosave (draft) in text editor #1315

Open
pgaskin opened this issue Mar 17, 2017 · 10 comments
Open

Autosave (draft) in text editor #1315

pgaskin opened this issue Mar 17, 2017 · 10 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/feature Completely new functionality. Can only be merged if feature freeze is not active.

Comments

@pgaskin
Copy link
Contributor

pgaskin commented Mar 17, 2017

It would be useful if the web based text editor automatically saved the contents of the file until it is out of date or is commited or canceled. This feature would protect users from accidental refreshes or browser crashes,

@lunny lunny added this to the 1.x.x milestone Mar 18, 2017
@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Mar 18, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Apr 18, 2017

How would it do that? new branch? hanging commits? what is a user makes a change, doesn't commit, then goes back to make a change? should it bring up the saved commit? what if we're not in the same branch anymore? or have merge-conflicts? (I'm trying to say that this isn't an easy thing to fix correctly of user-friendly)

@lunny
Copy link
Member

lunny commented Apr 19, 2017

I think maybe the autosave is in the web browser side. Store the draft in local storage in HTML5 or userdata in lower version of IE.

@stale
Copy link

stale bot commented Feb 15, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Feb 15, 2019
@stale
Copy link

stale bot commented Mar 1, 2019

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this as completed Mar 1, 2019
@lunny lunny added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Mar 2, 2019
@lunny lunny reopened this Mar 2, 2019
@6543
Copy link
Member

6543 commented Sep 17, 2020

@pgaskin can you check if this is still an issue?

@delvh
Copy link
Member

delvh commented May 24, 2021

It is basically implemented by now:
When refreshing the browser to the same page (or even when going back and forth between site history in this tab), the text stays persisted (tested on Firefox, cannot speak for other browsers).
Only when clicking on any link inside the side specifically and navigating back to the issue without using the history, only then the text will be forgotten. But I think that pretty much closes this issue.
To implement the last bit, data needed to be stored on the server side, but that would be rather error prone, full of unnecessary edge cases and not really useful. I'd say this closes this issue.

@kdumontnu
Copy link
Contributor

It is basically implemented by now:
When refreshing the browser to the same page (or even when going back and forth between site history in this tab), the text stays persisted (tested on Firefox, cannot speak for other browsers).
Only when clicking on any link inside the side specifically and navigating back to the issue without using the history, only then the text will be forgotten. But I think that pretty much closes this issue.
To implement the last bit, data needed to be stored on the server side, but that would be rather error prone, full of unnecessary edge cases and not really useful. I'd say this closes this issue.

Thanks for the update re: Firefox!
I respectfully disagree with your conclusion though. Having autosave is incredibly useful when working on design review feedback (I use this all of the time in GH) - nothing worse than losing work you've spent significant time on. Having the ability to "back" to the page is a nice hack, but not really a solution to this problem - especially for the Pull Request Review case where it's not saved.

It might make sense to split these into different cases though. For instance, creating a new comment, editing a comment, leaving PR review, and creating a release might require different behaviors. Having a solution just for new comments and new design review feedback would be quite helpful.

I'm also not sure this needs to be server-side. As lunny mentioned a while back local storage could be a relatively simple solution here.

@delvh
Copy link
Member

delvh commented Jun 2, 2021

The problem with that is, that as far as I am aware, the local storage is already used simply because of the way it works.
My understanding of how local storage might be implemented is exactly how autosaving text currently works.
Otherwise you would have a memory leak in your browser, if local storage would never get deleted.
By emptying it every time you change your recent browsing history by adding a new link, browsers would ensure that text can get persisted across reloads and moving back in the history while also clearing it once this text is obviously not wanted anymore.

If you persist it on the server side, the memory leak problem only shifts to the server, who now has to store text possibly forever, without a cleanup option. The only exception to this would be if you say that you delete any saved text equally i.e. a week after it has been created, by running a weekly cleanup during low workloads.

@lunny
Copy link
Member

lunny commented Apr 4, 2023

This should be fixed by #23895

@wxiaoguang
Copy link
Contributor

Wait for other steps in:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

No branches or pull requests

7 participants