-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 top-level nested commands #7174
Comments
I prefer the layout of option 2. However, I could see a use case for people searching for a specific profile and the default behavior would be to open a new tab. |
I like 2 better as well, its layout seems more logical to me. IMO I feel like if I wanted to open a In addition to the nested |
Alright well that's enough consensus for me to go on. I'll ship a PR for option 2, and we'll finish discussion in the PR. Thanks guys! |
## Summary of the Pull Request ![cmdpal-default-nested-commands](https://user-images.githubusercontent.com/18356694/90684483-e6b13400-e22d-11ea-8ca6-fe90ca8d9e82.gif) Adds a pair of top-level commands that both have nested, iterable sub-commands. The "New Tab..." command has one child for each profile, and will open a new tab for that profile. The "Split Pane..." command similarly has a nested command for each profile, and also has a nested command for split auto/horizontal/vertical. ## References * megathread: #5400 * Would look better with icons from #6644 ## PR Checklist * [x] Closes #7174 * [x] I work here * [ ] Tests added/passed * [n/a] Requires documentation to be updated
🎉This issue was addressed in #7348, which has now been successfully released as Handy links: |
I'm filing this one to get team feedback for what commands we should include in the defaults once #6856 lands. I'm gonna throw the usual suspects on the Assigned To line, and remove yourself when you've shared thoughts
Shouldn't we replace the "newTab" and "splitPane" entries here with nested/iterable commands?
I was thinking, something neat would be to make the tree look like this:
_Originally posted by @carlos-zamora in #6856
In my head, this is how I pictured it:
Which then would evaluate to
The text was updated successfully, but these errors were encountered: