Skip to content

Modernize for macOS 14: drop dead fallback icons and deprecated SwiftUI modifiers - #242

Closed
superhighfives wants to merge 4 commits into
mainfrom
claude/pika-macos-14-modernize-wwzkde
Closed

Modernize for macOS 14: drop dead fallback icons and deprecated SwiftUI modifiers#242
superhighfives wants to merge 4 commits into
mainfrom
claude/pika-macos-14-modernize-wwzkde

Conversation

@superhighfives

Copy link
Copy Markdown
Owner

Summary

Two cleanup commits that take advantage of Pika's macOS 14 deployment floor to drop dead compatibility code and modernize deprecated SwiftUI APIs. Behaviour is unchanged throughout.

Changes

Remove dead pre-macOS-11 SF Symbol fallback icons
IconImage used to fall back to hand-drawn SVG copies of SF Symbols from the asset catalog on macOS < 11. With the deployment target now at macOS 14, IconImage always uses Image(systemName:), leaving the entire Assets.xcassets/Icons/ folder as dead fallback assets. This deletes the 12 unused imagesets and updates the one remaining consumer — the Touch Bar copy button — to load doc.on.doc directly from SF Symbols via NSImage(systemSymbolName:accessibilityDescription:).

Modernize deprecated SwiftUI modifiers for macOS 14
Replaces SwiftUI modifiers Apple deprecated at or below the macOS 14 floor with their modern equivalents:

  • .foregroundColor(_:).foregroundStyle(_:) (56 sites)
  • .cornerRadius(_:).clipShape(.rect(cornerRadius:)) (3 sites)
  • .edgesIgnoringSafeArea(.all).ignoresSafeArea() (4 sites)
  • .accentColor(_:).tint(_:) (1 site)

The ColorPair.foregroundColor / .backgroundColor properties are untouched — only the SwiftUI modifier form was migrated.

Test plan

  • Build both the Pika and Pika (Mac App Store) targets in Xcode
  • Confirm no remaining deprecation warnings for the migrated modifiers
  • Visually verify icons (including the Touch Bar copy button) render correctly

🤖 Generated with Claude Code

claude and others added 3 commits July 19, 2026 16:20
The IconImage view fell back to loading custom SVG copies of SF Symbols
from the asset catalog on macOS < 11 (see the 'fix: move to mac 11'
commit). Now that the deployment target is macOS 14, IconImage always
uses Image(systemName:), leaving the entire Assets.xcassets/Icons/ folder
as dead fallback assets.

Delete the 12 hand-drawn icon imagesets and modernize the one remaining
consumer — the Touch Bar copy button — to load doc.on.doc directly from
SF Symbols via NSImage(systemSymbolName:accessibilityDescription:).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FD1HGdhvposK8kYH2em6c3
Replace SwiftUI view modifiers that Apple deprecated at or below the new
macOS 14 deployment floor with their modern equivalents. Behaviour is
unchanged; the codebase was already partially migrated, so this brings the
remaining stragglers in line:

- .foregroundColor(_:) -> .foregroundStyle(_:)          (56 sites)
- .cornerRadius(_:)    -> .clipShape(.rect(cornerRadius:)) (3 sites)
- .edgesIgnoringSafeArea(.all) -> .ignoresSafeArea()    (4 sites)
- .accentColor(_:)     -> .tint(_:)                     (1 site)

The ColorPair.foregroundColor / .backgroundColor properties are untouched;
only the SwiftUI modifier form was migrated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FD1HGdhvposK8kYH2em6c3
The macOS 14 modifier migration changed .foregroundColor to
.foregroundStyle on the compliance footer, but the ternary
'isCompliant ? .primary : .secondary.opacity(0.5)' no longer type-checks:
under .foregroundStyle the branches resolve to HierarchicalShapeStyle,
and .opacity(0.5) yields a differently-typed 'some ShapeStyle', so the
two arms disagree. Pin both branches to Color so they unify, matching
the pre-migration behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
.accentColor is not a member of ShapeStyle (unlike .primary/.secondary/
.white/.black, which SwiftUI provides via ShapeStyle extensions), so the
macOS 14 migration to .foregroundStyle/.tint broke every site that passed
a bare .accentColor. Qualify these as Color.accentColor across
NavigationMenu, ColorHistoryDrawer, HelpView, and SplashView.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@superhighfives

Copy link
Copy Markdown
Owner Author

@claude Review please.

@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Claude finished @superhighfives's task in 3m 10s —— View job


Code Review

  • Gather context (PR description, changed files)
  • Review deleted icon assets & Touch Bar fallback change
  • Review modernized SwiftUI modifier sites
  • Check CLAUDE.md for repo-specific guidelines (none found in repo)
  • Post review feedback

Overall this is a clean, mechanical modernization pass, and the diff matches the PR description well. The foregroundStyle ternary/accentColor fixup commits already in the branch show the author caught the type-inference gotchas (isSelected ? Color.accentColor : Color.secondary etc. needed explicit Color. prefixes where a bare .secondary.opacity(...) would otherwise produce a mismatched opaque type) — I checked every migrated foregroundStyle ternary site and they're all consistent with that fix.

One issue worth checking before merge:

Touch Bar copy button icon may lose template tintingPika/TouchBar/PikaTouchBar.swift:85

image: NSImage(systemSymbolName: "doc.on.doc", accessibilityDescription: nil)!,

The asset this replaces (Icons/doc.on.doc.imageset/Contents.json, now deleted) explicitly set "template-rendering-intent" : "template". That flag is what let AppKit auto-tint the icon to match the Touch Bar's dark background. NSImage(systemSymbolName:accessibilityDescription:) does not set isTemplate automatically on macOS (unlike UIKit's UIImage(systemName:)), so this button's icon may now render as a plain black glyph instead of adapting like it used to — which would contradict the PR's "Behaviour is unchanged throughout" claim. Worth explicitly setting isTemplate = true on the image and visually confirming on a Touch Bar Mac (or Touch Bar simulator) that the icon still looks right in both light/dark contexts.

Fix this →

Minor/non-blocking notes:

  • .cornerRadius(_:).clipShape(.rect(cornerRadius:)) is the correct modern replacement everywhere it's used (KeyboardShortcutKey.swift:18, Toast.swift:32, TouchBarVisual.swift:20); no behavior change expected there.
  • Deleted asset catalog cleanup looks complete — IconImage.swift already only uses Image(systemName:), so nothing else references the removed Icons.xcassets entries, and no leftover references exist in the .pbxproj.
  • I wasn't able to run xcodebuild in this sandbox (no permission to execute it), so I couldn't independently confirm a clean build / zero deprecation warnings — worth confirming that part of the PR's test plan still holds, especially given the isTemplate question above.

Copy link
Copy Markdown
Owner Author

Addressed the flagged Touch Bar copy button template tinting issue.

The deleted doc.on.doc asset explicitly set template-rendering-intent: template, which is what let AppKit auto-tint the glyph to match the Touch Bar's dark background. Since NSImage(systemSymbolName:accessibilityDescription:) does not guarantee isTemplate on macOS, the replacement now sets it explicitly:

func createTouchBarCopyButton(action: Selector) -> NSButton? {
    let image = NSImage(systemSymbolName: "doc.on.doc", accessibilityDescription: nil)!
    image.isTemplate = true
    let button = NSButton(
        image: image,
        target: nil,
        action: action
    )
    button.widthAnchor.constraint(equalToConstant: 35).isActive = true
    return button
}

This keeps the icon adapting exactly as the old template asset did, preserving the "behaviour is unchanged" guarantee. Fix pushed in #245.

🤖 Generated with Claude Code


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants