-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Move to macOS Ventura-style Settings in place of Preferences #1215
Conversation
* add initial ventura-style preferences ui * fix lint errors * refactor Settings -> Preferences * add comments and rename GeneralSettings -> Preferences * make font size 12 and images 20x20 * minor rephrase * add venturacolors.swift * fix colors * fix text-misaligment * remove VStack, fixing an issue * preferences initial refactor * continue preference refactor * refactor SourceControlGeneralView.swift and start refactor on SourceControlGitView * refactor SourceControlGitView and fix lint errors * fix issue * fix code inconstencies * fix warnings * edit docs * fix docs problem * remove public * Partially fix search issue * fix luah5#3 * add explicit values, fix luah5#1 and intiate fix to issue where text if cut-off * small changes * add default selection
…w to get it closer to our target
…into luah5-ventura-settings
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
…nto refactor/settings
…strain their height to be within the parent window.
…king at details view.
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
…nto refactor/settings
…tabs in source control settings page.
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple comments for changes to make and an indentation. Otherwise great job, this is a large transition and I couldn't find any regressions in any preferences.
One thing that I would suggest that might be out of scope is adding tooltips for settings so users can hover over them and get a longer description of what they do. I noticed there isn't any of those in this PR. SwiftUI makes it quite easy to do with the help
modifier.
CodeEdit/Features/Settings/Pages/TextEditingSettings/TextEditingSettingsView.swift
Outdated
Show resolved
Hide resolved
CodeEdit/Features/Settings/Pages/GeneralSettings/GeneralSettingsView.swift
Outdated
Show resolved
Hide resolved
CodeEdit/Features/Settings/Pages/GeneralSettings/GeneralSettingsView.swift
Show resolved
Hide resolved
CodeEdit/Features/Settings/Pages/TerminalSettings/TerminalSettingsView.swift
Outdated
Show resolved
Hide resolved
CodeEdit/Features/Settings/Pages/SourceControlSettings/SourceControlGitView.swift
Show resolved
Hide resolved
CodeEdit/Features/Settings/Pages/SourceControlSettings/SourceControlGitView.swift
Show resolved
Hide resolved
…ar. Added new ExternalLink view that we are using in Locations settings.
…a toggle to use text editor font in terminal settings. Using the grouped form stepper throughout.
…st of the UI is not delayed while retrieving fonts.
Quick update... I've added a new monospaced font picker as seen here... Screen.Recording.2023-04-24.at.1.12.30.PM.mp4 |
…ed some of the theme settings options to work as expected.
Changes have been made since, need re-review
@thecoolwinter I can't think of anything right away that needs a help tooltip. Let's push this to another scope of work. If you think of anything submit a Settings help tooltips issue and we can add them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We do need to have some code style improvements, but that is a project wide thing to fix and unrelated to this PR.
Description
The current Preferences implementation of preferences now will use the new grouped form style introduced in the latest version of SwiftUI and will also adopt a sidebar for to accommodate a larger set of Settings Pages. It also resembles System Settings in macOS Ventura. We are now calling it Settings instead of Preferences to follow the newer macOS naming convention.
Please note: This is a large PR. I'd encourage reviewers to pull it down and play with it in addition to a thorough review of the code.
Related Issues
Checklist
Behaviors(not yet implemented)Navigation(not yet implemented)Key Bindings(not yet implemented)Components(not yet implemented)Advanced(not yet implemented)Screenshots