feat: add preference option to change MiniSim icon#82
feat: add preference option to change MiniSim icon#82schrobingus wants to merge 4 commits intookwasniewski:mainfrom
Conversation
okwasniewski
left a comment
There was a problem hiding this comment.
Thanks for those changes! I've left few comments
MiniSim/Views/Preferences.swift
Outdated
|
|
||
| struct Preferences: View { | ||
| var menuImages = [ "menu_icon_1", "menu_icon_2", "menu_icon_3" ] | ||
| @State var menuImageSelected = 0 |
There was a problem hiding this comment.
I think you can use the @AppStorage property wrapper to save directly to user defaults 👍🏻
There was a problem hiding this comment.
From my testing, AppStorage does save directly, but the appearance of the Picker will not change without State. Both cannot be used simultaneously in a single variable (menuImageSelected) either. Two variables I'd say would likely not be advantageous to using onChange, but if you'd like, I can do that as well. How should I implement this?
There was a problem hiding this comment.
I've used AppStorage for Picker in this file: MiniSim/Views/Onboarding/SetupView.swift - check it out
| } | ||
| } | ||
| .frame(minWidth: 650, minHeight: 450) | ||
| .onAppear { |
There was a problem hiding this comment.
If you will use @AppStorage this also won't be needed
|
Thanks mate, I'll get to it in a few. |
|
I've made the changes suggested, albeit I do have a concern regarding @AppStorage's usage. I've resolved the suggestions that have been applied. :) |
| set { set(newValue, forKey: Keys.isOnboardingFinished) } | ||
| } | ||
|
|
||
| @objc dynamic public var menuImage: String { |
There was a problem hiding this comment.
Can you make it return MenuImage enum that you've added?
And also as the default let's use MenuImage.iphone
There was a problem hiding this comment.
Did that and it works great. Just noting though, Objective-C compliance had to be removed from menuImage, but I was able to rework the observer to that.
| ]) | ||
| } | ||
|
|
||
| private func configureMenuImages() { |
There was a problem hiding this comment.
let's move this logic to the MenuImage enum:
enum MenuImage: String, CaseIterable {
case iphone = "iphone"
case ipad = "ipad"
case box = "box"
var image: NSImage? {
guard let itemImage = NSImage(systemSymbolName: self.rawValue, accessibilityDescription: self.rawValue) else {
return nil
}
itemImage.size = NSSize(
width: (itemImage.size.width) * 0.78,
height: (itemImage.size.height) * 0.78)
itemImage.isTemplate = true
return itemImage
}
}There was a problem hiding this comment.
Then you can call MenuImage.iphone.image
There was a problem hiding this comment.
configureMenuImages is only ran once ever (setup) because the image gets smaller and smaller every single time the function is reran, NSImage remembers the previous size every time. Pardon me if I misunderstand, but this change would supposedly assume that MenuImage.image would be called multiple times?
There was a problem hiding this comment.
Image was getting smaller and smaller because you was mutating the button image in this case each item would have its own image
| import LaunchAtLogin | ||
|
|
||
| struct Preferences: View { | ||
| @State var menuImageSelected: String = "iphone" |
There was a problem hiding this comment.
I'm using @AppStorage in MiniSim/Views/Onboarding/SetupView.swift and it works great with Picker so I'm not sure whats the issue here. Can you check out that file?
There was a problem hiding this comment.
@okwasniewski Sorry for the long delay, I've been a bit busy lately. SetupView works because enableiOSSimulators and enableAndroidSimulators are changed before the menu setup is called. With menuImageSelected, it's a bit different, because the menu needs to change on the fly, as a @State would. I have seen examples of @AppStorage acting this way, but it isn't recurring for me for some reason.
For example, if I call enableAndroidSimulators in this way:
struct Preferences: View {
...
@AppStorage(UserDefaults.Keys.enableAndroidEmulators, store: .standard) var enableAndroidEmulators: Bool = true
var body: some View {
Settings.Container(contentWidth: 400) {
...
Settings.Section(title: "Enable Android Emulators:") {
Toggle(isOn: $enableAndroidEmulators) {
Text("Enable Android Emulators")
}
...
it doesn't mutate whilst it's open, because @AppStorage doesn't call like @State, at least in this case.
|
|
||
| private func configureMenuImages() { | ||
| MenuImage.allCases.forEach { image in | ||
| let itemImage = NSImage(imageLiteralResourceName: image.rawValue) |
There was a problem hiding this comment.
According to docs this initializer of NSImage shouldn't be used directly
Adds an option in the preferences for the user to change the menu bar icon for MiniSim. Addresses #69 (nice).
MiniSim.Demonstration.1.mp4