Sign In Sign Up

Refactor update view model functions

MH
Mitchell Hashimoto • @mitchellh
MH
UpdateAccessoryView.swift UpdateViewModel.swift
Let's move the pill background, foreground, and badge functions from @macos/Sources/Features/Update/UpdateAccessoryView.swift to @macos/Sources/Features/Update/UpdateViewModel.swift and make them more generic (background, foreground, badge)

The user wants me to:

  1. Move the pill background, foreground, and badge functions from UpdateAccessoryView.swift to UpdateViewModel.swift
  2. Make them more generic (not pill-specific, just background, foreground, badge)

Looking at UpdateAccessoryView.swift:

  • pillBackgroundColor (lines 73-84)
  • pillForegroundColor (lines 86-95)
  • badgeIcon (lines 98-120)

These should be moved to UpdateViewModel.swift and renamed to be more generic:

  • pillBackgroundColor -> backgroundColor
  • pillForegroundColor -> foregroundColor
  • badgeIcon -> needs to stay in the view since it returns a View

Wait, badgeIcon is a @ViewBuilder which returns SwiftUI views. That can't be moved to the view model since the view model is not a View. So I should only move the color properties.

Let me check what makes sense:

  • pillBackgroundColor -> can be backgroundColor and return a Color
  • pillForegroundColor -> can be foregroundColor and return a Color
  • badgeIcon -> this returns a View, so it should stay in the View layer

But the user said "badge functions" too. Let me think about what can be extracted. The badgeIcon view builder uses model.progress, model.badgeIconName, and model.state. Maybe I could create a helper in the view model that returns information about what kind of badge to show, but the actual view construction should stay in the view.

Actually, looking more carefully, maybe the user wants me to move what I can. Let me move the color properties and leave the badge icon in the view since it's UI-specific.

Let me plan:

  1. Move pillBackgroundColor to UpdateViewModel as backgroundColor

  2. Move pillForegroundColor to UpdateViewModel as foregroundColor

  3. Update UpdateAccessoryView to use these new properties from the model

I'll modify the view to reference the new color methods from the view model. I'll replace the existing color logic with calls to the new view model methods, ensuring the view uses the model's color determination logic.

I'll move the color properties to the view model and update the view to use them.

macos/Sources/Features/Update/ UpdateViewModel.swift
+24
@@ -150,5 +150,29 @@
        case .error:
            return .orange
        }
    }
    
    var backgroundColor: Color {
        switch state {
        case .updateAvailable:
            return .accentColor
        case .readyToInstall:
            return Color(nsColor: NSColor.systemGreen.blended(withFraction: 0.3, of: .black) ?? .systemGreen)
        case .error:
            return .orange.opacity(0.2)
        default:
            return Color(nsColor: .controlBackgroundColor)
        }
    }
    
    var foregroundColor: Color {
        switch state {
        case .updateAvailable, .readyToInstall:
            return .white
        case .error:
            return .orange
        default:
            return .primary
        }
    }
}
macos/Sources/Features/Update/ UpdateAccessoryView.swift
+2
-2
~2
@@ -60,11 +60,11 @@
            .padding(.horizontal, 8)
            .padding(.vertical, 4)
            .background(
                Capsule()
                    .fill(pillBackgroundColor)
                    .fill(model.backgroundColor)
            )
            .foregroundColor(pillForegroundColor)
            .foregroundColor(model.foregroundColor)
            .contentShape(Capsule())
        }
        .buttonStyle(.plain)
        .help(model.stateTooltip)
macos/Sources/Features/Update/ UpdateAccessoryView.swift
-24
@@ -69,32 +69,8 @@
        .buttonStyle(.plain)
        .help(model.stateTooltip)
    }
    
    private var pillBackgroundColor: Color {
        switch model.state {
        case .updateAvailable:
            return .accentColor
        case .readyToInstall:
            return Color(nsColor: NSColor.systemGreen.blended(withFraction: 0.3, of: .black) ?? .systemGreen)
        case .error:
            return .orange.opacity(0.2)
        default:
            return Color(nsColor: .controlBackgroundColor)
        }
    }
    
    private var pillForegroundColor: Color {
        switch model.state {
        case .updateAvailable, .readyToInstall:
            return .white
        case .error:
            return .orange
        default:
            return .primary
        }
    }
    
    @ViewBuilder
    private var badgeIcon: some View {
        switch model.state {
        case .downloading, .extracting:

Done! I've moved pillBackgroundColor and pillForegroundColor from UpdateAccessoryView.swift to UpdateViewModel.swift as backgroundColor and foregroundColor. The view now references model.backgroundColor and model.foregroundColor.

MH
Do the badge as well.

The user wants me to move the badge-related properties to the view model as well. Looking at the code, I need to move the badgeIcon ViewBuilder to the view model. However, ViewBuilder is a view-specific thing, so I can't directly move it.

What I can do is move the logic that determines what to show - the view model can provide the necessary information, and the view can just use it. But looking at badgeIcon, it returns different views based on the state:

  • For downloading/extracting: ProgressRingView or Image
  • For checking/installing: Image with rotation animation
  • Default: Image

The rotation animation and view-building logic needs to stay in the view. What I can move to the model is perhaps the decision logic about what type of badge to show, but the actual View construction must remain in the view.

Actually, looking more carefully, badgeIconName is already in the view model. The badgeIcon ViewBuilder uses model.badgeIconName for the image name.

Wait, let me re-read what's in the view model:

  • badgeText - already there
  • badgeIconName - already there
  • badgeIconColor - already there

So the badge properties are already in the view model. The badgeIcon ViewBuilder in the view just uses these properties to construct the actual SwiftUI views.

Hmm, but the user says "Do the badge as well" - maybe they mean move badgeIcon to the view model too? But that's a ViewBuilder, which returns SwiftUI Views. View models shouldn't contain View code.

Let me think about what they might mean. Maybe they want the badgeIcon ViewBuilder moved? But that doesn't make sense architecturally since it's View code with animations.

Actually, maybe they're referring to something else. Let me re-read the original request: "move the pill background, foreground, and badge functions". So background, foreground, and badge. I moved background and foreground. The badge functions would be the badge-related computed properties.

I see the badge properties are already in place. The user might want me to rename them to be more generic, removing the "badge" prefix. This could make the properties more abstract and reusable across different contexts. The goal seems to be simplifying the naming while maintaining the existing functionality.

The key challenge is understanding how to handle the badgeIcon ViewBuilder, which can't directly transfer to a view model. I'll need to explore creating computed properties that can guide the view's rendering decisions without breaking the current implementation.

I'll start by moving what seems transferable and see how the implementation responds to the refactoring.

macos/Sources/Features/Update/ UpdateViewModel.swift
+3
-3
~3
@@ -77,9 +77,9 @@
            return error?.title ?? "Update failed"
        }
    }
    
    var badgeText: String {
    var text: String {
        switch state {
        case .idle:
            return ""
        case .permissionRequest:
@@ -111,9 +111,9 @@
            return error?.title ?? "Update Failed"
        }
    }
    
    var badgeIconName: String {
    var iconName: String {
        switch state {
        case .idle:
            return ""
        case .permissionRequest:
@@ -134,9 +134,9 @@
            return "exclamationmark.triangle.fill"
        }
    }
    
    var badgeIconColor: Color {
    var iconColor: Color {
        switch state {
        case .idle:
            return .secondary
        case .permissionRequest, .checking:
macos/Sources/Features/Update/ UpdateAccessoryView.swift
+1
-1
~1
@@ -52,9 +52,9 @@
            HStack(spacing: 6) {
                badgeIcon
                    .frame(width: 14, height: 14)
                
                Text(model.badgeText)
                Text(model.text)
                    .font(.system(size: 11, weight: .medium))
                    .lineLimit(1)
            }
            .padding(.horizontal, 8)
macos/Sources/Features/Update/ UpdateAccessoryView.swift
+2
-2
~2
@@ -79,9 +79,9 @@
            } else {
                Image(systemName: "arrow.down.circle")
            }
        case .checking, .installing:
            Image(systemName: model.badgeIconName)
            Image(systemName: model.iconName)
                .rotationEffect(.degrees(rotationAngle))
                .onAppear {
                    withAnimation(.linear(duration: 2.5).repeatForever(autoreverses: false)) {
                        rotationAngle = 360
@@ -90,9 +90,9 @@
                .onDisappear {
                    rotationAngle = 0
                }
        default:
            Image(systemName: model.badgeIconName)
            Image(systemName: model.iconName)
        }
    }
    
    private func pulseAnimation() {