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

EasyMDE: enable autosave #25026

Closed
wants to merge 1 commit into from
Closed

EasyMDE: enable autosave #25026

wants to merge 1 commit into from

Conversation

shastik
Copy link

@shastik shastik commented May 31, 2023

Enable autosave editor by default. It is annoying that you can lose whole text when you accidentally close a window or you do some miss click.

The easy markdown editor has options for this behavior.

https://github.com/Ionaru/easy-markdown-editor#options-list

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 31, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label May 31, 2023
@yardenshoham yardenshoham added the type/enhancement An improvement of existing functionality label May 31, 2023
@wxiaoguang
Copy link
Contributor

Related to:

I didn't do so because EasyMDE's autosave doesn't work well with Gitea's form. For example, if you enter some texts in the EasyMDE edtior, click "Comment and close", then you still get the drafts on the new page. And, some comments are submitted by AJAX (eg: inline comment, edit), EasyMDE's autosave doesn't work well with it either. Another reason is there is a plan to deprecate EasyMDE, so there needs a general solution for the native textarea editor.

Just share my opinion and knowledge, there is no block from my side if people like the EasyMDE autosave.

@silverwind
Copy link
Member

silverwind commented Jun 1, 2023

We already have something that preserves unsaved comment text for example on page reload, and I just tested it and it works for both editors, so I don't think we need this.

@wxiaoguang do you know which mechanism currently does it currently? We should remove the unsaved warning box if it works reliably.

@wxiaoguang
Copy link
Contributor

do you know which mechanism currently does it currently?

Do you mean "preserves unsaved comment text for example on page reload"? IIRC, it's provided by browser's cache, and related to : #22604

@silverwind
Copy link
Member

do you know which mechanism currently does it currently?

Do you mean "preserves unsaved comment text for example on page reload"? IIRC, it's provided by browser's cache, and related to : #22604

Yes. Interesting, I didn't expect this to be a browser-native mechanism.

@@ -190,6 +190,10 @@ class ComboMarkdownEditor {
// EasyMDE's CSS should be loaded via webpack config, otherwise our own styles can not overwrite the default styles.
const {default: EasyMDE} = await import(/* webpackChunkName: "easymde" */'easymde');
const easyMDEOpt = {
autosave: {
enabled: true,
uniqueId: this.textarea.id
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this PR! Is this ID unique to the textarea on issue pages as a whole, or does the textarea on each issue page have its own ID? The context on why I am asking is that users may have "drafts" for many issue comments, and if it were the same ID for all issue pages then they would only be able to have one draft.

A followup on @silverwind's comment re: removal of easymde, is we could use https://github.com/github/session-resume to achieve the same behaviour with the new text editor.

Copy link
Member

@silverwind silverwind Jun 2, 2023

Choose a reason for hiding this comment

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

https://github.com/github/session-resume looks interesting and can likely be used in place of jquery.are-you-sure for automatic form restoration. This could be used for all forms, not just issue comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually session-resume doesn't work well either. Think about 2 cases:

  1. Content in the textarea -> submit the form -> clean the draft -> network fails -> content lost
  2. Content in the textarea -> submit the form by AJAX -> draft is still in local storage

That's why "AJAX" is before the "LocalStorage Draft" in my summary issue #23290

@shastik shastik closed this Aug 1, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants