-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Conversation
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 like to see a bit more detail regarding the specific pages and like a storyboard for the experience. We don't need "booleans are checkboxes" level of detail, but a rough mockup of each page should suffice.
I think we also need to think about some of the other features that came in recently or are coming in soon. Specifically, I'm thinking about these:
- Pointer Bindings (you and I can chat about this offline for the vision I have there)
- Command palette commands
- XAML Theming (PR)
- New Tab Menu Customization (PR)
Really, just Command Palette is the big one. All of the other ones we could probably wait a bit for the specs to settle?
Also, it'd be neat if we could include links to the docs somewhere. Idk how often or where, but just a small callout for "each page will have a link to the docs" or something like that should be fine.
- Globals | ||
- Profiles | ||
- Color schemes | ||
- Key bindings |
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.
For the Key bindings page:
- Where does Command Palette customization come in? In the JSON, we're overriding the
keybindings
key withbindings
. Should that be replicated here by replacing the "Key bindings" navigation item with "Bindings"? Is that intuitive? - Also, where will pointer bindings go in?
- How do we record/represent the default and user-defined keybindings?
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.
In my opinion, not everything needs to be exposed with the UI, at least not by default. Consider themes - these might be impossible to configure with the UI, so let's leave it as an exemplary case of "doesn't use the UI".
I'd think that at first, the command palette also isn't in the UI. There might be harder edge cases for editing commands with a UI, so lets just assume that we're not getting to it by v2.0.
In the future, I'd imagine that "Keybindings", "Commands", "Mouse Bindings" are all separate top-level settings pages, despite all being stored in the bindings
. From the user's perspective, they're all separate entities, despite all being named the same thing.
#### 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should not be "Save" but "Apply 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 suppose provide feedback early and often. I'm not sure this is all my thoughts on the subject, but this certainly is some of them
|
||
### Top-level navigation | ||
|
||
#### Proposal 1 - Align with 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.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Proposal 2 seems more intuitive. Let's go with that one.
- Add new | ||
- Keyboard | ||
- Mouse* | ||
- Command Palette |
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 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 comment
The 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?
|
||
#### 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 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.
#### 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. | ||
|
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 a SettingsPage
" 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.)
|
||
This does not affect performance, power, nor efficiency. | ||
|
||
## Potential Issues |
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.
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. | ||
|
||
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. | ||
|
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.
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 hidden
.
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.
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.
|
||
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. |
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?...
- Open the JSON file
- Open the Defaults JSON file
(name could use some massaging but you get the idea)
|
||
### Top-level navigation | ||
|
||
#### Proposal 1 - Align with 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.
+1 Proposal 2 seems more intuitive. Let's go with that one.
- Tabs | ||
- Profiles | ||
- Defaults | ||
- Enumerate profiles |
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?
- Add new | ||
- Keyboard | ||
- Mouse* | ||
- Command Palette |
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 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?
|
||
This does not affect performance, power, nor efficiency. | ||
|
||
## Potential Issues |
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.
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. | ||
|
||
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. | ||
|
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.
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.
Friday Spec Brownbag Quick SummaryToday we had a meeting about this spec. Here's some quick notes of how the discussion went. Launch Methods
Preview Window in Appearance Page
Editing/Saving
Navigation
Keyboard Bindings
(I didn't catch which one Leon was up with)
|
Hello, The
should have a info sign |
|
Ideally, we'd have validation through the Settings UI too or instead. I'm realizing that through the Settings UI, you don't have to escape characters. So you can't just copy/paste some text from the JSON. |
|
||
## Appendix 1 | ||
|
||
Below is the list of all settings on their respective pages in the settings UI. The title row aligns with the navigation view on the left of the UI. Bolded headers in those columns align with top nav on the page. |
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.
Anything with a path should also have a "browse" option:
- Background image
- Command line
- Icon
- Starting directory
Here's a doc that might help with that?
- Tabs | ||
- Profiles | ||
- Defaults | ||
- Enumerate profiles |
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?
My humble feedback:
|
Co-authored-by: Carlos Zamora <[email protected]>
Maybe we should have the title bar only extend to the side of the navbar and have the navigation bar extend to the top? And we should also have an acrylic (transparent) navbar. |
@Chips1234 The view with title bar looks ugly i mean look at the settings app of windows it is a beauty because of acrylic navigation and no title bar. It has a settings app title but on the acrylic nav bar |
Yes, agreed. |
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.
Extend the navbar all the way to the top and maybe have it acrylic.
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.
You know what, I'm looking through these designs and I dig it. I'm cool with this spec as-is, and I think the big contentious things (like general page layout, autosave or not, and in-window or pop-up window) are all answered.
Let's ship this and figure the rest out in post 🚀
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.
🚢
On the setting topic, I wonder if we could have setting json open in a new tab? Simular how vscode have both text and gui versions |
@quangkieu that only works because VSCode is also a text editor... which terminal is not 😄 |
@DHowett What I mean to suggest is if we have "Open JSON File" in this pull request #7370 to open a new panel that only have an editable text area box of the JSON content with save button. We would only check for JSON validation before save as full Setting validation would be check on read anyway. The "Open JSON File" would have a small > arrow button next to it for open external program. This request is mostly for quick edit like name of profile as I have default JSON open in Visual Studio and that is huge load time. |
This commit introduces a rough prototype of the Settings UI as the `TerminalSettingsEditor`. This project was added to OpenConsole.sln and deploys as a standalone app. Some databinding is configured to a fake TerminalSettingsModel (located in the ObjectModel folder). This commit will start the settings UI feature branch, which will receive a full review on merge-back into main. References #6720 - Settings UI Spec and Design References #1564 - Settings UI Feature/Epic
This commit introduces a rough prototype of the Settings UI as the `TerminalSettingsEditor`. This project was added to OpenConsole.sln and deploys as a standalone app. Some databinding is configured to a fake TerminalSettingsModel (located in the ObjectModel folder). This commit will start the settings UI feature branch, which will receive a full review on merge-back into main. References #6720 - Settings UI Spec and Design References #1564 - Settings UI Feature/Epic
This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in #6720, #8269 Follow-up work in #6800 Closes #1564 Closes #8048 (PR) Co-authored-by: Carlos Zamora <[email protected]> Co-authored-by: Kayla Cinnamon <[email protected]> Co-authored-by: Alberto Medina Gutierrez <[email protected]> Co-authored-by: John Grandle <[email protected]> Co-authored-by: xerootg <[email protected]> Co-authored-by: Scott <[email protected]> Co-authored-by: Vineeth Thomas Alex <[email protected]> Co-authored-by: Leon Liang <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]> Signed-off-by: Dustin L. Howett <[email protected]>
This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in #6720, #8269 Follow-up work in #6800 Closes #1564 Closes #8048 (PR) Co-authored-by: Carlos Zamora <[email protected]> Co-authored-by: Kayla Cinnamon <[email protected]> Co-authored-by: Alberto Medina Gutierrez <[email protected]> Co-authored-by: John Grandle <[email protected]> Co-authored-by: xerootg <[email protected]> Co-authored-by: Scott <[email protected]> Co-authored-by: Vineeth Thomas Alex <[email protected]> Co-authored-by: Leon Liang <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]> Signed-off-by: Dustin L. Howett <[email protected]>
This commit introduces the terminal settings editor (to wit: the Settings UI) as a standalone project. This project, and this commit, is the result of two and a half months of work. TSE started as a hackathon project in the Microsoft 2020 Hackathon, and from there it's grown to be a bona-fide graphical settings editor. There is a lot of xaml data binding in here, a number of views and a number of view models, and a bunch of paradigms that we've been reviewing and testing out and designing and refining. Specified in microsoft#6720, microsoft#8269 Follow-up work in microsoft#6800 Closes microsoft#1564 Closes microsoft#8048 (PR) Co-authored-by: Carlos Zamora <[email protected]> Co-authored-by: Kayla Cinnamon <[email protected]> Co-authored-by: Alberto Medina Gutierrez <[email protected]> Co-authored-by: John Grandle <[email protected]> Co-authored-by: xerootg <[email protected]> Co-authored-by: Scott <[email protected]> Co-authored-by: Vineeth Thomas Alex <[email protected]> Co-authored-by: Leon Liang <[email protected]> Co-authored-by: Dustin L. Howett <[email protected]> Signed-off-by: Dustin L. Howett <[email protected]>
Summary of the Pull Request
This is the spec for the overall functionality of the settings UI - #1564.
There are proposals for the launch method and editing and saving settings that we should discuss.
References
PR Checklist
Detailed Description of the Pull Request / Additional comments
spec.md
design.md
Validation Steps Performed