From 963e62442e0cfcfc0c3af66cc8cc1b2f3e75bfec Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 20 Nov 2020 12:25:49 -0500 Subject: [PATCH 01/18] draft spec --- ...object for focused and unfocused states.md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 doc/specs/Configuration object for focused and unfocused states.md diff --git a/doc/specs/Configuration object for focused and unfocused states.md b/doc/specs/Configuration object for focused and unfocused states.md new file mode 100644 index 00000000000..2c69c5da294 --- /dev/null +++ b/doc/specs/Configuration object for focused and unfocused states.md @@ -0,0 +1,91 @@ +--- +author: +created on: <2020-11-20> +last updated: <2020-11-20> +issue id: +--- + +# Configuration object for focused/unfocused states + +## Abstract + +This spec outlines how we can support 'configuration objects' in our profiles, which +will allow us to render differently depending on whether the control is focused or unfocused. + +## Inspiration + +Reference: [#3062](https://github.com/microsoft/terminal/issues/3062) + +Users want there to be a more visible indicator than the one we have currently for which +pane is focused and which panes are unfocused. This change would grant us that feature. + +## Solution Design + +A new object in the `TerminalControl` namespace, called `UnfocusedRenderingParams`, +that will contain parameters for determining how a control, in an unfocused state, should be rendered. +This object will be available to both `TermControl` and `TerminalCore`. + +Our `TerminalSettingsModel` will parse out that object from the settings json file and pipe it over to +`TermControl`/`TerminalCore` to use. This will be done through `IControlSettings` and `ICoreSettings`, which +is the way we already pipe over information that these interfaces need to know regarding rendering (such as +`CursorStyle` and `FontFace`). + +### Allowed parameters + +Not all parameters which can be defined in a `Profile` can be defined in this new object (for example, we +do not want parameters which would cause a resize in this object.) Here are the list of parameters we +will allow: + +- Anything regarding colors: `colorScheme`, `foreground`, `background`, `cursorColor` etc +- `cursorShape` +- `backgroundImage` + +## UI/UX Design + +Users will be able to add a new setting to their profiles that will look like this: + +``` +"unfocusedState": +{ + "colorScheme": "Campbell", + "cursorColor": "#888", + "cursorShape": "emptyBox", + "foreground": "#C0C0C0", + "background": "#000000" +} +``` + +## Capabilities + +### Accessibility + +Does not affect accessibility. + +### Security + +Does not affect security. + +### Reliability + +This is another location in the settings where parsing/loading the settings may fail. However, this is the case +for any new setting we add so I would say that this is a reasonable cost for this feature. + +### Compatibility + +Should not affect compatibility. + +### Performance, Power, and Efficiency + +## Potential Issues + +Inactive tabs will be 'rendered' in the background with the `UnfocusedRenderingParams` object, we need to make +sure that switching to an inactive tab (and so causing the renderer to update with the 'normal' parameters) +does not cause the window to flash/show a jarring indicator that the rendering values changed. + +## Future considerations + +[comment]: # What are some of the things that the fixes/features might unlock in the future? Does the implementation of this spec enable scenarios? + +## Resources + + From a24d166892acb55c88946c36bd2b6a20257d6f06 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 20 Nov 2020 12:32:47 -0500 Subject: [PATCH 02/18] my alias is a misspelling --- .github/actions/spell-check/dictionary/names.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spell-check/dictionary/names.txt b/.github/actions/spell-check/dictionary/names.txt index dd4eaae1812..0d76d6e60ca 100644 --- a/.github/actions/spell-check/dictionary/names.txt +++ b/.github/actions/spell-check/dictionary/names.txt @@ -44,6 +44,7 @@ nvda oising oldnewthing osgwiki +pabhojwa paulcam pauldotknopf PGP From d5f6ced0e8264be8f63c1ede242ad8963104645f Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 20 Nov 2020 12:35:30 -0500 Subject: [PATCH 03/18] future consideration --- .../Configuration object for focused and unfocused states.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/specs/Configuration object for focused and unfocused states.md b/doc/specs/Configuration object for focused and unfocused states.md index 2c69c5da294..c1e9e8eac6e 100644 --- a/doc/specs/Configuration object for focused and unfocused states.md +++ b/doc/specs/Configuration object for focused and unfocused states.md @@ -2,7 +2,7 @@ author: created on: <2020-11-20> last updated: <2020-11-20> -issue id: +issue id: <#8345> --- # Configuration object for focused/unfocused states @@ -84,7 +84,8 @@ does not cause the window to flash/show a jarring indicator that the rendering v ## Future considerations -[comment]: # What are some of the things that the fixes/features might unlock in the future? Does the implementation of this spec enable scenarios? +When we allow for running elevated states in Terminal, a similar object can be used for rendering that session +differently. ## Resources From 83b78bae4bd607d33571e51b67a9bc20f3509bd6 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 23 Nov 2020 10:15:13 -0800 Subject: [PATCH 04/18] general object, not just for focused/unfocused --- ...s.md => Configuration object for profiles.md} | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) rename doc/specs/{Configuration object for focused and unfocused states.md => Configuration object for profiles.md} (76%) diff --git a/doc/specs/Configuration object for focused and unfocused states.md b/doc/specs/Configuration object for profiles.md similarity index 76% rename from doc/specs/Configuration object for focused and unfocused states.md rename to doc/specs/Configuration object for profiles.md index c1e9e8eac6e..1fa4e934a2f 100644 --- a/doc/specs/Configuration object for focused and unfocused states.md +++ b/doc/specs/Configuration object for profiles.md @@ -5,12 +5,15 @@ last updated: <2020-11-20> issue id: <#8345> --- -# Configuration object for focused/unfocused states +# Configuration object for profiles ## Abstract This spec outlines how we can support 'configuration objects' in our profiles, which -will allow us to render differently depending on whether the control is focused or unfocused. +will allow us to render differently depending on the state of the control. For example, a +control can be rendered differently if its focused as compared to when its unfocused. Another +example is that an elevated state control can be rendered differently as compared to a +non-elevated one. ## Inspiration @@ -21,9 +24,9 @@ pane is focused and which panes are unfocused. This change would grant us that f ## Solution Design -A new object in the `TerminalControl` namespace, called `UnfocusedRenderingParams`, -that will contain parameters for determining how a control, in an unfocused state, should be rendered. -This object will be available to both `TermControl` and `TerminalCore`. +A new class in the `TerminalControl` namespace, called `CustomRenderConfig`, that will contain rendering +parameters. `TermControl` and `TerminalCore` will use objects of this class to pass along rendering parameters +to the renderer. Our `TerminalSettingsModel` will parse out that object from the settings json file and pipe it over to `TermControl`/`TerminalCore` to use. This will be done through `IControlSettings` and `ICoreSettings`, which @@ -84,8 +87,7 @@ does not cause the window to flash/show a jarring indicator that the rendering v ## Future considerations -When we allow for running elevated states in Terminal, a similar object can be used for rendering that session -differently. +We will need to decide how this will look in the settings UI. ## Resources From 887af3b1f3ece9b970f68b837220bb4d403e8c4d Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 30 Nov 2020 10:44:33 -0800 Subject: [PATCH 05/18] clearer implementation details --- .../Configuration object for profiles.md | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 1fa4e934a2f..55dd14e8fe3 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -11,7 +11,7 @@ issue id: <#8345> This spec outlines how we can support 'configuration objects' in our profiles, which will allow us to render differently depending on the state of the control. For example, a -control can be rendered differently if its focused as compared to when its unfocused. Another +control can be rendered differently if it's focused as compared to when it's unfocused. Another example is that an elevated state control can be rendered differently as compared to a non-elevated one. @@ -24,20 +24,22 @@ pane is focused and which panes are unfocused. This change would grant us that f ## Solution Design -A new class in the `TerminalControl` namespace, called `CustomRenderConfig`, that will contain rendering -parameters. `TermControl` and `TerminalCore` will use objects of this class to pass along rendering parameters -to the renderer. +We will add an new interface in the `TerminalControl` namespace, called `IAppearanceConfig`, which defines how +`TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about (such as `FontFace`). +`TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. -Our `TerminalSettingsModel` will parse out that object from the settings json file and pipe it over to -`TermControl`/`TerminalCore` to use. This will be done through `IControlSettings` and `ICoreSettings`, which -is the way we already pipe over information that these interfaces need to know regarding rendering (such as -`CursorStyle` and `FontFace`). +On the Settings Model side, there will be a new class called `ProfileAppearanceConfig`. When we parse out the +settings json file, each state-appearance will be stored in an object of this class. Later on, these values get +piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the way we already pipe over information +such as `FontFace` and `CursorStyle` from the settings model to the app. + +#### Need to talk about inheritance here ### Allowed parameters -Not all parameters which can be defined in a `Profile` can be defined in this new object (for example, we -do not want parameters which would cause a resize in this object.) Here are the list of parameters we -will allow: +For now, these states are meant to be entirely appearance-based. So, not all parameters which can be +defined in a `Profile` can be defined in this new object (for example, we do not want parameters which +would cause a resize in this object.) Here are the list of parameters we will allow: - Anything regarding colors: `colorScheme`, `foreground`, `background`, `cursorColor` etc - `cursorShape` From 7ddb322010ea4116e06a28671ec5f83c43f9836e Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 30 Nov 2020 11:36:27 -0800 Subject: [PATCH 06/18] renaming --- doc/specs/Configuration object for profiles.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 55dd14e8fe3..a9f9e38ff01 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -24,11 +24,11 @@ pane is focused and which panes are unfocused. This change would grant us that f ## Solution Design -We will add an new interface in the `TerminalControl` namespace, called `IAppearanceConfig`, which defines how +We will add an new interface in the `TerminalControl` namespace, called `IAppearance`, which defines how `TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about (such as `FontFace`). `TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. -On the Settings Model side, there will be a new class called `ProfileAppearanceConfig`. When we parse out the +On the Settings Model side, there will be a new class called `AppearanceConfig`. When we parse out the settings json file, each state-appearance will be stored in an object of this class. Later on, these values get piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the way we already pipe over information such as `FontFace` and `CursorStyle` from the settings model to the app. From 5cf82166cc1392b1ae306a3240382ea9c3716b58 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 30 Nov 2020 16:03:20 -0800 Subject: [PATCH 07/18] inheritance --- doc/specs/Configuration object for profiles.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index a9f9e38ff01..6fc5e16aac6 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -33,8 +33,6 @@ settings json file, each state-appearance will be stored in an object of this cl piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the way we already pipe over information such as `FontFace` and `CursorStyle` from the settings model to the app. -#### Need to talk about inheritance here - ### Allowed parameters For now, these states are meant to be entirely appearance-based. So, not all parameters which can be @@ -45,6 +43,18 @@ would cause a resize in this object.) Here are the list of parameters we will al - `cursorShape` - `backgroundImage` +We may wish to allow further parameters in these objects in the future (like `bellStyle`?). The addition +of further parameters can be discussed in the future and is out of scope for this spec. + +### Inheritance + +We have to decide how we want to deal with the case(s) where not all parameters are defined - i.e. what +values should we use for undefined parameters? The current proposal is as follows: + +If the profile defines an `unfocusedState`, any parameters not explicitly defined within it will adopt +the values from the profile itself. If the profile does not define an `unfocusedState`, then the global/default `unfocusedState` is used +for this profile. + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: From 0e7b57850656d935ad85f25eea91bc328a800273 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 30 Nov 2020 16:07:08 -0800 Subject: [PATCH 08/18] elaboration on inheritance --- doc/specs/Configuration object for profiles.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 6fc5e16aac6..2013adac428 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -55,6 +55,11 @@ If the profile defines an `unfocusedState`, any parameters not explicitly define the values from the profile itself. If the profile does not define an `unfocusedState`, then the global/default `unfocusedState` is used for this profile. +Thus, if a user wishes for the unfocused state to look the same as the focused state for a particular profile, +while still having a global/default unfocused state appearance, they simply need to define an emplty `unfocusedState` +for that profile (similarly, they could define just 1 or 2 parameters if they wish for minimal changes between the focused +and unfocused states for that profile). + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: From 6812b76a11d0b1a9212459d6cf91913311ca5527 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 2 Dec 2020 16:34:43 -0800 Subject: [PATCH 09/18] further details --- .../Configuration object for profiles.md | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 2013adac428..b086a05f26c 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -25,23 +25,33 @@ pane is focused and which panes are unfocused. This change would grant us that f ## Solution Design We will add an new interface in the `TerminalControl` namespace, called `IAppearance`, which defines how -`TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about (such as `FontFace`). +`TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about (such as `CursorShape`). `TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. -On the Settings Model side, there will be a new class called `AppearanceConfig`. When we parse out the -settings json file, each state-appearance will be stored in an object of this class. Later on, these values get -piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the way we already pipe over information -such as `FontFace` and `CursorStyle` from the settings model to the app. +We will also have `IControlSettings` require `IAppearance`. That way, the control `settings` object can +itself also be used as an object that implements `IAppearance`. We do this so we do not need a separate +'focused' configuration object when we wish to switch back to the 'regular' appearance from the unfocused +appearance - we can simply pass in the settings. + +On the Settings Model side, there will be a new interface called `IAppearanceConfig`, which is essentially a +mirror of the `IAppearance` interface described earlier. A new class, `AppearanceConfig`, will implement this +interface and so will `Profile` itself (for the same reason as earlier - so that no new configuration object is +needed for the regular appearance). + +When we parse out the settings json file, each state-appearance will be stored in an object of the `AppearanceConfig` +class. Later on, these values get piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the +similar to the way we already pipe over information such as `FontFace` and `CursorStyle` from the settings +model to the app. ### Allowed parameters For now, these states are meant to be entirely appearance-based. So, not all parameters which can be defined in a `Profile` can be defined in this new object (for example, we do not want parameters which -would cause a resize in this object.) Here are the list of parameters we will allow: +would cause a resize in this object.) Here is the list of parameters we will allow: - Anything regarding colors: `colorScheme`, `foreground`, `background`, `cursorColor` etc +- Anything regarding background image: `path`, `opacity`, `alignment`, `stretchMode` - `cursorShape` -- `backgroundImage` We may wish to allow further parameters in these objects in the future (like `bellStyle`?). The addition of further parameters can be discussed in the future and is out of scope for this spec. @@ -56,9 +66,10 @@ the values from the profile itself. If the profile does not define an `unfocused for this profile. Thus, if a user wishes for the unfocused state to look the same as the focused state for a particular profile, -while still having a global/default unfocused state appearance, they simply need to define an emplty `unfocusedState` +while still having a global/default unfocused state appearance, they simply need to define an empty `unfocusedState` for that profile (similarly, they could define just 1 or 2 parameters if they wish for minimal changes between the focused -and unfocused states for that profile). +and unfocused states for that profile). If they do not define any `unfocusedState` for the profile, then +the global/default one will be used. ## UI/UX Design From ad352dfa27adfa5ffd003137ca33f70a733292c5 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 4 Dec 2020 14:47:20 -0800 Subject: [PATCH 10/18] updates from brownbag --- .../Configuration object for profiles.md | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index b086a05f26c..50de91fa589 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -1,11 +1,11 @@ --- author: created on: <2020-11-20> -last updated: <2020-11-20> +last updated: <2020-12-4> issue id: <#8345> --- -# Configuration object for profiles +# Appearance configuration objects for profiles ## Abstract @@ -22,6 +22,13 @@ Reference: [#3062](https://github.com/microsoft/terminal/issues/3062) Users want there to be a more visible indicator than the one we have currently for which pane is focused and which panes are unfocused. This change would grant us that feature. +## Note: this is going to be an experimental change + +Upon discussion we realized that this feature might cause scaling problems in the future. Thus, we +have decided to make this an experimental change for now - meaning that we are not (at this point) committed +to supporting this feature in the long term. We will decide on the longevity of this feature after obtaining +user feedback from the initial experimental phase. + ## Solution Design We will add an new interface in the `TerminalControl` namespace, called `IAppearance`, which defines how @@ -58,8 +65,8 @@ of further parameters can be discussed in the future and is out of scope for thi ### Inheritance -We have to decide how we want to deal with the case(s) where not all parameters are defined - i.e. what -values should we use for undefined parameters? The current proposal is as follows: +In the case that not all of the allowed parameters are defined in an appearance object, we will obtain the +values for those parameters in the following matter: If the profile defines an `unfocusedState`, any parameters not explicitly defined within it will adopt the values from the profile itself. If the profile does not define an `unfocusedState`, then the global/default `unfocusedState` is used @@ -71,12 +78,21 @@ for that profile (similarly, they could define just 1 or 2 parameters if they wi and unfocused states for that profile). If they do not define any `unfocusedState` for the profile, then the global/default one will be used. +### Multiple states + +At the time of writing this, we have noted two possible 'appearance states' that we should allow: unfocused and elevated. +The question then is, in the case of an unfocused and elevated control, which appearance should we use? + +For now, we will solve this problem by allowing a third state "unfocused elevated". This solution style of combining states +is of course not going to be feasible if we add more states in the future - hence why this feature is going to remain +experimental for now. + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: ``` -"unfocusedState": +"experimental.state.unfocused": { "colorScheme": "Campbell", "cursorColor": "#888", From df039d2326a3edeaa7419ff8990b06ce23da8a6f Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 4 Dec 2020 14:59:33 -0800 Subject: [PATCH 11/18] ICore/ControlAppearance --- doc/specs/Configuration object for profiles.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 50de91fa589..dcd9def4c33 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -31,19 +31,20 @@ user feedback from the initial experimental phase. ## Solution Design -We will add an new interface in the `TerminalControl` namespace, called `IAppearance`, which defines how -`TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about (such as `CursorShape`). -`TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. +We will add new interfaces in the `TerminalControl` namespace, called `IControlAppearance` and `ICoreAppearance`, +which defines how `TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about +(such as `CursorShape`). `TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. -We will also have `IControlSettings` require `IAppearance`. That way, the control `settings` object can -itself also be used as an object that implements `IAppearance`. We do this so we do not need a separate -'focused' configuration object when we wish to switch back to the 'regular' appearance from the unfocused +We will also have `IControlSettings` require `IControlAppearance`, and `ICoreSettings` will require `ICoreAppearance`. +That way, the control's `settings` object can itself also be used as an object that implements both appearance interfaces. We do this so we +do not need a separate 'regular' configuration object when we wish to switch back to the 'regular' appearance from the unfocused appearance - we can simply pass in the settings. On the Settings Model side, there will be a new interface called `IAppearanceConfig`, which is essentially a -mirror of the `IAppearance` interface described earlier. A new class, `AppearanceConfig`, will implement this +combination/mirror of the appearance interfaces described earlier. A new class, `AppearanceConfig`, will implement this interface and so will `Profile` itself (for the same reason as earlier - so that no new configuration object is -needed for the regular appearance). +needed for the regular appearance). We are choosing to have a separate interface on the settings model side to maintain +its separation from the rest of the terminal. When we parse out the settings json file, each state-appearance will be stored in an object of the `AppearanceConfig` class. Later on, these values get piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the From 95ab38cacbc7baaabfbbc4ba7c10fa96702a511d Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Tue, 8 Dec 2020 17:22:02 -0800 Subject: [PATCH 12/18] moved elevated state to future considerations, no longer experimental --- .../Configuration object for profiles.md | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index dcd9def4c33..dc04671c51a 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -11,9 +11,7 @@ issue id: <#8345> This spec outlines how we can support 'configuration objects' in our profiles, which will allow us to render differently depending on the state of the control. For example, a -control can be rendered differently if it's focused as compared to when it's unfocused. Another -example is that an elevated state control can be rendered differently as compared to a -non-elevated one. +control can be rendered differently if it's focused as compared to when it's unfocused. ## Inspiration @@ -22,13 +20,6 @@ Reference: [#3062](https://github.com/microsoft/terminal/issues/3062) Users want there to be a more visible indicator than the one we have currently for which pane is focused and which panes are unfocused. This change would grant us that feature. -## Note: this is going to be an experimental change - -Upon discussion we realized that this feature might cause scaling problems in the future. Thus, we -have decided to make this an experimental change for now - meaning that we are not (at this point) committed -to supporting this feature in the long term. We will decide on the longevity of this feature after obtaining -user feedback from the initial experimental phase. - ## Solution Design We will add new interfaces in the `TerminalControl` namespace, called `IControlAppearance` and `ICoreAppearance`, @@ -79,21 +70,12 @@ for that profile (similarly, they could define just 1 or 2 parameters if they wi and unfocused states for that profile). If they do not define any `unfocusedState` for the profile, then the global/default one will be used. -### Multiple states - -At the time of writing this, we have noted two possible 'appearance states' that we should allow: unfocused and elevated. -The question then is, in the case of an unfocused and elevated control, which appearance should we use? - -For now, we will solve this problem by allowing a third state "unfocused elevated". This solution style of combining states -is of course not going to be feasible if we add more states in the future - hence why this feature is going to remain -experimental for now. - ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: ``` -"experimental.state.unfocused": +"unfocusedConfig": { "colorScheme": "Campbell", "cursorColor": "#888", @@ -134,6 +116,9 @@ does not cause the window to flash/show a jarring indicator that the rendering v We will need to decide how this will look in the settings UI. +We may wish to add more states in the future (like 'elevated'). When that happens, we will need to deal with how +these appearance objects can scale/layer over each other. + ## Resources From 2f18add533c1d114c2c6b8638056fa6ecf91483a Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 9 Dec 2020 16:45:19 -0800 Subject: [PATCH 13/18] 1 more risk, naming consistency, spelling --- .github/actions/spell-check/dictionary/apis.txt | 1 + doc/specs/Configuration object for profiles.md | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/.github/actions/spell-check/dictionary/apis.txt b/.github/actions/spell-check/dictionary/apis.txt index 867d3c831f2..568d499a023 100644 --- a/.github/actions/spell-check/dictionary/apis.txt +++ b/.github/actions/spell-check/dictionary/apis.txt @@ -19,6 +19,7 @@ Hashtable HIGHCONTRASTON HIGHCONTRASTW href +IAppearance IAsync IBind IBox diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index dc04671c51a..a55c52df47b 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -60,16 +60,19 @@ of further parameters can be discussed in the future and is out of scope for thi In the case that not all of the allowed parameters are defined in an appearance object, we will obtain the values for those parameters in the following matter: -If the profile defines an `unfocusedState`, any parameters not explicitly defined within it will adopt -the values from the profile itself. If the profile does not define an `unfocusedState`, then the global/default `unfocusedState` is used +If the profile defines an `unfocusedConfig`, any parameters not explicitly defined within it will adopt +the values from the profile itself. If the profile does not define an `unfocusedConfig`, then the global/default `unfocusedConfig` is used for this profile. Thus, if a user wishes for the unfocused state to look the same as the focused state for a particular profile, -while still having a global/default unfocused state appearance, they simply need to define an empty `unfocusedState` +while still having a global/default unfocused state appearance, they simply need to define an empty `unfocusedConfig` for that profile (similarly, they could define just 1 or 2 parameters if they wish for minimal changes between the focused -and unfocused states for that profile). If they do not define any `unfocusedState` for the profile, then +and unfocused states for that profile). If they do not define any `unfocusedConfig` for the profile, then the global/default one will be used. +If neither the profile nor the global settings have defined an `unfocusedConfig`, then there will be no +appearance change when switching between focused and unfocused states. + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: @@ -112,6 +115,11 @@ Inactive tabs will be 'rendered' in the background with the `UnfocusedRenderingP sure that switching to an inactive tab (and so causing the renderer to update with the 'normal' parameters) does not cause the window to flash/show a jarring indicator that the rendering values changed. +When certain appearance settings are changed via OSC sequences (such as the background color), only the focused/regular +appearance will change and the unfocused one will remain unchanged. This means that even if no unfocused configuration was set +(i.e. the user simply wants the unfocused configuration to look like the regular appearance), there will end up being +a difference between both appearances. + ## Future considerations We will need to decide how this will look in the settings UI. From 744528292eda7195d1eb054792f56c6ea379631f Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 11 Dec 2020 11:01:01 -0800 Subject: [PATCH 14/18] notes on why multiple states is now a future consideration --- doc/specs/Configuration object for profiles.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index a55c52df47b..9ff08439747 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -62,7 +62,7 @@ values for those parameters in the following matter: If the profile defines an `unfocusedConfig`, any parameters not explicitly defined within it will adopt the values from the profile itself. If the profile does not define an `unfocusedConfig`, then the global/default `unfocusedConfig` is used -for this profile. +for this profile (and any undefined parameters within *that* will be set to default values). Thus, if a user wishes for the unfocused state to look the same as the focused state for a particular profile, while still having a global/default unfocused state appearance, they simply need to define an empty `unfocusedConfig` @@ -125,7 +125,12 @@ a difference between both appearances. We will need to decide how this will look in the settings UI. We may wish to add more states in the future (like 'elevated'). When that happens, we will need to deal with how -these appearance objects can scale/layer over each other. +these appearance objects can scale/layer over each other. We had a lot of dicussion about this and could not find +a suitable solution to the problem of multiple states being valid at the same time (like unfocused and elevated). +This, along with the fact that it is uncertain if there even will be more states we would want to add led us to +the conclusion that we should only support the unfocused state for now, and come back to this issue later. If there +are no more states other than unfocused and elevated, we could allow combining them (like having an 'unfocused elevated' state). +If there are more states, we could do the implementation as an extension rather than inherently supporting it. ## Resources From 8a62a44799a8e10a539d044faf8d903899977b07 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 11 Dec 2020 15:00:11 -0800 Subject: [PATCH 15/18] spell --- doc/specs/Configuration object for profiles.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 9ff08439747..c2949413d71 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -125,7 +125,7 @@ a difference between both appearances. We will need to decide how this will look in the settings UI. We may wish to add more states in the future (like 'elevated'). When that happens, we will need to deal with how -these appearance objects can scale/layer over each other. We had a lot of dicussion about this and could not find +these appearance objects can scale/layer over each other. We had a lot of discussion about this and could not find a suitable solution to the problem of multiple states being valid at the same time (like unfocused and elevated). This, along with the fact that it is uncertain if there even will be more states we would want to add led us to the conclusion that we should only support the unfocused state for now, and come back to this issue later. If there From 5048f8f48415bec30a4b395bf794763c7b18eb38 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 14 Dec 2020 14:41:28 -0800 Subject: [PATCH 16/18] additional notes from PR comments --- doc/specs/Configuration object for profiles.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index c2949413d71..65803581136 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -73,6 +73,11 @@ the global/default one will be used. If neither the profile nor the global settings have defined an `unfocusedConfig`, then there will be no appearance change when switching between focused and unfocused states. +This inheritance model can be thought of as an 'all-or-nothing' approach in the sense that the `unfocusedConfig` object +is considered as a *single* setting instead of an object with many settings. We have chosen this model because it is cleaner +and easier to understand than the alternative, where each setting within an `unfocusedConfig` object has a parent from which +it obtains its value. + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: @@ -88,6 +93,9 @@ Users will be able to add a new setting to their profiles that will look like th } ``` +When certain appearance settings are changed via OSC sequences (such as the background color), only the focused/regular +appearance will change and the unfocused one will remain unchanged. + ## Capabilities ### Accessibility @@ -115,11 +123,6 @@ Inactive tabs will be 'rendered' in the background with the `UnfocusedRenderingP sure that switching to an inactive tab (and so causing the renderer to update with the 'normal' parameters) does not cause the window to flash/show a jarring indicator that the rendering values changed. -When certain appearance settings are changed via OSC sequences (such as the background color), only the focused/regular -appearance will change and the unfocused one will remain unchanged. This means that even if no unfocused configuration was set -(i.e. the user simply wants the unfocused configuration to look like the regular appearance), there will end up being -a difference between both appearances. - ## Future considerations We will need to decide how this will look in the settings UI. From df9627b4e4f75db6e2a286404e19443c1543cca8 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Mon, 21 Dec 2020 11:47:15 -0800 Subject: [PATCH 17/18] updates from recent changes --- .../Configuration object for profiles.md | 59 +++++++------------ 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/Configuration object for profiles.md index 65803581136..a5d75c3234c 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/Configuration object for profiles.md @@ -1,7 +1,7 @@ --- author: created on: <2020-11-20> -last updated: <2020-12-4> +last updated: <2020-12-21> issue id: <#8345> --- @@ -22,25 +22,15 @@ pane is focused and which panes are unfocused. This change would grant us that f ## Solution Design -We will add new interfaces in the `TerminalControl` namespace, called `IControlAppearance` and `ICoreAppearance`, -which defines how `TerminalControl` and `TerminalCore` will ask for the rendering settings they need to know about -(such as `CursorShape`). `TerminalApp` will implement this interface through a class called `AppAppearanceConfig`. +The implementation design for appearance config objects centers around the recent change where inheritance was added to the +`TerminalSettings` class in the Terminal Settings Model - i.e. different `TerminalSettings` objects can inherit from each other. +The reason for this change was that we did not want a settings reload to erase any overrides `TermControl` may have made +to the settings during runtime. By instead passing a child of the `TerminalSettings` object to the control, we can change +the parent of the child during a settings reload without the overrides being erased (since those overrides live in the child). -We will also have `IControlSettings` require `IControlAppearance`, and `ICoreSettings` will require `ICoreAppearance`. -That way, the control's `settings` object can itself also be used as an object that implements both appearance interfaces. We do this so we -do not need a separate 'regular' configuration object when we wish to switch back to the 'regular' appearance from the unfocused -appearance - we can simply pass in the settings. - -On the Settings Model side, there will be a new interface called `IAppearanceConfig`, which is essentially a -combination/mirror of the appearance interfaces described earlier. A new class, `AppearanceConfig`, will implement this -interface and so will `Profile` itself (for the same reason as earlier - so that no new configuration object is -needed for the regular appearance). We are choosing to have a separate interface on the settings model side to maintain -its separation from the rest of the terminal. - -When we parse out the settings json file, each state-appearance will be stored in an object of the `AppearanceConfig` -class. Later on, these values get piped over to the `AppAppearanceConfig` objects in `TerminalApp`. This is the -similar to the way we already pipe over information such as `FontFace` and `CursorStyle` from the settings -model to the app. +The idea behind unfocused appearance configurations is similar. We will pass in another `TerminalSettings` object to the control, +which is simply a child that already has some overrides in it. When the control gains or loses focus, it simply switches between +the two settings objects appropriately. ### Allowed parameters @@ -57,27 +47,21 @@ of further parameters can be discussed in the future and is out of scope for thi ### Inheritance -In the case that not all of the allowed parameters are defined in an appearance object, we will obtain the -values for those parameters in the following matter: - -If the profile defines an `unfocusedConfig`, any parameters not explicitly defined within it will adopt -the values from the profile itself. If the profile does not define an `unfocusedConfig`, then the global/default `unfocusedConfig` is used -for this profile (and any undefined parameters within *that* will be set to default values). - -Thus, if a user wishes for the unfocused state to look the same as the focused state for a particular profile, -while still having a global/default unfocused state appearance, they simply need to define an empty `unfocusedConfig` -for that profile (similarly, they could define just 1 or 2 parameters if they wish for minimal changes between the focused -and unfocused states for that profile). If they do not define any `unfocusedConfig` for the profile, then -the global/default one will be used. - -If neither the profile nor the global settings have defined an `unfocusedConfig`, then there will be no -appearance change when switching between focused and unfocused states. - -This inheritance model can be thought of as an 'all-or-nothing' approach in the sense that the `unfocusedConfig` object +The inheritance model can be thought of as an 'all-or-nothing' approach in the sense that the `unfocusedConfig` object is considered as a *single* setting instead of an object with many settings. We have chosen this model because it is cleaner and easier to understand than the alternative, where each setting within an `unfocusedConfig` object has a parent from which it obtains its value. +Note that when `TerminalApp` initializes a control, it creates a `TerminalSettings` object for that profile and passes the +control a child of that object (so that the control can store overrides in the child, as described earlier). If an unfocused +config is defined in the profile (or in globals/profile defaults), then `TerminalApp` will create a *child of that child*, +put the relevant overrides in it, and pass that into the control as well. Thus, the inheritance of any undefined parameters +in the unfocused config will be as follows: + +1. The unfocused config specified in the profile (or in globals/profile defaults) +2. Overrides made by term control +3. The parent profile + ## UI/UX Design Users will be able to add a new setting to their profiles that will look like this: @@ -94,7 +78,8 @@ Users will be able to add a new setting to their profiles that will look like th ``` When certain appearance settings are changed via OSC sequences (such as the background color), only the focused/regular -appearance will change and the unfocused one will remain unchanged. +appearance will change and the unfocused one will remain unchanged. However, since the unfocused settings object inherits +from the regular one, it will still apply the change (provided it does not define its own value for that setting). ## Capabilities From c50df8d6bc694abafaec0eee27344f35fe475a0c Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Fri, 5 Feb 2021 11:26:07 -0800 Subject: [PATCH 18/18] address comments --- ...ance configuration object for profiles.md} | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) rename doc/specs/{Configuration object for profiles.md => #3062 - Appearance configuration object for profiles.md} (89%) diff --git a/doc/specs/Configuration object for profiles.md b/doc/specs/#3062 - Appearance configuration object for profiles.md similarity index 89% rename from doc/specs/Configuration object for profiles.md rename to doc/specs/#3062 - Appearance configuration object for profiles.md index a5d75c3234c..b15d7a5ee2e 100644 --- a/doc/specs/Configuration object for profiles.md +++ b/doc/specs/#3062 - Appearance configuration object for profiles.md @@ -1,8 +1,8 @@ --- -author: -created on: <2020-11-20> -last updated: <2020-12-21> -issue id: <#8345> +author: Pankaj Bhojwani, pabhojwa@microsoft.com +created on: 2020-11-20 +last updated: 2021-2-5 +issue id: #8345 --- # Appearance configuration objects for profiles @@ -47,9 +47,9 @@ of further parameters can be discussed in the future and is out of scope for thi ### Inheritance -The inheritance model can be thought of as an 'all-or-nothing' approach in the sense that the `unfocusedConfig` object +The inheritance model can be thought of as an 'all-or-nothing' approach in the sense that the `unfocusedAppearance` object is considered as a *single* setting instead of an object with many settings. We have chosen this model because it is cleaner -and easier to understand than the alternative, where each setting within an `unfocusedConfig` object has a parent from which +and easier to understand than the alternative, where each setting within an `unfocusedAppearance` object has a parent from which it obtains its value. Note that when `TerminalApp` initializes a control, it creates a `TerminalSettings` object for that profile and passes the @@ -59,7 +59,7 @@ put the relevant overrides in it, and pass that into the control as well. Thus, in the unfocused config will be as follows: 1. The unfocused config specified in the profile (or in globals/profile defaults) -2. Overrides made by term control +2. Overrides made by the terminal control 3. The parent profile ## UI/UX Design @@ -67,7 +67,7 @@ in the unfocused config will be as follows: Users will be able to add a new setting to their profiles that will look like this: ``` -"unfocusedConfig": +"unfocusedAppearance": { "colorScheme": "Campbell", "cursorColor": "#888", @@ -102,6 +102,9 @@ Should not affect compatibility. ### Performance, Power, and Efficiency +Rapidly switching between many panes, causing several successive appearance changes in a short period of time, could +potentially impact performance. However, regular/reasonable pane switching should not have a noticeable effect. + ## Potential Issues Inactive tabs will be 'rendered' in the background with the `UnfocusedRenderingParams` object, we need to make