Refactoring menu building flow

[Garfeild]

This PR contains set of changes to simplify how Menu is built:

• populate() function is split into several case specific functions.
• Android and iOS specific submenu enum types are replaced with more flexible combination of protocol and structs. This allows to unify logic of submenu building.
• Pre-population code is updated to have separate logic of creating main menu actions and platform headers.

Overall refactored Menu should be more flexible for future changes, e.g. adding new actions to device sub menu.

[okwasniewski]

This looks very good! I will do some more testing later this week, here are some initial comments I've found

[okwasniewski]

Looks like something is wrong with the divider appearing after first 3 items

[okwasniewski]

This logic needs to be aligned to account for the separator.

You can change it like so (If you won't find better solution):

self.safeInsertItem(menuItem, at: section == .iOS ? startIndex + 1 : startIndex + index + 1)

[Garfeild]

Very strange. Could you give me steps to reproduce?

[okwasniewski]

If you have more than 3 simulators that's constantly happening. I've checked also on my second Macbook and it's the same. It happens on the initial launch of the app and if you later on add new simulator through Xcode same thing happens.

What you could do is always insert new items at the top of the list:

self.safeInsertItem(menuItem, at: startIndex + 1)

[Garfeild]

Not sure why, but I haven't experienced such behavior. What macOS version do you run? Regarding your suggestion. It will work, but it's not ideal since order will be different at restart of the app. What if we completely rebuild section by removing all items between header and separator?

guard let header = self.items.first(where: { $0.tag == section.rawValue }),
      let startIndex = self.items.firstIndex(of: header),
      let lastIndex = self.items[startIndex..<self.items.count].firstIndex(where: { $0.isSeparatorItem }) else {
    return
}

if startIndex != lastIndex-1 {
    self.items[startIndex+1..<lastIndex]
        .forEach { self.safeRemoveItem($0)}
}

items.forEach { self.safeInsertItem($0, at: startIndex + 1) }

[okwasniewski]

@Garfeild Im running latest MacOS 14.0. Why items will order will be different after restart? Rebuling sections after every open of the menu doesn't sound too efficent. We can always reverse the items array and append them at the top like so:

for menuItem in items.reversed() {
            if let itemIndex = self.items.firstIndex(where: { $0.title == menuItem.title }) {
                self.replaceMenuItem(at: itemIndex, with: menuItem)
                continue
            }
            self.safeInsertItem(menuItem, at: startIndex + 1)
        }

It was working reliably for me without any issues.

[Garfeild]

The order is alphabetical, right? If you add sim that doesn't match order, it will be on top. Once you restart app, it will be placed properly.

Screen.Recording.2023-11-05.at.21.32.00.mov

Another question, do you see the same behavior of incorrectly located separator in commit 5ebe102?

[okwasniewski]

I'm fine with the behaviour you showed that it's not alphabetical when you insert new item - it isn't the most important thing 👍🏻

And yeah it's the same on 5ebe102

[Garfeild]

Alright, I will look into that afterwards 😉 I would like to insert item into right place for very beginning. And I'm kinda concerned about current way of managing sections.

[okwasniewski]

Sure let's merge it to unblock others and I will also take a look at inserting items 👍🏻

[okwasniewski]

Thanks for huge amount of great work! 🎉