Control 'Touch Keyboard and Handwriting Panel Service' warning #9015

WVVxm · 2021-02-03T01:42:14Z

Summary of the Pull Request

Add a setting to turn off the warning if 'Touch Keyboard and Handwriting Panel Service' is disabled.

References

PR Checklist

Closes #xxx
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
Schema updated.
I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

The service might not start in some case, and it doesn't affect the input in some computer.
This PR turn off the warning even if the service is disabled.
The setting name is "inputServiceWarning".

part of settings.json:

// If enabled, selections are automatically copied to your clipboard.
"copyOnSelect": false,

// If disabled, don't show warning if 'Touch Keyboard and Handwriting Panel Service' disabled.
"inputServiceWarning": false,

Validation Steps Performed

I manually set the service to "Disabled", restarted the Terminal, verified the warning up, then set "inputServiceWarning" to false and restarted the Terminal, and the warning didn't appear.

…rd and Handwriting Panel Service' is disabled.

DHowett · 2021-02-05T07:56:38Z

Hey, thanks for writing this up! I'm not as averse to this setting as I was in the linked issues, but we'll need a little bit to review this. It's a busy week for the team -- since 1.6 just came out, we have a bit of a backlog. Thanks for understanding!

Don-Vito

I am neither an expert nor from the team, but besides few nits below the change looks good to me.

Probably the next missing pieces are

Adding this to Settings UI
Modifying the documentation.

However before you do it, probably it worth understanding from the team what are their plans around this feature and #8771 that introduces application state. Once that PR lands, we can introduce a UX that will allow the user to "silence" this warning forever, without editing the settings (e.g., by adding "never show this warning again" to the bar).

Don-Vito · 2021-02-07T19:20:02Z

doc/cascadia/profiles.schema.json

@@ -629,6 +629,11 @@
          "description": "When set to true, tabs are always displayed. When set to false and \"showTabsInTitlebar\" is set to false, tabs only appear after opening a new tab.",
          "type": "boolean"
        },
+        "inputServiceWarning": {


Nit: consider renaming into "showInputServiceWarning" (here and in other places)

I'm gonna say that this one is probably okay as is. The other warnings are largePasteWarning and multiLinePasteWarning, so this is actually consistent.

Don-Vito · 2021-02-07T19:21:30Z

src/cascadia/TerminalApp/AppLogic.cpp

@@ -487,11 +487,15 @@ namespace winrt::TerminalApp::implementation
    void AppLogic::_OnLoaded(const IInspectable& /*sender*/,
                             const RoutedEventArgs& /*eventArgs*/)
    {
-        const auto keyboardServiceIsDisabled = !_IsKeyboardServiceEnabled();
-        if (keyboardServiceIsDisabled)
+        if (_settings.GlobalSettings().InputServiceWarning())


Nit: consider unifying into a single if statement with &&

Don-Vito · 2021-02-07T19:26:15Z

src/cascadia/TerminalSettingsModel/userDefaults.json

@@ -16,6 +16,9 @@
    // If enabled, selections are automatically copied to your clipboard.
    "copyOnSelect": false,

+    // If disabled, don't show warning if 'Touch Keyboard and Handwriting Panel Service' disabled.


Not sure if we want to add it here.

I concur, we should remove these lines from this file. This file is intentionally fairly bare.

zadjii-msft

Alright so I'm blocking over removing the lines from IControlSettings and from userDefaults.json, but otherwise this looks fine to me. The other warnings aren't expressed in the settings UI quite yet, so I wouldn't worry about this one either.

zadjii-msft · 2021-02-08T22:31:44Z

doc/cascadia/profiles.schema.json

@@ -629,6 +629,11 @@
          "description": "When set to true, tabs are always displayed. When set to false and \"showTabsInTitlebar\" is set to false, tabs only appear after opening a new tab.",
          "type": "boolean"
        },
+        "inputServiceWarning": {


I'm gonna say that this one is probably okay as is. The other warnings are largePasteWarning and multiLinePasteWarning, so this is actually consistent.

zadjii-msft · 2021-02-08T22:33:10Z

src/cascadia/TerminalControl/IControlSettings.idl

@@ -39,6 +39,7 @@ namespace Microsoft.Terminal.TerminalControl
        Microsoft.Terminal.TerminalControl.IKeyBindings KeyBindings;

        Boolean CopyOnSelect;
+        Boolean InputServiceWarning;


This doesn't actually need to be here. We don't need this setting down in the control layer, so we can omit it here.

zadjii-msft · 2021-02-08T22:34:07Z

src/cascadia/TerminalSettingsModel/userDefaults.json

@@ -16,6 +16,9 @@
    // If enabled, selections are automatically copied to your clipboard.
    "copyOnSelect": false,

+    // If disabled, don't show warning if 'Touch Keyboard and Handwriting Panel Service' disabled.


I concur, we should remove these lines from this file. This file is intentionally fairly bare.

ghost · 2021-02-16T00:00:10Z

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

src/cascadia/TerminalSettingsModel/defaults-universal.json

src/cascadia/TerminalSettingsModel/defaults.json

DHowett · 2021-02-22T20:08:55Z

Thank you for your contribution!

ghost · 2021-03-01T18:03:22Z

🎉Windows Terminal Preview v1.7.572.0 has been released which incorporates this pull request.:tada:

Handy links:

add setting "inputServiceWarning" to control warning if 'Touch Keyboa…

ad341d2

…rd and Handwriting Panel Service' is disabled.

zadjii-msft assigned DHowett Feb 3, 2021

Don-Vito reviewed Feb 7, 2021

View reviewed changes

zadjii-msft requested changes Feb 8, 2021

View reviewed changes

ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 8, 2021

ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Feb 16, 2021

zadjii-msft assigned WVVxm Feb 16, 2021

ghost removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Feb 16, 2021

WVVxm requested a review from zadjii-msft February 22, 2021 00:37

ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Feb 22, 2021

WVVxm force-pushed the input-service-warning branch from 7058889 to ad341d2 Compare February 22, 2021 01:28

WVVxm added 2 commits February 22, 2021 10:28

merge upstream

c481d75

remove 'inputServiceWarning' from userDefaults.json

cb06a9b

zadjii-msft approved these changes Feb 22, 2021

View reviewed changes

zadjii-msft unassigned WVVxm Feb 22, 2021

DHowett approved these changes Feb 22, 2021

View reviewed changes

src/cascadia/TerminalSettingsModel/defaults-universal.json Outdated Show resolved Hide resolved

src/cascadia/TerminalSettingsModel/defaults.json Outdated Show resolved Hide resolved

Apply suggestions from code review

bac3db5

DHowett approved these changes Feb 22, 2021

View reviewed changes

DHowett merged commit 8ad4d1f into microsoft:main Feb 22, 2021

zadjii-msft mentioned this pull request Jul 7, 2021

Disable "Touch keyboard and Handwriting Panel Service" warning #10571

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Control 'Touch Keyboard and Handwriting Panel Service' warning #9015

Control 'Touch Keyboard and Handwriting Panel Service' warning #9015

WVVxm commented Feb 3, 2021

DHowett commented Feb 5, 2021

Don-Vito left a comment

Don-Vito Feb 7, 2021

zadjii-msft Feb 8, 2021

Don-Vito Feb 7, 2021

Don-Vito Feb 7, 2021

zadjii-msft Feb 8, 2021

zadjii-msft left a comment

zadjii-msft Feb 8, 2021

zadjii-msft Feb 8, 2021

zadjii-msft Feb 8, 2021

ghost commented Feb 16, 2021

DHowett commented Feb 22, 2021

ghost commented Mar 1, 2021

Control 'Touch Keyboard and Handwriting Panel Service' warning #9015

Control 'Touch Keyboard and Handwriting Panel Service' warning #9015

Conversation

WVVxm commented Feb 3, 2021

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

DHowett commented Feb 5, 2021

Don-Vito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Feb 16, 2021

DHowett commented Feb 22, 2021

ghost commented Mar 1, 2021