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

Partial move to SwiftUI lifecycle #1152

Merged
merged 21 commits into from
Mar 15, 2023
Merged

Conversation

Wouter01
Copy link
Member

@Wouter01 Wouter01 commented Mar 13, 2023

Description

This PR moves parts of CodeEdit to the SwiftUI lifecycle. This way, the settings window can take full advantage of the new APIs released with macOS Ventura. Other views, like the Welcome Window, have also been ported and simplified. Methods are provided so there's compatibility with AppKit.

This PR also fixes some small issues, such as Cmd+W not working properly for closing windows. (This should actually close tabs, but we'll keep it windows for now).

No changes should be visible, except the menu bar which might look slightly different.

The old settings menu option has been moved over to Old Settings..., which has the shortcut Command + Option + ','

A nice bonus is a new API to easily add hidden options to a SwiftUI contextMenu. This was needed for some menus of the menubar, but it also works in all other contextMenus throughout the app.
Example:

.contextMenu {
    // Show this button when no control or option are pressed
    Button("Some Menu Item") {
    }
    .keyboardShortcut("c", modifiers: [.command])
    
    // Show this button when option is pressed
    Button("Some Hidden Menu Item") {
    }
    .keyboardShortcut("c", modifiers: [.command, .option, .hidden])

    // Show this button when control is pressed
    Button("Some Other Hidden Menu Item") {
    }
    .keyboardShortcut("c", modifiers: [.command, .control, .hidden])

    // Show this button when both option and control are pressed
    Button("Some Other Other Hidden Menu Item.") {
    }
    .keyboardShortcut("c", modifiers: [.command, .control, .option, .hidden])
}

Related Issues

#1066

Checklist

  • Create Settings boilerplate
  • Create function to open Settings from anywhere in the app.
  • Create functions to open SwiftUI windows from AppKit.
  • Create method to use option key in menus.
  • Create @FirstResponder Property Wrapper
  • Support for ForEach in Commands.

Modify the menu bar so it's somewhat equal to the previous implementation:

  • Main Menu
  • File Menu
  • Edit Menu
  • View Menu
  • Find Menu
  • Navigate Menu
  • Source Control Menu
  • Window Menu
  • Help Menu

Additional

  • Move Welcome Window to SwiftUI Window
  • Move About Window to SwiftUI Window
  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
@Wouter01 Wouter01 added the enhancement New feature or request label Mar 13, 2023
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
@Wouter01 Wouter01 marked this pull request as ready for review March 14, 2023 02:27
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
0xWDG
0xWDG previously approved these changes Mar 14, 2023
@austincondiff
Copy link
Collaborator

austincondiff commented Mar 14, 2023

The proposed menu API doesn't quite make sense to me.

// Hide this button when option is pressed
Button("Some menu item to be replaced when pressing option") {
}
.keyboardShortcut("c", modifiers: [.command])
.hideOnModifierKeys([.option])

// Show this button when option is pressed
Button("Some hidden menu item visible when pressing option") {
}
.keyboardShortcut("c", modifiers: [.command, .option])
.showOnModifierKeys([.option])

Wouldn't this be more appropriate rather than combining it with the keyboardShortcut modifier?

@Wouter01
Copy link
Member Author

Wouter01 commented Mar 14, 2023

Wouldn't this be more appropriate rather than combining it with the keyboardShortcut modifier?

Yeah, definately. But I'm limited to what I can do here.

I somehow need to know in a method in AppKit that a SwiftUI menu item wants to be a hidden menu item.
The only way to pass this info down is by:

  • having some kind of custom title which could be detected
  • some custom key modifier combination which could be detected

These are the only two options available atm as far as I know, as these are the only two properties that are customizable with SwiftUI.

So, I went with the latter. The .hidden modifier is actually just an alias for .numericPad. I check in the AppKit code if the .numericPad modifier is present. If so, I set the isAlternate value of the item to true, which will apply the AppKit magic.

I went this way as .numericPad is never used.

Note: I reported feedback to Apple to add a view modifier which would be able to do this. Let's hope they add this soon.

Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

The changes look good overall, just have some questions about the settings pane and the CommandsForEach struct.

CodeEdit/AppDelegate.swift Outdated Show resolved Hide resolved
CodeEdit/CodeEditApp.swift Show resolved Hide resolved
CodeEdit/Features/WindowCommands/MainCommands.swift Outdated Show resolved Hide resolved
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
@Wouter01 Wouter01 dismissed stale reviews from matthijseikelenboom and 0xWDG via 5d115a9 March 15, 2023 00:43
Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

Looks great now, just one more tiny thing for some future-proofing.

Signed-off-by: Wouter01 <wouterhennen@gmail.com>
Copy link
Collaborator

@thecoolwinter thecoolwinter left a comment

Choose a reason for hiding this comment

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

LGTM

@Wouter01 Wouter01 merged commit 3386966 into CodeEditApp:main Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants