feature: pin emulator to the top of the list #91
Conversation
There was a problem hiding this comment.
Overall, good start! I have couple of comments and one big question 😉
I know it was mentioned in the feature description. But do we really need Device to keep this flag? Are we going to use anywhere else, where this model is passed? Mainly question to @okwasniewski.
Maybe it's time to create another model specifically for Menu?
EDIT: Looks like it's inline with your last question @dnafication 😉
| @@ -46,4 +48,15 @@ extension UserDefaults { | |||
| get { bool(forKey: Keys.enableAndroidEmulators) } | |||
| set { set(newValue, forKey: Keys.enableAndroidEmulators) } | |||
| } | |||
|
|
|||
| public var pinnediOSSimulators: [String]? { | |||
There was a problem hiding this comment.
You can store everything under one key. This will simplify logic in DeviceService.
| @@ -216,18 +240,21 @@ extension DeviceService { | |||
| static private func parseIOSDevices(result: [String]) -> [Device] { | |||
| var devices: [Device] = [] | |||
| var osVersion = "" | |||
| let pinnediOSSimulators: [String] = UserDefaults.standard.pinnediOSSimulators ?? [] | |||
There was a problem hiding this comment.
I would say it's quite invasive to the parsing function. Maybe you can move it to getIOSDevices?
|
Thanks for opening the PR! Storing this inside of
I'm not really sure how to achieve this with current refactor of menus. Maybe we could pass the current device to its menu item? I've quickly launched the app - what I've noticed is that although pinned items are correctly persisted across app launches, they are not sorted. You need to take that into account in I was also thinking about the appearance - maybe we could leverage the |
…r-to-the-top-of-the-list
This pull request introduces the ability to pin simulators/emulators to the top of the list. It includes changes to the
UserDefaultsextension to store pinned devices, modifications to theSubMenuItemstructure to add a "TogglePinToTop" action, updates to theDevicemodel to include a "pinned" Bool property, and adjustments to theDeviceServicefor toggling the pinned status.Note that the menu items are not being sorted currently. I just wanted to create the PR to get an initial feedback on if I am going in the right direction.
Some thoughts:
SubMenuActionItemor the content ofSubMenuActionItembased on the state of the Device. Example:Pin to toplabel and icon could change toUnpin to topbased on the device state.Please provide your feedback @okwasniewski @Garfeild
Closes #70