-
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
Add initial settings UI spec #6720
Changes from 4 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
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,123 @@ | ||
--- | ||
author: Kayla Cinnamon @cinnamon-msft | ||
created on: 2020-06-29 | ||
last updated: 2020-07-08 | ||
issue id: #1564 | ||
--- | ||
|
||
# Settings UI Implementation | ||
|
||
## Abstract | ||
|
||
This spec describes the basic functionality of the settings UI, including disabling it, the navigation items, launch methods, and editing of settings. The specific layout of each page will defined in later design reviews. | ||
|
||
## Inspiration | ||
|
||
We have been wanting a settings UI since the dawn of Terminal time, so we need to define how it will interact with the application and how users should expect to interact with it. | ||
|
||
## Solution Design | ||
|
||
The settings UI will be the default experience. We will provide users an option to skip the settings UI and edit the raw JSON file. | ||
|
||
### Ability to disable displaying the settings UI | ||
|
||
Some users don't want a UI for the settings. We can update the `openSettings` key binding with a `settingsUI` option. | ||
|
||
If people still like the UI but want to access the JSON file, we can provide an "Open the JSON file" button at the bottom of the navigation menu. | ||
|
||
### Launch methods | ||
|
||
#### Proposal 1 - Launch in a new window | ||
|
||
Clicking the settings button in the dropdown menu will open the settings UI in a new window. This allows the user to edit their settings and see the Terminal live update with their changes. | ||
|
||
In the Windows taskbar, the icon will appear as if Terminal has multiple windows open. | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
#### Proposal 2 - Launch in a new tab | ||
|
||
Clicking the settings button in the dropdown menu will open the settings UI in a new tab. This helps us take steps toward supporting non-terminal content in a tab. However, tab tear-off should be implemented before we have this. This way, users can still see their changes take effect if they want by tearing out the settings UI. | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
### Editing and saving settings | ||
|
||
#### Proposal 1 - Automatically save settings | ||
|
||
As users edit fields in the settings UI, they are automatically saved and written to the JSON file. This allows the user to see their settings changes taking place in real time. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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 commentThe 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. |
||
|
||
#### Proposal 2 - Implement a save button | ||
|
||
Users will only see their settings changes take place once they click "Save". Clicking "Save" will write to the settings.json file. This aligns with the functionality that exists today by editing the settings.json file in a text editor and saving it. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I prefer this? Should we have a "reset" button and a "reset to defaults button" too? We could preview the changes as you make them in the open Terminal, then "save" actually applies them to the JSON file? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it should not be "Save" but "Apply Changes"? |
||
|
||
A future option could be adding a TerminalControl inside the settings UI to preview what the changes will look like before actually saving them to the settings.json file. | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we discuss at all the technical details of how the settings are saved? Mention something about trying to make minimal updates to the user's settings file whenever possible? This might be more dev-spec'y, but I'm thinking also about things like deleting profiles. For user-defined profiles, we can just remove them from the json, easy. But for something like the default profiles and dynamic ones, we need to mark them as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just spoke with Kayla. I can handle that side of things and spec out the technical details, but add it to this PR. That way everything is all in one spot. I'll probably start with this next week, after we have our design review meeting on Friday. |
||
## UI/UX Design | ||
|
||
### Top-level navigation | ||
|
||
#### Proposal 1 - Align with JSON | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really hate this proposal, and love "Proposal 2 - More descriptive navigation". Let's expose the setting to the user in a more user-friendly rather than exposing how the code works. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 Proposal 2 seems more intuitive. Let's go with that one. |
||
|
||
The settings UI could have top-level navigation that aligns with the overall structure of the settings.json file. The following are the proposed navigation items: | ||
|
||
- Globals | ||
- Profiles | ||
- Color schemes | ||
- Bindings | ||
|
||
For Bindings, it would have key bindings, mouse bindings, and command palette inside it. I would like to discuss a better name aside from Bindings. I'm afraid having command palette under Bindings would be confusing. | ||
|
||
![Settings UI navigation 1](./navigation.png) | ||
|
||
#### Proposal 2 - More descriptive navigation | ||
|
||
Alternatively, we could break up the navigation menu into more digestible sections. This aligns more closely to other terminals. The following are the proposed navigation items: | ||
|
||
- General | ||
- Appearance | ||
- Global | ||
- Color schemes | ||
- Themes | ||
- Tabs | ||
- Profiles | ||
- Defaults | ||
- Enumerate profiles | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe 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? |
||
- Add new | ||
- Keyboard | ||
- Mouse* | ||
- Command Palette | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm really okay with the UI for the command palette settings not being in Terminal 2.0 - it might have more edge cases than we're prepared to deal with in the rest of this cycle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want Command Palette SUI for Terminal v2. But I'm ok with that being something we add in like a month or 2 after Command Palette is widely available and finished. The main concern is adding nested and iterable commands right? We could even just design/implement the Command Palette SUI without that, then add those two in later? |
||
- Marketplace* | ||
|
||
\* Mouse and marketplace will be added once they're implemented. | ||
|
||
![Settings UI navigation 2](./navigation-2.png) | ||
|
||
## Capabilities | ||
|
||
### Accessibility | ||
|
||
This will have to undergo full accessibility testing because it is a new UI element. All items inside the settings UI should be accessible by a screen reader and the keyboard. Additionally, all of the settings UI will have to be localized. | ||
|
||
### Security | ||
|
||
This does not impact security. | ||
|
||
### Reliability | ||
|
||
This will not improve reliability. | ||
|
||
### Compatibility | ||
|
||
This will change the default experience to open the UI, rather than the JSON file in a text editor. This behavior can be reverted with the setting listed [above](#ability-to-disable-displaying-the-settings-ui). | ||
|
||
### Performance, Power, and Efficiency | ||
|
||
This does not affect performance, power, nor efficiency. | ||
|
||
## Potential Issues | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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. |
||
|
||
## Future considerations | ||
|
||
- We will have to have design reviews for all of the content pages. | ||
- We should have undo functionality. In a text editor, you can type `Ctrl+Z` however the settings UI is a bit more complex. | ||
carlos-zamora marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- Once we have a marketplace for themes and extensions, this should be added to the top-level navigation. | ||
- As we add more features, the top-level navigation is subject to change in favor of improved usability. | ||
|
||
## Resources |
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)