wakatime / macos-wakatime

Mac system tray app for automatic time tracking and metrics generated from your Xcode, Figma, Postman, etc. usage.
https://wakatime.com/mac
BSD 3-Clause "New" or "Revised" License
126 stars 22 forks source link

Make monitored apps scrollable #235

Closed starbugs closed 3 months ago

starbugs commented 3 months ago

Closes #232.

After some fiddling with the stack views, I gave up and rewrote the whole view to use a proper NSOutlineView, which is pretty standard on macOS. This one correctly adapts to the scroller visibility, which can be tricky on macOS and is hard to work out if you do your own customer NSScrollView with stack views setup.

What is more, there was a bug where, on first app launch, the Monitored Apps window would display all apps as disabled, even though the default apps were enabled (race condition) via MonitoringManager.enableByDefault.

I had a hard time understanding the following in the old code and I didn't overtake it to the new code. Please let me know if we need this and what it's supposed to do:

    @objc func switchToggled(_ sender: NSSwitch) {
        let isSetApp = !sender.tag.isMultiple(of: 2)
        let index = (isSetApp ? sender.tag - 1 : sender.tag) / 2
        var bundleId = MonitoredApp.allBundleIds[index]

        if isSetApp {
            bundleId = bundleId.appending("-setapp")
        }

        MonitoringManager.set(monitoringState: sender.state == .on ? .on : .off, for: bundleId)
    }

The new implementation just does this (which appears to be working fine):

    @objc func switchToggled(_ sender: NSSwitch) {
        guard sender.tag >= 0 && sender.tag < MonitoredApp.allBundleIds.count else { return }
        let bundleId = apps[sender.tag].bundleId
        MonitoringManager.set(monitoringState: sender.state == .on ? .on : .off, for: bundleId)
    }
alanhamlett commented 3 months ago

I had a hard time understanding the following in the old code and I didn't overtake it to the new code. Please let me know if we need this and what it's supposed to do:

I wrote that code. It's not very readable, but what it does is assign a unique key to each NSSwitch based on it's index. But, we have a special case with apps installed via SetApp.com where their bundleIDs are appended with -setapp. We need to check both the original bundleID and the one installed via SetApp with -setapp suffix. Since we only have 1 list of bundleIDs, I added 2 potential NSSwitch for each bundleID. So, if an app is installed normally and with SetApp then we can have 2 NSSwitch for 1 bundleID. This above code is to check via index number if the NSSwitch is turned on, but we have to go backwards from the index number to the app's bundleID.

So, if we modify MonitoredApp.allBundleIds to only have 1 app: ["com.linear"] then we install Linear normally and via SetApp too, we have 2 NSSwitch rows. The two rows have indexes 0 and 1. So we use the formula to get the bundleID for each app via their index: 0 gives com.linear and 1 gives com.linear-setapp.

The new implementation just does this (which appears to be working fine):

That works only because you removed support for apps installed via SetApp. Here's where I added the NSSwitch for the normal bundleID and the SetApp bundleID, so potentially 2 NSSwitch per bundleID in the MonitoredApp.allBundleIds list:

https://github.com/wakatime/macos-wakatime/pull/235/files#diff-7ddf2b2fcf8afdcffb0da42497d87b4bac163f275b74711f0899d0bb0e2946cdL31

starbugs commented 3 months ago

Ok I will fix the logic then. Thanks for the explanation.

starbugs commented 3 months ago

This should be fixed now.

I replaced the index logic with this:

    private lazy var apps: [AppData] = {
        var apps = [AppData]()
        let bundleIds = MonitoredApp.allBundleIds.filter { !MonitoredApp.unsupportedAppIds.contains($0) }
        var index = 0
        for bundleId in bundleIds {
            if let icon = AppInfo.getIcon(bundleId: bundleId),
               let name = AppInfo.getAppName(bundleId: bundleId) {
                apps.append(AppData(bundleId: bundleId, icon: icon, name: name, tag: index))
                index += 1
            }

            let setAppBundleId = bundleId.appending("-setapp")
            if let icon = AppInfo.getIcon(bundleId: setAppBundleId),
               let name = AppInfo.getAppName(bundleId: setAppBundleId) {
                apps.append(AppData(bundleId: setAppBundleId, icon: icon, name: name, tag: index))
                index += 1
            }
        }
        return apps
    }()

The switchToggled func then simply becomes this:

    @objc func switchToggled(_ sender: NSSwitch) {
        let appData = apps[sender.tag]
        MonitoringManager.set(monitoringState: sender.state == .on ? .on : .off, for: appData.bundleId)
    }