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

refactor: Removed UIKit dependencies from many files #464

Merged
merged 3 commits into from
Nov 6, 2024
Merged

Conversation

tomsci
Copy link
Collaborator

@tomsci tomsci commented Oct 26, 2024

And made CanvasView and RootView derive from either UIView or NSView, and images be either UIImage or NSImage

@tomsci tomsci requested a review from jbmorley October 26, 2024 18:48
Copy link
Collaborator

@jbmorley jbmorley left a comment

Choose a reason for hiding this comment

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

This looks really clean. Thank you for taking the time to do it—it'll be nice to get a more native macOS version working. I think my only request is a rename on Image to something that doesn't conflict with SwiftUI. (Apple's new API naming is as self-important and short sighted as their app naming.)

#else

import AppKit
typealias Image = NSImage
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not call this Image as that conflicts with SwiftUI and is pretty misleading. I suspect there's actually more we can do with SwiftUI's image in this scenario but for the time being how about we simply call it CommonImage or somesuch?


extension Image {

static func clockMedium() -> Image {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Xcode will now synthesize symbols for images in the project so we might be able to get away without these now. Still, not a high priority.

@@ -208,6 +222,7 @@ class CanvasView : UIView, Drawable {
return canvas.size.cgSize()
}

#if canImport(UIKit) // TODO AppKit version
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤣

@tomsci tomsci requested a review from jbmorley November 5, 2024 08:42
@jbmorley
Copy link
Collaborator

jbmorley commented Nov 5, 2024

Thank you for the change. Approved! 🎉

@tomsci tomsci enabled auto-merge (squash) November 6, 2024 15:25
@tomsci tomsci merged commit 1ba7a5d into main Nov 6, 2024
2 checks passed
@tomsci tomsci deleted the tomsci-deuikit branch November 6, 2024 15:26
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.

2 participants