-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Hook up the Save and Reset buttons #8348
Conversation
@DHowett somehow you looked at the commit that isn't HEAD haha. The logic for refreshing the list of profiles was added in 23f08e8. I don't see how I can consolidate that with the constructor though. |
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.
I'd sign off if not for the one UnparsedDefaultProfile
question
Hello @carlos-zamora! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Summary of the Pull Request
Adds an event handler for the Save and Reset buttons. "Save" writes the settings to disk using the API introduced by #8018. "Reset" simply overwrites the
settingsClone
(what the Settings UI reads from) withsettingsSource
(provided by TermApp on Settings UI initialization).References
#1564 - Settings UI
Validation Steps Performed
The following scenarios were tested and are verified to work properly: