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 a snippets pane #17330

Merged
merged 30 commits into from
Jul 8, 2024
Merged

Add a snippets pane #17330

merged 30 commits into from
Jul 8, 2024

Conversation

zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft commented May 28, 2024

This adds a snippets pane, which can be a static pane with all your snippets (sendInput actions) in it. (See #17329)

This pane has a treeview with these actions in it, that we can filter with a textbox at the top.

Play buttons next to entries make it quick to run the command you found.

Bound in the default actions with

        { "command": { "action": "splitPane", "type": "snippets" }, "id": "Terminal.OpenSnippetsPane", "name": { "key": "SnippetsPaneCommandName" } },

re: #1595


TODO, from 06-04 bug bash

  • Snippets pane doesn't display some "no snippets found" text if there aren't any yet
  • open snippets pane; find a "send input"; click the play button on it; input is sent to active pane; begin typing
  • I can open an infinite amount of suggestions panes
    • I'm closing this as by-design for now at least. Nothing stopping anyone from opening infinite of any kind of pane.
    • This would require kind of a lot of refactoring in this PR to mark a kind of pane as being a singleton or singleton-per-tab
    • Okay everyone hates infinite suggestions panes, so I got rid of that
  • Ctrl+Shift+W should still work in the snippets pane even if focus isn't in textbox
  • open snippets pane; click on text box; press TAB key;
    • If you press TAB again, I have no idea where focus went
  • some previews don't work. Like ^c ("input": "\u0003")
  • nested items just give you a bit of extra space for no reason and it looks a little awkward
  • UI Suggestion: add padding on the right side
  • [Accessibility] Narrator says "Clear buffer; Suggestions found 132" when you open the snippets pane
    • Note: this is probably Narrator reading out the command palette (since that's where I opened it from)
    • We should probably expect something like "Snippets", then (assuming focus is thrown into text box) "Type to filter snippets" or something like that

commit 727b051
Merge: 37b4b7b 7c4dfff
Author: Mike Griese <[email protected]>
Date:   Thu May 2 16:27:09 2024 -0500

    Merge branch 'dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit 7c4dfff
Merge: a0e014f 67ae9f6
Author: Mike Griese <[email protected]>
Date:   Wed Apr 3 11:03:46 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/f/sui-panes

commit a0e014f
Merge: e05b2bb 2bcbe6b
Author: Mike Griese <[email protected]>
Date:   Wed Apr 3 10:13:40 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/f/sui-panes

commit e05b2bb
Merge: ef560bf 75dea24
Author: Mike Griese <[email protected]>
Date:   Fri Mar 29 15:13:01 2024 -0500

    Merge branch 'main' into dev/migrie/f/sui-panes

commit ef560bf
Merge: 10e1e46 7243d22
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 13:56:21 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 7243d22
Merge: 1d20599 501522d
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 13:55:54 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/scratchpad-pane

commit 10e1e46
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 13:55:11 2024 -0500

    ALSO doesn't really need to be projected

commit ddc88c8
Merge: 216cc3f 1d20599
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 13:26:16 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 1d20599
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 11:38:47 2024 -0500

    un fix this file

commit b6e4b62
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 11:37:35 2024 -0500

    Doesn't really need to be projected

commit 0979cd6
Merge: d417934 61e952c
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 11:22:45 2024 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 61e952c
Merge: df73d75 08dc346
Author: Mike Griese <[email protected]>
Date:   Tue Mar 26 11:22:27 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 37b4b7b
Author: Mike Griese <[email protected]>
Date:   Fri Mar 22 06:54:29 2024 -0500

    fix builds

commit 6576f94
Merge: 66878d2 216cc3f
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 14:02:45 2024 -0500

    Merge branch 'dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit 216cc3f
Merge: 77022e9 d417934
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 14:02:24 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit d417934
Merge: f1ab16e df73d75
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:51:52 2024 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit df73d75
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:51:34 2024 -0500

    derp

commit 66878d2
Merge: 7b84041 77022e9
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:21:51 2024 -0500

    Merge branch 'dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit 77022e9
Merge: bcceb85 f1ab16e
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:17:20 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit f1ab16e
Merge: e0bb840 2083b2f
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:16:08 2024 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 2083b2f
Merge: 52970ef b9a0cae
Author: Mike Griese <[email protected]>
Date:   Thu Mar 21 13:14:32 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 7b84041
Merge: 8f2a129 bcceb85
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 09:38:28 2024 -0500

    Merge branch 'dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit bcceb85
Merge: 352e0a2 e0bb840
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 09:20:16 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 352e0a2
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 09:14:28 2024 -0500

    fix settings pane for merge

    (cherry picked from commit 0c6a353)

commit e0bb840
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 09:13:54 2024 -0500

    Fix scratch pane for merge

    (cherry picked from commit 591080d)

commit 8f2a129
Merge: 5b4747f c8d0c0a
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 06:50:28 2024 -0500

    Merge branch 'dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit c8d0c0a
Merge: 863840e 2357653
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 06:40:43 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 2357653
Merge: 1951f30 52970ef
Author: Mike Griese <[email protected]>
Date:   Wed Mar 20 06:40:04 2024 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 52970ef
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 16:30:00 2024 -0500

    RegisterBigTimeEncapsulationViolatingTerminalPaneContentEvents

commit 826fc08
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 15:50:58 2024 -0500

    hey there buddy, did you get lost?

commit a7533fa
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 15:15:08 2024 -0500

    eh these events are from pane content anyways!

commit 052dc78
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 14:54:30 2024 -0500

    more nits

commit fd8b083
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 13:55:11 2024 -0500

    get rid of this file

commit 6789ec0
Author: Mike Griese <[email protected]>
Date:   Tue Mar 19 13:34:35 2024 -0500

    some of the easier nits

commit 863840e
Merge: 978fd6e 1951f30
Author: Mike Griese <[email protected]>
Date:   Mon Mar 18 13:20:43 2024 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 1951f30
Merge: 35651bc 524d658
Author: Mike Griese <[email protected]>
Date:   Mon Mar 18 13:20:26 2024 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 524d658
Author: Mike Griese <[email protected]>
Date:   Fri Mar 15 12:01:42 2024 -0500

    GREAT-GREAT-GRANDPARENT: Hey when a pane wants to get closed, we should close it

commit ef775a8
Merge: a3fbc64 287422b
Author: Mike Griese <[email protected]>
Date:   Mon Mar 18 13:19:26 2024 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 5b4747f
Merge: 14a00c8 978fd6e
Author: Mike Griese <[email protected]>
Date:   Thu Mar 14 14:10:21 2024 -0500

    Merge remote-tracking branch 'origin/dev/migrie/f/sui-panes' into dev/migrie/fhl/tasks-pane

commit 978fd6e
Merge: b6254f8 35651bc
Author: Mike Griese <[email protected]>
Date:   Fri Mar 8 10:36:17 2024 -0600

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 35651bc
Merge: 25a8851 a3fbc64
Author: Mike Griese <[email protected]>
Date:   Fri Mar 8 10:33:31 2024 -0600

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit a3fbc64
Merge: de5f7af 8a1e8ac
Author: Mike Griese <[email protected]>
Date:   Fri Mar 8 10:33:00 2024 -0600

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit b6254f8
Author: Mike Griese <[email protected]>
Date:   Thu Mar 7 09:49:48 2024 -0600

    GREAT-GRANDPARENT: This fixes a crash in parent pane selection

    (cherry picked from commit 91a0d0e)

commit 4d47cd5
Author: Mike Griese <[email protected]>
Date:   Mon Mar 4 16:34:36 2024 -0600

    cleanup

commit 0a11643
Author: Mike Griese <[email protected]>
Date:   Mon Mar 4 16:30:16 2024 -0600

    sanely pass around a cache instead of... whatever that was.

commit 17075d6
Merge: 092b355 25a8851
Author: Mike Griese <[email protected]>
Date:   Wed Feb 28 11:50:29 2024 -0600

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 25a8851
Merge: c244633 de5f7af
Author: Mike Griese <[email protected]>
Date:   Wed Feb 28 11:36:02 2024 -0600

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit de5f7af
Merge: 3982358 94e74d2
Author: Mike Griese <[email protected]>
Date:   Wed Feb 28 11:32:40 2024 -0600

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 14a00c8
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 12:14:47 2024 -0600

    lots of cleanup

commit 02a5593
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 12:07:45 2024 -0600

    this is amazing, I'm amazing, everything is awesome

commit 2960476
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 11:39:58 2024 -0600

    holy fuck this is better than I imagined a week ago

commit 9300647
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 11:20:33 2024 -0600

    turns out Visibility works exactly like you'd want for a TreeView

commit b706a6d
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 11:01:36 2024 -0600

    yes yes subclasses are hard in C

commit 17d70f5
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 10:57:05 2024 -0600

    weird but works surprisingly well

commit 69cb545
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 10:16:41 2024 -0600

    Blindly, what if TaskViewModel was a FilteredCommand?

commit def6630
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 09:32:50 2024 -0600

    I wanted to start adding a filter box, but that got tricky

commit d86bd57
Merge: 01ca03f afa7a24
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 09:18:46 2024 -0600

    Merge branch 'dev/migrie/fhl/tasks-pane' of https://github.com/microsoft/terminal into dev/migrie/fhl/tasks-pane

commit 01ca03f
Author: Mike Griese <[email protected]>
Date:   Wed Feb 21 09:17:44 2024 -0600

    Don't generate names with visualized SPC and BS for the sxnui

    You know, this doesn't really help right now, but it should fix #16577,
    and maybe help with #16578 (but not the command palette part)

commit c94c00b
Author: Mike Griese <[email protected]>
Date:   Tue Feb 20 16:12:12 2024 -0600

    more better styling

commit afa7a24
Author: Mike Griese <[email protected]>
Date:   Tue Feb 20 16:12:12 2024 -0600

    more better styling

commit db427f2
Author: Mike Griese <[email protected]>
Date:   Tue Feb 20 16:11:34 2024 -0600

    better styling

commit 2f3ecf1
Author: Mike Griese <[email protected]>
Date:   Tue Feb 20 15:41:10 2024 -0600

    plumb the action through to the task pane

commit d7a6b18
Author: Mike Griese <[email protected]>
Date:   Tue Feb 20 09:28:42 2024 -0600

    re-add the visibility hack; add a play button that does nothing

commit 365c068
Author: Mike Griese <[email protected]>
Date:   Mon Feb 19 12:42:24 2024 -0600

    this fixes that random item reuse problem

commit 8418d6a
Author: Mike Griese <[email protected]>
Date:   Fri Feb 16 15:54:21 2024 -0600

    omg bind to the IsSelected

commit 0bb13f9
Author: Mike Griese <[email protected]>
Date:   Fri Feb 16 09:23:50 2024 -0600

    real xaml

commit e72b1bf
Author: Mike Griese <[email protected]>
Date:   Fri Feb 16 08:40:29 2024 -0600

    nested commands are easy guys

commit 81b35ff
Author: Mike Griese <[email protected]>
Date:   Fri Feb 16 08:32:30 2024 -0600

    this worked surprisingly quickly

commit 092b355
Merge: 389ba20 c244633
Author: Mike Griese <[email protected]>
Date:   Thu Feb 8 09:47:33 2024 -0600

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit c244633
Merge: 7bc1457 3982358
Author: Mike Griese <[email protected]>
Date:   Thu Feb 8 09:46:46 2024 -0600

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 3982358
Merge: 0d528f8 71c35cf
Author: Mike Griese <[email protected]>
Date:   Thu Feb 8 09:40:19 2024 -0600

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 0d528f8
Merge: 6bc711d 92f9ff9
Author: Mike Griese <[email protected]>
Date:   Fri Jan 19 16:25:31 2024 -0600

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 6bc711d
Author: Mike Griese <[email protected]>
Date:   Wed Nov 8 11:10:58 2023 -0600

    maybe I'm not that good at coding

commit f622d80
Merge: 4cec7e9 077d63e
Author: Mike Griese <[email protected]>
Date:   Wed Nov 8 05:55:27 2023 -0600

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 4cec7e9
Author: Mike Griese <[email protected]>
Date:   Mon Nov 6 06:01:55 2023 -0600

    try to remove a few of these but ultimately, eh

commit cf920e7
Merge: 58e8f3c 0289cb0
Author: Mike Griese <[email protected]>
Date:   Thu Nov 2 06:13:22 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 389ba20
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 14:41:57 2023 -0500

    spel

commit dd8606f
Merge: fb74fc8 7bc1457
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 11:04:43 2023 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 7bc1457
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 11:03:41 2023 -0500

    nits and such

commit e9e04d4
Merge: b49997b 58e8f3c
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 09:37:37 2023 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 58e8f3c
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 09:37:23 2023 -0500

    mostly nits

commit 8df9523
Merge: fd06409 d0d3039
Author: Mike Griese <[email protected]>
Date:   Wed Oct 25 09:04:35 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit fd06409
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 15:17:38 2023 -0500

    annoying build break

commit fb74fc8
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 14:58:19 2023 -0500

    dead code

commit 5f4087f
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 14:56:50 2023 -0500

    finish exorcising SettingsTab

commit 81889a6
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 14:08:49 2023 -0500

    derp

commit e82c627
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 12:09:08 2023 -0500

    dead code removal

commit d726165
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 12:06:59 2023 -0500

    terrible, but it works

commit 57e1f26
Merge: 6107c3e b49997b
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 11:36:27 2023 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit b49997b
Merge: 46469aa 2086e0f
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 11:12:24 2023 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 2086e0f
Merge: c869b47 544cdd7
Author: Mike Griese <[email protected]>
Date:   Fri Oct 13 10:39:02 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 6107c3e
Merge: 9531069 46469aa
Author: Mike Griese <[email protected]>
Date:   Mon Sep 11 05:43:06 2023 -0500

    Merge branch 'dev/migrie/fhl/scratchpad-pane' into dev/migrie/f/sui-panes

commit 46469aa
Merge: 1cc9835 c869b47
Author: Mike Griese <[email protected]>
Date:   Mon Sep 11 05:24:30 2023 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit c869b47
Merge: e0b003a 4ddfc3e
Author: Mike Griese <[email protected]>
Date:   Mon Sep 11 05:22:43 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit 9531069
Author: Mike Griese <[email protected]>
Date:   Mon Aug 7 15:17:09 2023 -0500

    background brush, done

commit 521e301
Author: Mike Griese <[email protected]>
Date:   Thu Aug 3 13:50:11 2023 -0500

    update settings should work now

commit 842326d
Author: Mike Griese <[email protected]>
Date:   Thu Aug 3 11:31:57 2023 -0500

    icons for non-terminal pane content

commit fb7c809
Author: Mike Griese <[email protected]>
Date:   Tue Aug 1 11:37:10 2023 -0500

    derp

commit 29d0d57
Author: Mike Griese <[email protected]>
Date:   Thu Jul 27 16:29:26 2023 -0500

    this works better than it has any right to

commit cbd61b0
Author: Mike Griese <[email protected]>
Date:   Thu Jul 27 15:55:05 2023 -0500

    POC: yea, this works

commit 1cc9835
Author: Mike Griese <[email protected]>
Date:   Thu Jul 27 13:58:44 2023 -0500

    feature flags too

commit 86914bd
Merge: a23c1a2 e0b003a
Author: Mike Griese <[email protected]>
Date:   Tue Jul 25 13:28:25 2023 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit e0b003a
Merge: f89368c 5daf498
Author: Mike Griese <[email protected]>
Date:   Mon Jul 24 14:10:40 2023 -0500

    Merge branch 'main' into dev/migrie/fhl/non-terminal-panes-2023

commit f89368c
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 13:47:38 2023 -0500

    [PARENT] try to use GetActiveTerminalControl less in TerminalTab

    (cherry picked from commit 262d95a)

commit 5582e1b
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 13:21:18 2023 -0500

    [PARENT] You know what, I just went for it.

    (cherry picked from commit 63ba8e1)

commit a23c1a2
Author: Mike Griese <[email protected]>
Date:   Thu Jul 20 07:39:02 2023 -0500

    keybindings too

commit 5f9add4
Merge: 2d40306 11126f9
Author: Mike Griese <[email protected]>
Date:   Thu Jul 20 07:04:10 2023 -0500

    Merge branch 'dev/migrie/fhl/non-terminal-panes-2023' into dev/migrie/fhl/scratchpad-pane

commit 11126f9
Merge: e31202b 7010626
Author: Mike Griese <[email protected]>
Date:   Thu Jul 20 07:02:16 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit e31202b
Merge: e6dc314 6a10ea5
Author: Mike Griese <[email protected]>
Date:   Thu Jul 20 07:02:04 2023 -0500

    Merge commit '6a10ea5' into dev/migrie/fhl/non-terminal-panes-2023

commit e6dc314
Merge: 049c043 b4042ea
Author: Mike Griese <[email protected]>
Date:   Wed Jul 19 16:22:03 2023 -0500

    Merge commit 'b4042ea' into dev/migrie/fhl/non-terminal-panes-2023

commit 2d40306
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 13:47:58 2023 -0500

    Let's just make it experimental

commit 262d95a
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 13:47:38 2023 -0500

    [PARENT] try to use GetActiveTerminalControl less in TerminalTab

commit 63ba8e1
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 13:21:18 2023 -0500

    [PARENT] You know what, I just went for it.

commit 1b39db7
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 12:58:55 2023 -0500

    Single commit that adds the whole scratchpad and action

commit 2dd8f40
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 11:48:33 2023 -0500

    [TO PARENT] dead code

commit 049c043
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 10:26:32 2023 -0500

    some last cleanups

commit a1da6c1
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 10:13:44 2023 -0500

    huge shuffling so that pane content can raise events instead of relying on termcontrol

commit 7c9ffb0
Author: Mike Griese <[email protected]>
Date:   Tue Jul 18 06:06:07 2023 -0500

    snapping now uses an interface, so that it's not TermControl-specific

commit 84df819
Author: Mike Griese <[email protected]>
Date:   Mon Jul 17 14:22:12 2023 -0500

    close event

commit 5b3aa54
Author: Mike Griese <[email protected]>
Date:   Mon Jul 17 12:42:43 2023 -0500

    move GetNewTerminalArgs into IPaneContent

commit ef6bb8a
Author: Mike Griese <[email protected]>
Date:   Mon Jul 17 12:35:27 2023 -0500

    hey look, it builds now

commit 4e14442
Merge: f353323 f4bcbfb
Author: Mike Griese <[email protected]>
Date:   Mon Jul 17 10:53:56 2023 -0500

    Merge remote-tracking branch 'origin/main' into dev/migrie/fhl/non-terminal-panes-2023

commit f353323
Author: Mike Griese <[email protected]>
Date:   Fri May 12 13:32:12 2023 -0500

    I wanted to do this in one shot but _zelda_

This comment has been minimized.

@zadjii-msft
Copy link
Member Author

snippets-pane

@zadjii-msft zadjii-msft added this to the Terminal v1.22 milestone May 31, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 31, 2024
I noticed this while working on #17330. We're constructing a whole
lambda just to do this wacky weak_ref logic, and that feels... gross. We
should just make this a bound method and a typed event, so we can just
use the one event handler regardless
zadjii-msft added a commit that referenced this pull request Jun 3, 2024
@zadjii-msft
Copy link
Member Author

  • Snippets pane doesn't display some "no snippets found" text if there aren't any yet

Adding
image
in the latest commit.

This comment has been minimized.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Found a few more bugs while testing:

  • "close pane" keybinding --> crash (looks like we're failing to check IsReadOnly()?)
  • pressing space on an item "selects" it, but doesn't open it. Honestly, I'd treat space as enter
  • When snippets are open, pressing tab goes like this: textBox -> treeView -> unknown -> unknown -> textBox (we've cycled). Idk where those two unknown are, but we should remove them.
  • I'd expect enter/space on the tree view item itself to invoke the "play" button. It's just a dead key rn. Pretty annoying to have to tab to "play" button to get it injected imo.
  • Clicking on a root node (i.e. "git..." which expands to a bunch more options) doesn't actually expand it! We need to click on the chevron. It'd be nice if we could just click on the item itself and it would expand.

Accessibility issues:
The UIA tree has a bunch of problems in it. I'm gonna list a bunch of changes we need here, but the ones that are labelled with a "❌" are the ones that are most urgent. We might get flagged with the others during an a11y pass, but I'd be fine with them as a follow-up.
Accessibility Insights UIA tree

  • pane and custom have no a11y name
    • MG: As discussed, punting this out of this review, considering it seems all the panes have that? Maybe it's the Border inside panes, but it's not unique to this PR. The SUI has two!
  • tree items have the name TreeViewNode... so screen readers just read TreeViewNode as you move focus from one node to another... so please please please give them a name (should be the displayed text from the text blocks). Key thing here is that all text that's displayed on the screen must be read by a screen reader.
  • ❌ button doesn't have a name. It should have the same name as the tooltip
  • We should probably announce "open snippets pane" when we open the snippets pane. It's a context shift we should make clear to the user (especially if they're visually impaired).

Some misc. things I also found during code review:

  • Do we want to x:Load="False" the whole snippets pane too?
    • MG: Nah, cause it's just instantiated on demand anyways. It's not in the startup path.

Some funny things that can/should/maybe/idk be fixed(?):

  • I get that it's a snippets pane and we can/should be allowed to open an infinite amount. I'm still a bit concerned about having a bunch of snippets panes open and not knowing where the input will be sent to
  • Maybe we shouldn't allow a snippets pane to be opened from a snippets pane?...
  • snippets squared
  • I somehow got into a state where pressing the "open snippets" keybinding will open TWO snippets panes at once! SUPER SNIPPETS!
    • update from 5 minutes later: oh huh, it suddenly stopped doing that... that was weird
    • MG: Uh, okay? I'll just assume this was nothing then?

Ok, I think I've bug bashed this enough. I have yet to review SnippetsPaneContent files, but this should be plenty to keep you busy for at least a morning.

src/cascadia/TerminalApp/AppActionHandlers.cpp Outdated Show resolved Hide resolved
src/inc/til/string.h Outdated Show resolved Hide resolved
src/cascadia/TerminalSettingsModel/ActionMap.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/FilteredCommand.idl Show resolved Hide resolved
src/cascadia/TerminalApp/TerminalPaneContent.h Outdated Show resolved Hide resolved
Margin="4"
FontSize="24" />

<TextBlock Grid.Row="1"
Copy link
Member

Choose a reason for hiding this comment

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

x:Load="False"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Meh? We're already instantiating the entire pane at runtime (so loading it isn't on the startup path). I guess this would save us instantiating like, 4 controls at runtime, but idk

src/cascadia/TerminalApp/SnippetsPaneContent.xaml Outdated Show resolved Hide resolved
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 11, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 11, 2024

This comment has been minimized.

@zadjii-msft
Copy link
Member Author

Addressed the feedback in #17330 (review), save for one thing that isn't unique to this PR, and the infinite snippets pane thing. I guess everyone hates that, so I'll do something dumb to go prevent that.

Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Omg. This feels really nice with the keyboard. Thank you!

Found one more bug:

  • first invocation doesn't work
    • open a new tab
    • open snippets pane using a keybinding (NOT command palette)
    • tab to an entry of interest
    • tab to the play button on it
    • invoke the play button (i.e. space, enter, click)
    • BUG: input is not sent anywhere

These are more design-related changes. I'd love it if they made it in this PR, but at the very least, they should be tracked and/or discussed for this milestone imo:

  • We can get into a state where the tab only contains a snippets pane. Should that be allowed?
    • in this state, snippets cannot be injected into a terminal in another tab (which is a good thing imo)
  • Aside from the situation above, input should always go somewhere, right? If that's not the case, we should maybe display a warning at the top of the pane or maybe in a teaching tip? Have the warning say "no active terminal detected"?
    • really, I'm just trying to avoid the play button being a dead button. Because when that happens, I just get confused and sad.
  • Should we add a short description at the top of the Snippets pane? On one hand, it'll probably be ignored by most people. On the other, it could be a nice/short/quick intro to the feature.
  • Double click a tree view item to send input? 👀

Requesting changes still, but once we get the last a11y stuff done and you fix that one bug above, we should be good. So close! But it's great!

src/cascadia/TerminalApp/SnippetsPaneContent.h Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.xaml Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.xaml Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/SnippetsPaneContent.xaml Outdated Show resolved Hide resolved
Comment on lines +239 to +241
<Run x:Uid="SnippetsPaneNoneFoundHeader" />
<LineBreak />
<Run x:Uid="SnippetsPaneNoneFoundDetails" />
Copy link
Member

Choose a reason for hiding this comment

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

📝 This is what this looks like now
image

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 12, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jun 12, 2024
@zadjii-msft zadjii-msft added this pull request to the merge queue Jul 8, 2024
Merged via the queue into main with commit 02a7c02 Jul 8, 2024
20 checks passed
@zadjii-msft zadjii-msft deleted the dev/migrie/f/snippets-pane branch July 8, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants