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

Save UI state per Workspace #984

Merged
merged 11 commits into from
Feb 20, 2023
Merged

Save UI state per Workspace #984

merged 11 commits into from
Feb 20, 2023

Conversation

Code-DJ
Copy link
Contributor

@Code-DJ Code-DJ commented Jan 16, 2023

Description

Save UI state per Workspace.

  • sidebar width
  • which files are open
  • which editor tab is active
  • order of editor tabs
  • the navigator sidebar, inspector sidebar, or debugger drawer
    • opened/closed state
    • active tab,
    • tab order in each
  • editor layout (when we build split editor layout in - does not need to be done now consequently)

Related Issue

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • I documented my code
  • Review requested

Screenshots

@matthijseikelenboom
Copy link
Contributor

Looks good to me so far

@Code-DJ Code-DJ marked this pull request as ready for review January 25, 2023 16:55
@Code-DJ Code-DJ force-pushed the main branch 2 times, most recently from b35c7c3 to 26ba72d Compare January 25, 2023 17:27
@austincondiff austincondiff self-requested a review January 28, 2023 18:02
@Code-DJ
Copy link
Contributor Author

Code-DJ commented Feb 11, 2023

Ready for review. Will create a new PR for remaining items after this one is merged.

austincondiff
austincondiff previously approved these changes Feb 11, 2023
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 great, very nice work! 👍🏻

Copy link

@Setting-Bunny Setting-Bunny left a comment

Choose a reason for hiding this comment

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

Approving

Copy link

@Setting-Bunny Setting-Bunny left a comment

Choose a reason for hiding this comment

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

So good


Copy link
Contributor

@matthijseikelenboom matthijseikelenboom left a comment

Choose a reason for hiding this comment

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

My only concern is what happens when the user decides to delete CodeEdit, do these states stay on the machine? Or is UserDefault a thing that is encapsulated in the app itself?

@Code-DJ
Copy link
Contributor Author

Code-DJ commented Feb 17, 2023

My only concern is what happens when the user decides to delete CodeEdit, do these states stay on the machine? Or is UserDefault a thing that is encapsulated in the app itself?

I think it stays back and doesn't get deleted. We need to let macOS cleaner apps also to make money /jk

On a serious note, apps usually don't delete the state and just pickup where they left off in-case user reinstalls the app. Since uninstalling is to move the app to Trash, I am not sure if we have control over it.

@austincondiff
Copy link
Collaborator

Yeah, I think @Code-DJ is right. I think this is intentional and by design. If the user wants to get rid of these settings that consume an insignificant amount of space on their disk, they can either do so manually by deleting our folder in Application Support or they can use on of many apps to clean out this kind of data. Unlike Windows, it will not slow down your machine or degrade your experience in any way by that data being there.

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.

Fix failed tests

@Code-DJ
Copy link
Contributor Author

Code-DJ commented Feb 20, 2023

Fix failed tests

Rebased the fork which resolved the build issue. A couple of variable definitions were missing from the file in my fork. Verified that it builds correctly.

@austincondiff austincondiff merged commit eb14077 into CodeEditApp:main Feb 20, 2023
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.

5 participants