Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add initial settings UI spec #6720
Add initial settings UI spec #6720
Changes from all commits
53242c7
b414852
edfd673
f289008
70d76b2
de97933
47698e6
ae30dc9
3b6701f
2f31826
2eecb22
79e2b25
44d0c84
c67e8f5
fe4b76d
53e305c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Minor thing but it should be mentioned/discussed. How do you open defaults.json from the Settings UI instead of settings.json?
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.
Is this something we want? I'm imagining a "reset to defaults" button, which I can add to this spec.
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.
Not reset to defaults. Open the defaults.json. Right now, people alt-click "Settings" in the dropdown to open defaults.json. We've seen a ton of cases where people don't actually know that's a thing. Would it be possible to put 2 buttons in the Nav menu?...
(name could use some massaging but you get the idea)
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.
opinion - I think I prefer 1 to 2 as far as launching goes. In my head, there was a third "switch the
TerminalPage
for aSettingsPage
" option, where the entire contents of the window were changed into the settings UI until the user dismissed the settings. That certainly doesn't let the user preview the settings as well, and might be technically very challenging (esp with the titlebar shennanigans we currently have.)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'm not sure how I feel about this. I typically prefer a conscious "save" button. Think of something like the Settings app on Windows - it doesn't really provide any feedback when you change a setting, even though it auto-saves changes. As a user, I often worry that the settings app didn't actually change the values, because there's no feedback that the value I changed was persisted.
That being said, auto-saving is convenient. I think there just needs to be some sort of _subtle_visual cue that the settings were saved when the user makes changes.
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'm just going to aside that it's generally bad to get too far away from the OS default behaviors, so if Windows 10 is auto-saving settings, apps designed for Windows 10 should follow suit or convince MS to go back to the Apply/OK paradigm.
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.
My opinion: I liked the Apply/Cancel/Okay button since you can easily revert back if you don't like something.
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 think there should be an auto-save option. However, there should also be manual option such as Chips has mentioned. So basically how you save the settings should be an option in settings. That can go under a General or Maintenance Page at the very bottom that's used for directly configuring how you want the Settings UI to look and feel as well how it does things like saving.
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.
When we enumerate the profiles, how are "hidden" profiles handled? Do they still appear in the dropdown, but when you open it, there's a "hidden" switch? Or is there a way we can denote it as hidden in the dropdown (I think that would be the most ideal)?
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.
bug: #4139
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.
So, because of how we serialize "hidden", if a user hides a profile, the only way to un-hide it is to go to the json?
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.
Do we have any concerns about the way any of the current settings we have will be exposed to the user? Like, are there any where we're worried about what control we use to let the user change the setting?
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.
Keybindings and commands (CmdPlt) are gonna be the weird ones, but Kayla and I have been thinking about them for a while now so you'll get those mockups in the upcoming commits 😉
I think Enum flags might be a bit weird to represent. But we don't really have any settings out right now that use enum flags, so we'll probably just deal with them when the Settings UI XAML is actually being implemented.