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

fix(#1557): autosave #1577

Merged
merged 2 commits into from
Jan 28, 2024
Merged

fix(#1557): autosave #1577

merged 2 commits into from
Jan 28, 2024

Conversation

ElvisWong213
Copy link
Contributor

Description

When making changes to a file and having autosave enabled, the edits will be saved.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

@austincondiff
Copy link
Collaborator

This looks great, just make sure if the user disables autosave in Settings it works as expected.

- When the user changes the autosave setting from off to on, it will save the previous changes.
- The user discarded change, but the editor didn't update.
@ElvisWong213
Copy link
Contributor Author

It should work as expected now.

Autosave On

Screen.recording.2024-01-28.am.12.34.22.mov

Autosave Off

Screen.recording.2024-01-28.am.12.31.23.mov

Copy link
Collaborator

@austincondiff austincondiff left a comment

Choose a reason for hiding this comment

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

Looks good!

tom-ludwig
tom-ludwig previously approved these changes Jan 28, 2024
Copy link
Member

@tom-ludwig tom-ludwig left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for your work.
It closes #1557, doesn't it?

@tom-ludwig
Copy link
Member

Oh just ran into a glitch: If I edit a doc with auto-save off, then switch auto-save back on, the changed file indicator sticks around. It's not behaving as expected. @austincondiff, should I go ahead and merge this PR? It technically addresses the auto-save issue.

Screen.Recording.2024-01-28.at.11.00.13.AM.mov

@tom-ludwig tom-ludwig dismissed their stale review January 28, 2024 10:05

I have noticed a problem and am waiting for Austin's approval.

@austincondiff
Copy link
Collaborator

It seems that you are turning auto save on with an unsaved file open. If it has been saved and it is turned on does it work as expected?

In this case we need to merge this PR and open a subsequent issue addressing the enabling of auto save while an unsaved file is open. What is the expected behavior when this happens? Should it save all unsaved files? Should we wait to save an unsaved file after the first modification? Should we wait to save after the first manual save then auto save works normally for that file?

@tom-ludwig
Copy link
Member

Yeah, saving first and turning auto-save back on works fine, so let's go ahead and merge.

Regarding the other questions, I'm currently uncertain. Let's discuss those matters in detail within the issue once it's created

@tom-ludwig tom-ludwig enabled auto-merge (squash) January 28, 2024 15:54
@tom-ludwig tom-ludwig merged commit aac6540 into CodeEditApp:main Jan 28, 2024
2 checks passed
@tom-ludwig tom-ludwig mentioned this pull request Jan 28, 2024
@ElvisWong213 ElvisWong213 deleted the fix/autosave branch January 28, 2024 22:54
@ElvisWong213 ElvisWong213 restored the fix/autosave branch February 4, 2024 09:16
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.

3 participants