Skip to content

Modernize for macOS 14 + preserve Touch Bar copy icon tinting - #245

Merged
superhighfives merged 5 commits into
mainfrom
claude/github-review-roxl4b
Jul 20, 2026
Merged

Modernize for macOS 14 + preserve Touch Bar copy icon tinting#245
superhighfives merged 5 commits into
mainfrom
claude/github-review-roxl4b

Conversation

@superhighfives

Copy link
Copy Markdown
Owner

Summary

Builds on the macOS 14 modernization work from #242 and addresses the review feedback flagged there.

Changes

Remove dead pre-macOS-11 SF Symbol fallback icons
IconImage used to fall back to hand-drawn SVG copies of SF Symbols on macOS < 11. With the deployment target at macOS 14, IconImage always uses Image(systemName:), leaving Assets.xcassets/Icons/ as dead fallback assets. Deletes the unused imagesets and updates the Touch Bar copy button to load doc.on.doc from SF Symbols directly.

Modernize deprecated SwiftUI modifiers for macOS 14

  • .foregroundColor(_:).foregroundStyle(_:)
  • .cornerRadius(_:).clipShape(.rect(cornerRadius:))
  • .edgesIgnoringSafeArea(.all).ignoresSafeArea()
  • .accentColor(_:).tint(_:)

Preserve Touch Bar copy icon tinting (review fix)
The deleted doc.on.doc asset set template-rendering-intent: template, which let AppKit auto-tint the Touch Bar copy glyph to match the dark background. NSImage(systemSymbolName:accessibilityDescription:) does not guarantee isTemplate on macOS, so the replacement now sets isTemplate = true explicitly to keep the icon adapting as before.

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 the Touch Bar copy button still tints correctly in light/dark contexts

🤖 Generated with Claude Code


Generated by Claude Code

claude and others added 5 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>
The deleted doc.on.doc asset set template-rendering-intent: template,
which let AppKit auto-tint the Touch Bar copy glyph to match the dark
background. NSImage(systemSymbolName:accessibilityDescription:) does not
guarantee isTemplate on macOS, so set it explicitly to keep the icon
adapting as before.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WfkE8HKafLHRo2knFLeX5x
@superhighfives

Copy link
Copy Markdown
Owner Author

@claude Review please.

Copy link
Copy Markdown
Owner Author

Code Review

Reviewed the full diff — the macOS 14 modifier migration plus the Touch Bar fix. No correctness issues found. This is a clean, mechanical modernization and the risky spots are all handled correctly.

.foregroundColor.foregroundStyle — every ternary site is type-consistent. Crucially, wherever accentColor appears both branches are explicitly prefixed with Color. (required, since ShapeStyle has no static .accentColor — a bare .accentColor wouldn't compile):

  • ColorHistoryDrawer.swift, HelpView.swift, NavigationMenu.swift — all Color.accentColor : Color.X.
  • Footer.swift isCompliant ? Color.primary : Color.secondary.opacity(0.5) — the opaque-.opacity case is safe because both branches are explicitly Color.
  • Remaining bare ternaries (.primary : .secondary, .black : .white) infer to the same concrete type cleanly.

.cornerRadius.clipShape(.rect(cornerRadius:))KeyboardShortcutKey, Toast, TouchBarVisual. Behaviourally equivalent (the old .cornerRadius is itself an antialiased rounded-rect clip), all applied to views, no clipping-order regression.

.edgesIgnoringSafeArea(.all).ignoresSafeArea() — the 4 WindowCoordinator sites are all-region hosting roots; .ignoresSafeArea() defaults to all regions/edges, so equivalent.

.accentColor.tintSplashView, equivalent at that site.

Touch Bar isTemplate = true — correct. NSImage(systemSymbolName: "doc.on.doc", ...) is a valid symbol (force-unwrap safe), and setting isTemplate preserves the tinting the deleted template asset provided. (Symbol images are generally template by default on macOS, so it's belt-and-suspenders — but it matches the old asset's declared template-rendering-intent and guarantees the behaviour regardless.)

Deleted assets — verified genuinely dead: IconImage only uses Image(systemName:), no Image("…") catalog references remain, and the only asset-catalog NSImage(named:) pointing at a deleted icon was the one converted to systemSymbolName. No dangling references, no half-migrated sites.

Note: the claude-review check failed as an infra artefact — it declines/errors on claude/* head branches (the exact issue #244 addresses), not a defect in this diff. The Unit Tests check is the meaningful gate.

🤖 Generated with Claude Code


Generated by Claude Code

@superhighfives
superhighfives merged commit 3349451 into main Jul 20, 2026
2 of 3 checks passed
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