Add keyboard shortcuts to increase and decrease font size #2700

dsafa · 2019-09-09T05:30:06Z

Summary of the Pull Request

This hooks up the already existing shortcut actions for increasing and decreasing font size so that keyboard shortcuts can be used. Also adds default shortcuts ctrl+= and ctrl+- for increasing and decreasing font size.

References

A previous PR was done for this feature but was closed and never completed: #1740 . This is a continuation of that work.

PR Checklist

Closes Add keyboard shortcuts to increase and decrease font size (or zoom) #1708
CLA signed. If not, go over here and sign the CLA
Tests added/passed. No new tests added
Requires documentation to be 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

There were already actions increaseFontSize and decreaseFontSize that existed but without anything to handle them. The handler shares existing functionality for adjusting font size that already exists with ctrl and scrolling.

Validation Steps Performed

Add keybindings to profiles.json for decreaseFontSize and increaseFontSize
Verified that both keybindings work.
Verified that ctrl + scrolling still works.

~~1. Delete profiles.json and let terminal generate a default one.~~
~~2. Verified that the default keybindings are created.~~ No longer needed with defaults.json

src/cascadia/TerminalApp/AppKeyBindings.cpp

dsafa · 2019-09-09T22:24:56Z

I've combined the IncreaseFontSize and DecreaseFontSize events into a AdjustFontSize event and added a AdjustfontSizeArgs that contains the delta.

mdtauk · 2019-09-10T16:26:14Z

Does the feature get called Font Size, or would it be Zoom?

dsafa · 2019-09-10T18:26:08Z

Thats a good question. I just left it as font size since that is how zooming works for the terminal control. I know there's future plans to possibly support non terminal panes so maybe zoom would be a better term.

zadjii-msft

I'm a tiny bit worried about the choice of default keybinding here - ctrl+shift+plus/ctrl+shift+minus might be better.

Otherwise, lets just get the merge fixed and lets

zadjii-msft · 2019-09-17T15:54:57Z

src/cascadia/TerminalApp/CascadiaSettings.cpp

@@ -398,6 +398,13 @@ void CascadiaSettings::_CreateDefaultKeybindings()
    keyBindings.SetKeyBinding(ShortcutAction::SwitchToTab8,
                              KeyChord{ KeyModifiers::Alt | KeyModifiers::Ctrl,
                                        static_cast<int>('9') });
+
+    keyBindings.SetKeyBinding(ShortcutAction::IncreaseFontSize,


My only reservation is that as of #2515, the default keybindings aren't set here anymore. They should instead be included in src\cascadia\TerminalApp\defaults.json

Interesting conundrum here @zadjii-msft. To adjust text size / zoom using the mouse is ctrl + scroll. It would therefore, make sense if zoom via the keyboard was also ctrl + +/-.

What would also make THAT even more consistent with mouse scroll actions was if also ctrl + shift + +/- adjusted blur, but I may be pushing my luck here ... or am I? 😉

So, it looks like there's no VT encoding for Ctrl-+, so it's free to be bound! Problem, though: Ctrl+- emits ^_.
We also do need to be considerate of the fact that + is Shift+-...
Perhaps we should follow suit with other zoomable things and make Ctrl+= the "zoom in" binding?

Interesting. Ctrl+Shift+- emits ^_ in putty and conhost, but Ctrl+- only emits ^_ in putty. Perhaps the right behavior is that Ctrl alone doesn't do anything and is free to be bound for both?

So, like this then?

Key Chord Keys + Mouse Action

ctrl + =/- ctrl + mouse scroll up/down Zoom in/out

ctrl+shift + +/_ ctrl + shift + mouse scroll up/down Blur more/less

I've added the bindings to defaults.json as

{ "command": "decreaseFontSize", "keys": ["ctrl+-"] }, { "command": "increaseFontSize", "keys": ["ctrl+="] }

If a different default is decided I'll be happy to change it.

DHowett-MSFT

A comment indicating the requested changes.

zadjii-msft

I don't want to block on a simple tabs-> spaces thing, but this is the only nit preventing me from signing off.

zadjii-msft · 2019-09-18T13:16:43Z

src/cascadia/TerminalApp/defaults.json

@@ -209,6 +209,8 @@
        { "command": "switchToTab5", "keys": ["ctrl+alt+6"] },
        { "command": "switchToTab6", "keys": ["ctrl+alt+7"] },
        { "command": "switchToTab7", "keys": ["ctrl+alt+8"] },
-        { "command": "switchToTab8", "keys": ["ctrl+alt+9"] }
+        { "command": "switchToTab8", "keys": ["ctrl+alt+9"] },
+		{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },


Suggested change

{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },

{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },

Good catch. Shouldn't have to tell me this 😅. Fixed.

zadjii-msft · 2019-09-18T13:16:54Z

src/cascadia/TerminalApp/defaults.json

-        { "command": "switchToTab8", "keys": ["ctrl+alt+9"] }
+        { "command": "switchToTab8", "keys": ["ctrl+alt+9"] },
+		{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },
+		{ "command": "increaseFontSize", "keys": ["ctrl+="] }


Suggested change

{ "command": "increaseFontSize", "keys": ["ctrl+="] }

{ "command": "increaseFontSize", "keys": ["ctrl+="] }

andres-pcg · 2019-09-29T07:19:19Z

So, is this ready to be merge?

bitcrazed · 2019-10-01T20:09:57Z

@dsafa - Thanks soooo much for implementing this feature :) Been wanting to get around to doing this myself - glad you got there first 😁

)" This reverts commit 083be43.

ghost · 2019-10-23T19:55:45Z

🎉Windows Terminal Preview v0.6.2951.0 has been released which incorporates this pull request.:tada:

Handy links:

Hook up font size key bindings and events

eace996

dsafa commented Sep 9, 2019

View reviewed changes

src/cascadia/TerminalApp/AppKeyBindings.cpp Outdated Show resolved Hide resolved

Combine increase and decrease font size events

e0c932b

carlos-zamora approved these changes Sep 10, 2019

View reviewed changes

carlos-zamora requested a review from a team September 12, 2019 20:58

carlos-zamora mentioned this pull request Sep 12, 2019

Add profiles.json schema to doc folder #2704

Merged

5 tasks

zadjii-msft reviewed Sep 17, 2019

View reviewed changes

DHowett-MSFT reviewed Sep 17, 2019

View reviewed changes

dsafa added 2 commits September 17, 2019 16:19

Merge branch 'master' into add-fontsize-update-bindings

9b89f15

Add zoom keybindings to defaults.json

21f4886

zadjii-msft reviewed Sep 18, 2019

View reviewed changes

Fix whitespace

e48c651

zadjii-msft approved these changes Sep 18, 2019

View reviewed changes

zadjii-msft merged commit 083be43 into microsoft:master Sep 30, 2019

DHowett-MSFT pushed a commit that referenced this pull request Oct 3, 2019

Revert "Add keyboard shortcuts to increase and decrease font size (#2700

8d19a34

)" This reverts commit 083be43.

ghost mentioned this pull request Oct 23, 2019

Add keyboard shortcuts to increase and decrease font size (or zoom) #1708

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add keyboard shortcuts to increase and decrease font size #2700

Add keyboard shortcuts to increase and decrease font size #2700

dsafa commented Sep 9, 2019 •

edited

Loading

dsafa commented Sep 9, 2019

mdtauk commented Sep 10, 2019

dsafa commented Sep 10, 2019

zadjii-msft left a comment

zadjii-msft Sep 17, 2019

bitcrazed Sep 17, 2019

DHowett-MSFT Sep 17, 2019

DHowett-MSFT Sep 17, 2019

bitcrazed Sep 17, 2019

dsafa Sep 18, 2019

DHowett-MSFT left a comment

zadjii-msft left a comment

zadjii-msft Sep 18, 2019

dsafa Sep 18, 2019

zadjii-msft Sep 18, 2019

andres-pcg commented Sep 29, 2019

bitcrazed commented Oct 1, 2019

ghost commented Oct 23, 2019

Key Chord	Keys + Mouse	Action
`ctrl` + `=`/`-`	`ctrl` + mouse scroll up/down	Zoom in/out
`ctrl`+`shift` + `+`/`_`	`ctrl` + `shift` + mouse scroll up/down	Blur more/less

	{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },
	{ "command": "decreaseFontSize", "keys": ["ctrl+-"] },

	{ "command": "increaseFontSize", "keys": ["ctrl+="] }
	{ "command": "increaseFontSize", "keys": ["ctrl+="] }

Add keyboard shortcuts to increase and decrease font size #2700

Add keyboard shortcuts to increase and decrease font size #2700

Conversation

dsafa commented Sep 9, 2019 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

dsafa commented Sep 9, 2019

mdtauk commented Sep 10, 2019

dsafa commented Sep 10, 2019

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT left a 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

andres-pcg commented Sep 29, 2019

bitcrazed commented Oct 1, 2019

ghost commented Oct 23, 2019

dsafa commented Sep 9, 2019 •

edited

Loading