-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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 property to control dropdown speed of global summon #9977
Changes from all commits
3bef7bb
1f52d35
5a9cdc8
36539cf
27ace16
9a41647
5cabcfb
03bfc6e
590b9ff
0579b24
a3faed6
9c6eac4
0103331
b4fe1bf
d08e65c
e101efd
e1402d8
f978a9c
5939636
c088895
fa2df47
0f5c24f
921d915
00184e7
658db6b
977db46
bcbef34
813dbc6
0f0df5e
9fc2f0e
6537686
a75da0a
c02f25a
3e39ab9
52b2cb6
bc492f1
88ffc6f
be74b2e
2a7bc94
81c09d9
c34e4ce
5b8ace2
689c385
59deca1
f02969b
b2db317
d2a3438
a65f341
1c2f8e5
71f6b58
18d1a20
a41bee6
6e7ea61
18099d2
3e5d927
91b52d4
848682a
1dcb4cb
fee6473
342d3f2
5052d31
eff18d1
5c039ea
10779ca
25b31ff
7c2a514
ac8fef0
1fdb6b1
784ec73
b20222f
9d76c62
2a2f5cb
71577fc
f892752
4166afa
717db81
1b2e7a7
305e627
9f9eacb
f71c948
51617cf
2eec961
15a8a9c
8370789
4eb1d3a
c215a97
26d5194
91372ea
045a8db
c95a429
b5e0f2d
6c4238b
0abd963
643b860
84c1bd9
24bfbe6
4c2e6d2
aec2561
22036c2
173720f
b607a55
63b3658
5940f05
06cb41b
783be75
6c424f7
387d675
f190576
960e1fa
caeb17f
2d7ed8d
f4c9993
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ HIGHCONTRASTON | |
HIGHCONTRASTW | ||
hotkeys | ||
href | ||
hrgn | ||
IActivation | ||
IApp | ||
IAppearance | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -591,7 +591,7 @@ void AppHost::_DispatchCommandline(winrt::Windows::Foundation::IInspectable /*se | |
{ | ||
// Summon the window whenever we dispatch a commandline to it. This will | ||
// make it obvious when a new tab/pane is created in a window. | ||
_window->SummonWindow(false); | ||
_window->SummonWindow(false, 0); | ||
_logic.ExecuteCommandline(args.Commandline(), args.CurrentDirectory()); | ||
} | ||
|
||
|
@@ -693,6 +693,7 @@ void AppHost::_GlobalHotkeyPressed(const long hotkeyIndex) | |
args.OnCurrentDesktop(summonArgs.Desktop() == Settings::Model::DesktopBehavior::OnCurrent); | ||
args.SummonBehavior().MoveToCurrentDesktop(summonArgs.Desktop() == Settings::Model::DesktopBehavior::ToCurrent); | ||
args.SummonBehavior().ToggleVisibility(summonArgs.ToggleVisibility()); | ||
args.SummonBehavior().DropdownDuration(summonArgs.DropdownDuration()); | ||
|
||
_windowManager.SummonWindow(args); | ||
if (args.FoundMatch()) | ||
|
@@ -775,7 +776,7 @@ bool AppHost::_LazyLoadDesktopManager() | |
void AppHost::_HandleSummon(const winrt::Windows::Foundation::IInspectable& /*sender*/, | ||
const Remoting::SummonWindowBehavior& args) | ||
{ | ||
_window->SummonWindow(args.ToggleVisibility()); | ||
_window->SummonWindow(args.ToggleVisibility(), args.DropdownDuration()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason not to just pass down the whole There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose not really There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do not make IslandWindow more specific to Terminal. I'd prefer it to be less specific to Terminal IMO 😄 |
||
|
||
if (args != nullptr && args.MoveToCurrentDesktop()) | ||
{ | ||
|
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.
Can we make the default value something nice to make it look relatively modern? I assume '0' means that it'll just pop up immediately and not animate.
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.
Yea so now we're getting into the subjective realm - IMO the normal min/maximize behavior looks better than the dropdown. The animation seems more gimmicky - good for literally "quake mode", but seems more annoying for the others.
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 gonna tap in @DHowett and @cinnamon-msft, since:
I'd feel more comfortable making dropdown the default if the whole window could animate. But since it can't then this almost feels more experimental. That's why I'm thinking we should leave the default
0
. It'll still play the normal Windows "minimize" and "restore" animations that a window would usually get with"dropdownDuration": 0
.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 since the animation is requested, we can put it in for now. Then we can test it/play with it and if we don't like it we can just set it back to 0 in a follow up before 1.9?
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.
lol