-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Keyboard shortcut to switch sidebar navigator tabs #1382
Conversation
These buttons theoretically will still consume space otherwise not occupied. Maybe setting width and height to 0 and clip. We could create a custom modifier for this. However there may be a better alternative. Anyone else have any better ideas? |
There is no issue for this? |
Created an issue and updated the pull request comment |
Updated! |
Great work! Why is the Find Navigator command 4 and not command 3? |
I put it as 4 because in Xcode, 3 is reserved for the symbols tab, which I wasnt sure if it was going to be added to CodeEdit or not. If not I can change it to cmd+3. Lmk! |
I see. These keyboard shortcuts should be dynamic. You will be able to rearrange these icons. The shortcuts should always be in the order in which they appear. So say you move the Find Navigator from the third to the first position, it should then be ⌘1 instead of ⌘3. |
@FastestMolasses There are some SwiftLint errors that need to be fixed please. |
What do we have left on this PR? |
@matthijseikelenboom if this is the case, should we just merge this as is for now? |
@austincondiff Hmm yeah we could, but we don't know if it will ever get fixed. It can also just be a limitation of SwiftUI. Anyway, if we merge this, we need an issue that notes everything that is broken or missing. But I think its a better idea to disable dragging icons, otherwise we have broken functionality in the app, which I'm not a favor of. More so, it wasn't part of the issue that is being addressed here. |
This isn't a SwiftUI issue, but rather a mistake in the code. In ViewCommands.swift, a reference to the navigatorSidebarViewModel is used, but is not observed. Thus, the commands will only be set once and won't be updated afterwards. This can be easily solved: extension ViewCommands {
struct NavigatorCommands: View {
@ObservedObject var model: NavigatorSidebarViewModel
var body: some View {
Menu("Navigators", content: {
ForEach(Array(model.items.enumerated()), id: \.element) { index, tab in
Button(tab.title) {
model.setNavigatorTab(tab: tab)
}
.keyboardShortcut(KeyEquivalent(Character(String(index + 1))))
}
})
}
}
} And then in ViewCommands: if let model = windowController?.navigatorSidebarViewModel {
Divider()
NavigatorCommands(model: model)
} NavigatorCommands is provided as an extension of ViewCommands, so it doesn't clutter the global scope Could you add these changes in your PR, @FastestMolasses ? |
@Wouter01 Great catch, it's working perfectly now. This PR is ready to be reviewed now. |
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.
Great work @FastestMolasses! ✅
Theres 1 swift lint error, where the |
ac2d8b0
to
2612387
Compare
@FastestMolasses yes that's an okay exception. I don't feel like adding a struct just for that function call is a good trade off, which is what I'd usually suggest in place of it. Just make sure each parameter is on a new line in that function declaration. |
Ready for a re-review! |
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.
LGTM! ✅
Bumping! |
Changes were made
Description
Edited: This PR now adds the shortcuts to change between the sidebar navigator tabs. This also removes the old shortcut of using ⌘+index to change between files. Finally, these shortcuts are also added in the
View/Navigators
menu item like how Xcode has it. Each tab is now draggable and they can be reordered by the user to their preference.The shortcut is:
⌘+ number
With
number
ranging from 1 - 9. Because each sidebar navigator tab is now draggable, you can rearrange the order and the following keyboard shortcuts will update accordingly.Related Issues
Closes #1384
Checklist
Screenshots
Here's how the shortcut menu looks in Xcode
Here it is in CodeEdit
Demo