Skip to content
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

Merged
merged 16 commits into from
Sep 9, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added doc/specs/#1564 - Settings UI/navigation-2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/specs/#1564 - Settings UI/navigation.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
123 changes: 123 additions & 0 deletions doc/specs/#1564 - Settings UI/spec.md
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.
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

@carlos-zamora carlos-zamora Jul 10, 2020

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)


### 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

Copy link
Member

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.)

### 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.
Copy link
Member

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.

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.

Copy link
Contributor

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.

Copy link
Contributor

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 - 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.
Copy link
Member

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?

Copy link
Contributor

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"?


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

Copy link
Member

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.

Copy link
Member

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.

## UI/UX Design

### Top-level navigation

#### Proposal 1 - Align with JSON
Copy link
Member

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.

Copy link
Member

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.


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
Copy link
Member

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)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug: #4139

Copy link
Member

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
Copy link
Member

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.

Copy link
Member

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?

- 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
Copy link
Member

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?

Copy link
Member

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.


## 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