Skip to content

Workspace settings #1670

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

Merged
merged 11 commits into from
Apr 18, 2024
Merged

Conversation

armartinez
Copy link
Contributor

Description

This PR adds the base implementation for the workspace settings, which are created on the ./codeedit/ hidden folder inside the base path of the workspace once one of the settings is changed. Currently it just shows the general and tasks settings into a single sheet and the stored settings are not being used elsewhere.

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

Screenshot 2024-04-13 at 13 26 34 Screenshot 2024-04-13 at 13 25 39

@austincondiff
Copy link
Collaborator

@armartinez Looks good from what I can tell. Please resolve swift lint errors though.

Copy link
Collaborator

@0xWDG 0xWDG left a comment

Choose a reason for hiding this comment

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

The changes for the packages do not work with the tests, can you revert those so the tests can be run. Thanks!

@armartinez armartinez requested a review from 0xWDG April 14, 2024 10:13
0xWDG
0xWDG previously approved these changes Apr 14, 2024
Copy link
Collaborator

@0xWDG 0xWDG left a comment

Choose a reason for hiding this comment

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

🎉🚀

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.

Thanks for your amazing changes!

Some other issues I found while trying out the settings:

  • If a user has the workspace settings open, he would still be able to click into the editor without the window disappearing. I think this would be fixed by using a sheet instead of a custom window.
  • The “Tasks” toggle is not descriptive enough. Does the toggle indicate whether the task should be executed? Maybe a tooltip would help
  • Environment variables should be deleted when pressing backspace;
  • It seems there is an issue with the Environment Variable View, not sure if this is fixable:
Screenshot 2024-04-15 at 7 22 52 PM

@armartinez
Copy link
Contributor Author

armartinez commented Apr 15, 2024

Thanks for your amazing changes!

Some other issues I found while trying out the settings:

  • If a user has the workspace settings open, he would still be able to click into the editor without the window disappearing. I think this would be fixed by using a sheet instead of a custom window.
  • The “Tasks” toggle is not descriptive enough. Does the toggle indicate whether the task should be executed? Maybe a tooltip would help
  • Environment variables should be deleted when pressing backspace;
  • It seems there is an issue with the Environment Variable View, not sure if this is fixable:
Screenshot 2024-04-15 at 7 22 52 PM
  • I'm aiming for the same behaviour as the normal settings which are also a window.
  • Sure I can add some comment.
  • Can you be more specific about the issue in the env variables view ?

@tom-ludwig
Copy link
Member

@armartinez, it kinda looks off, doesn't it? I think it's the blue highlighting. No need to worry about this small design thing now; we can fix it later. I can be a bit to picky sometimes 😁

@armartinez armartinez requested a review from 0xWDG April 17, 2024 14:59
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.

LGTM

We should definitely take a look at how we can make better use of SwiftLint.

Copy link
Member

@FastestMolasses FastestMolasses left a comment

Choose a reason for hiding this comment

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

LGTM

@austincondiff austincondiff merged commit 2546dad into CodeEditApp:main Apr 18, 2024
@austincondiff
Copy link
Collaborator

@allcontributors add @armartinez for code

Copy link
Contributor

@austincondiff

I've put up a pull request to add @armartinez! 🎉

@armartinez armartinez deleted the workspace-settings branch April 18, 2024 18:41
@thecoolwinter thecoolwinter added the enhancement New feature or request label Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants