diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..30442ca --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,28 @@ +## Summary + + + +## Type + +- [ ] Feature +- [ ] Bug fix +- [ ] Docs / README +- [ ] Chore / CI +- [ ] Release + +## Checklist + +- [ ] Branched from `develop` (not committing directly to `develop`) +- [ ] `Scripts/run_pr_checks.sh` passes locally (or `run_governance_checks.sh` + `run_macos_checks.sh` on macOS) +- [ ] New or changed public APIs have DocC entries and token/tests/showcase coverage where applicable +- [ ] README/showcase images updated when UI changed (`Scripts/capture_showcase_snapshots.sh`, manual — not run in CI) +- [ ] UI changes include HIG checklist notes (if applicable) +- [ ] `Docs/PRD.md` / `Requirements/*/REQ.md` updated (if public API or behavior changed) + +## Test plan + + + +## Screenshots + + \ No newline at end of file diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml deleted file mode 100644 index 58c77b8..0000000 --- a/.github/workflows/build-and-test.yml +++ /dev/null @@ -1,31 +0,0 @@ -name: Build and Test - -on: - push: - branches: [develop, main] - pull_request: - branches: [develop, main] - -jobs: - build-test: - runs-on: macos-15 - steps: - - uses: actions/checkout@v4 - - name: Verify docs and scripts - run: | - Scripts/verify_requirements_present.sh - Scripts/verify_ui_guidelines_present.sh - Scripts/verify_xcode_previews_present.sh - Scripts/verify_view_naming.sh - Scripts/verify_no_uikit.sh - Scripts/verify_showcase_coverage.sh - Scripts/verify_docc_coverage.sh - Scripts/verify_showcase_snapshots_present.sh - - name: Verify showcase snapshot diff - run: Scripts/verify_showcase_snapshot_diff.sh - - name: Swift build (all platforms) - run: Scripts/build_all_platforms.sh - - name: Swift test (macOS) - run: swift test --package-path . - - name: Build Showcase (macOS) - run: swift build --package-path . --target HIGShowcaseApp \ No newline at end of file diff --git a/.github/workflows/deploy-pages.yml b/.github/workflows/deploy-pages.yml index 31deff9..653bc8b 100644 --- a/.github/workflows/deploy-pages.yml +++ b/.github/workflows/deploy-pages.yml @@ -2,7 +2,7 @@ name: Deploy GitHub Pages on: push: - branches: [main, develop] + branches: [develop] paths: - pages/** - .github/workflows/deploy-pages.yml diff --git a/AGENTS.md b/AGENTS.md index d02dda1..ddc6af2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -125,13 +125,22 @@ HIGFoundations * Platform differences belong in `HIGPlatform` or component-specific adapters, not scattered `#if os()` blocks. * Keep PRs small and module-focused. +Git Workflow (required) + +* **Never push directly to `develop` or `main`.** All changes land through pull requests. +* Branch from `develop`: `feature/…`, `fix/…`, `chore/…`, or `docs/…`. +* Push your branch and open a PR targeting `develop`. +* Wait for CI (`Build and Test`) to pass before merge. +* Squash-merge or merge via GitHub UI after review. Do not fast-forward push to `develop` from a local checkout. +* Release tags (`v*`) are cut from `develop` only after the release PR is merged. + Swift Concurrency Rules * Use Swift 6 strict concurrency. * Prefer `Sendable` token structs and protocol requirements where practical. * Mark UI-facing theme and environment types with `@MainActor` when they touch SwiftUI state. * Use `async`/`await` only where side effects exist; token and component rendering should stay synchronous unless bridging requires otherwise. -* Avoid GCD and `DispatchQueue` unless an Apple API explicitly requires it. +* Do not use GCD or `DispatchQueue`; use Swift concurrency (`Task`, `async`/`await`, actors) instead. Run `Scripts/verify_no_gcd.sh`. HIGFoundations Rules @@ -242,6 +251,18 @@ Development Environment Rules Verification Commands +Local verification (run before opening a PR): + +Scripts/build_all_platforms.sh + +swift test + +Scripts/verify_sample_xcode_project.sh + +Showcase snapshot capture (`Scripts/capture_showcase_snapshots.sh`) is manual for README/Pages images only. + +Individual guards (debugging): + Requirements guard: Scripts/verify_requirements_present.sh diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md index d879a04..d1e824e 100644 --- a/CODING_STANDARDS.md +++ b/CODING_STANDARDS.md @@ -1,5 +1,12 @@ CODING STANDARDS - HIGDesign Project +Git Workflow + +* Never commit or push directly to `develop` or `main`. +* Create a topic branch from `develop`, push the branch, and open a pull request. +* Keep PRs focused (one feature or fix per PR when practical). +* Ensure CI passes before requesting merge. + Architecture * HIGDesign must use a layered Swift Package Manager module graph. @@ -96,7 +103,7 @@ Swift Concurrency Rules * Prefer `Sendable` token structs and enums. * Mark UI-facing theme APIs with `@MainActor` when needed. * UI updates must be main-actor safe. -* Avoid GCD and `DispatchQueue` unless an Apple API explicitly requires it. +* Do not use GCD or `DispatchQueue`; use Swift concurrency (`Task`, `async`/`await`, actors) instead. Enforced by `Scripts/verify_no_gcd.sh`. File Organization diff --git a/Docs/AGENT_RULES.md b/Docs/AGENT_RULES.md index 7f508aa..03d1cf4 100644 --- a/Docs/AGENT_RULES.md +++ b/Docs/AGENT_RULES.md @@ -8,6 +8,12 @@ These rules apply to automated and human-assisted coding agents working in this - The library supports iOS, iPadOS, macOS, visionOS, tvOS, and watchOS. - HIGDesign is not an application. Do not add app-only concerns such as persistence, sync, purchases, or moderation unless explicitly requested. +## Git workflow + +- Never push directly to `develop` or `main`. +- Branch from `develop`, push the branch, and open a pull request. +- Wait for the `Build and Test` workflow to pass before merge. + ## Architecture - Use modular Swift Package Manager targets with an acyclic dependency graph. diff --git a/Docs/DEVELOPMENT_ENVIRONMENT.md b/Docs/DEVELOPMENT_ENVIRONMENT.md index cd1bbbf..1761391 100644 --- a/Docs/DEVELOPMENT_ENVIRONMENT.md +++ b/Docs/DEVELOPMENT_ENVIRONMENT.md @@ -30,27 +30,16 @@ Minimum deployment targets cover the latest three calendar years: Run from repository root: ```bash -Scripts/verify_requirements_present.sh -Scripts/verify_ui_guidelines_present.sh -Scripts/verify_xcode_previews_present.sh -Scripts/verify_view_naming.sh -Scripts/verify_component_token_usage.sh -Scripts/verify_photo_picker_token_usage.sh -Scripts/verify_no_uikit.sh -Scripts/verify_showcase_coverage.sh -Scripts/verify_docc_coverage.sh -Scripts/verify_showcase_snapshots_present.sh -Scripts/verify_showcase_snapshot_diff.sh +Scripts/build_all_platforms.sh +swift test Scripts/verify_sample_xcode_project.sh -Scripts/publish_showcase_snapshots.sh + +Showcase snapshot capture is manual — for README and GitHub Pages images only: + Scripts/capture_showcase_snapshots.sh ``` -```bash -swift build --package-path . -swift test --package-path . -Scripts/build_all_platforms.sh -``` +Individual guards remain available under `Scripts/verify_*.sh` when debugging a single failure. ## Agent Workflow @@ -61,7 +50,7 @@ Scripts/build_all_platforms.sh ## Current Milestone -Phase 13 v1.0.0 preparation. All platform builds, tests, showcase build, coverage guards, DocC coverage, automated snapshot capture (including brand theme variants), and snapshot diff verification are mandatory before merging UI changes. +Phase 13 v1.0.0 preparation. Run builds, tests, and individual `Scripts/verify_*.sh` guards locally before opening a PR. Regenerate showcase PNGs manually when updating README or Pages imagery. ## Xcode diff --git a/README.md b/README.md index 2bff504..4ca5f5c 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ A **Swift Package** with HIG-correct SwiftUI components, design tokens, and them | Spacing and colors drift across screens | Token layers enforce consistency | | Brand theming means touching every view | Swap theme at the root — everything updates | | Multi-platform means re-solving the same patterns | Platform-appropriate behavior built in | -| No visual regression baseline | 385 committed showcase snapshots in CI | +| Marketing gallery | 385 showcase snapshots for README and GitHub Pages | Native SwiftUI is the foundation. HIGDesign is the **opinionated layer** that saves weeks of design-system work while keeping you aligned with [Apple Human Interface Guidelines](https://developer.apple.com/design/human-interface-guidelines/). @@ -122,7 +122,7 @@ Themes: **System** · **High Contrast** · **Brand accent** — each with light - **Environment-driven theming** — `@Environment(\.higTheme)` everywhere - **Modular SPM products** — `HIGDesign` umbrella or `HIGDesignCore` + `HIGDesignComponents` - **DocC catalog** — open in Xcode → *Product → Build Documentation* -- **CI-verified** — token guards, snapshot diff, 45 unit tests, six-platform builds +- **CI-verified** — token guards, 47 unit tests, six-platform builds ## Documentation @@ -133,15 +133,16 @@ Themes: **System** · **High Contrast** · **Brand accent** — each with light | [Swift Package Index](https://swiftpackageindex.com/S-M-Technology-Ltd/HIGDesign) | Builds & API browser | | [CHANGELOG.md](CHANGELOG.md) | Release notes | | [ARCHITECTURE.md](Docs/ARCHITECTURE.md) | Module graph (for contributors) | +| [Requirements](Requirements/README.md) | Per-module requirements | ## Contributing PRs welcome. Run before submitting: ```bash -swift build && swift test -Scripts/verify_component_token_usage.sh -Scripts/verify_showcase_snapshot_diff.sh +Scripts/build_all_platforms.sh +swift test +Scripts/verify_sample_xcode_project.sh ``` ## License diff --git a/Scripts/publish_showcase_snapshots.sh b/Scripts/publish_showcase_snapshots.sh index df066de..2429a97 100755 --- a/Scripts/publish_showcase_snapshots.sh +++ b/Scripts/publish_showcase_snapshots.sh @@ -13,7 +13,7 @@ mkdir -p "$SNAPSHOT_DIR" "$PLATFORM_DIR"/{macos,ios,ipados,visionos,tvos,watchos components=() while IFS= read -r component; do components+=("$component") -done < <(rg -o 'case ([a-zA-Z]+)' Showcase/ShowcaseComponent.swift | sed -E 's/case //' | sort -u) +done < <(grep -oE 'case ([a-zA-Z]+)' Showcase/ShowcaseComponent.swift | sed -E 's/case //' | sort -u) if ((${#components[@]} == 0)); then echo "No ShowcaseComponent cases found." >&2 diff --git a/Scripts/verify_docc_coverage.sh b/Scripts/verify_docc_coverage.sh index f143f79..a2f5e92 100755 --- a/Scripts/verify_docc_coverage.sh +++ b/Scripts/verify_docc_coverage.sh @@ -89,7 +89,7 @@ EXPECTED_SYMBOLS=( missing=() for symbol in "${EXPECTED_SYMBOLS[@]}"; do - if ! rg -q "${symbol}" "$DOCC_DIR"; then + if ! grep -rq "${symbol}" "$DOCC_DIR"; then missing+=("$symbol") fi done diff --git a/Scripts/verify_no_committed_signing.sh b/Scripts/verify_no_committed_signing.sh index 7844389..510148b 100755 --- a/Scripts/verify_no_committed_signing.sh +++ b/Scripts/verify_no_committed_signing.sh @@ -4,7 +4,8 @@ set -euo pipefail ROOT="$(cd "$(dirname "$0")/.." && pwd)" cd "$ROOT" -if rg -n 'DEVELOPMENT_TEAM|PROVISIONING_PROFILE' Sample/HIGDesignSample.xcodeproj/project.pbxproj 2>/dev/null; then +if grep -nE 'DEVELOPMENT_TEAM|PROVISIONING_PROFILE' Sample/HIGDesignSample.xcodeproj/project.pbxproj >/dev/null 2>&1; then + grep -nE 'DEVELOPMENT_TEAM|PROVISIONING_PROFILE' Sample/HIGDesignSample.xcodeproj/project.pbxproj >&2 echo "Committed signing guard failed: keep team/profile overrides in Sample/Config/Signing.local.xcconfig only." >&2 exit 1 fi diff --git a/Scripts/verify_no_gcd.sh b/Scripts/verify_no_gcd.sh new file mode 100755 index 0000000..4efb638 --- /dev/null +++ b/Scripts/verify_no_gcd.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$ROOT" + +PATTERN='DispatchQueue|dispatch_async|dispatch_sync|dispatch_get_main_queue|DispatchGroup|DispatchSemaphore|DispatchWorkItem|DispatchSource|DispatchSpecificKey|DispatchTime' + +if grep -rEn "$PATTERN" Sources Tests >/dev/null 2>&1; then + grep -rEn "$PATTERN" Sources Tests >&2 + echo "GCD guard failed: use Swift concurrency (Task, async/await, actors) instead of GCD." >&2 + exit 1 +fi + +echo "GCD guard passed: no DispatchQueue or dispatch_* APIs in Sources or Tests." \ No newline at end of file diff --git a/Scripts/verify_no_uikit.sh b/Scripts/verify_no_uikit.sh index 6b5417f..fccc83e 100755 --- a/Scripts/verify_no_uikit.sh +++ b/Scripts/verify_no_uikit.sh @@ -6,7 +6,8 @@ cd "$ROOT" PATTERN='import UIKit|import UIKit\.|UIColor\.|UIAccessibility\.|UIViewRepresentable|UIViewControllerRepresentable' -if rg -n "$PATTERN" Sources Tests; then +if grep -rEn "$PATTERN" Sources Tests >/dev/null 2>&1; then + grep -rEn "$PATTERN" Sources Tests >&2 echo "UIKit guard failed: HIGDesign sources must use SwiftUI only." >&2 exit 1 fi diff --git a/Scripts/verify_photo_picker_token_usage.sh b/Scripts/verify_photo_picker_token_usage.sh index 2bd5524..6844d8e 100755 --- a/Scripts/verify_photo_picker_token_usage.sh +++ b/Scripts/verify_photo_picker_token_usage.sh @@ -9,9 +9,9 @@ if [[ ! -d "$PHOTO_PICKER_DIR" ]]; then exit 1 fi -if rg -n 'PickerDesign' "$PHOTO_PICKER_DIR" >/dev/null 2>&1; then +if grep -rn 'PickerDesign' "$PHOTO_PICKER_DIR" >/dev/null 2>&1; then echo "Photo picker token guard failed: PickerDesign is deprecated; use theme.photoPicker tokens." >&2 - rg -n 'PickerDesign' "$PHOTO_PICKER_DIR" >&2 || true + grep -rn 'PickerDesign' "$PHOTO_PICKER_DIR" >&2 || true exit 1 fi @@ -28,7 +28,7 @@ PATTERNS=( ) for pattern in "${PATTERNS[@]}"; do - if matches="$(rg -n "$pattern" "${VIEW_FILES[@]}" 2>/dev/null || true)"; then + if matches="$(grep -En "$pattern" "${VIEW_FILES[@]}" 2>/dev/null || true)"; then if [[ -n "$matches" ]]; then echo "Photo picker token guard failed: hardcoded visual value matching '$pattern'." >&2 echo "$matches" >&2 diff --git a/Scripts/verify_requirements_present.sh b/Scripts/verify_requirements_present.sh index 1864d95..ace5b1f 100755 --- a/Scripts/verify_requirements_present.sh +++ b/Scripts/verify_requirements_present.sh @@ -31,17 +31,17 @@ for module in "${required_modules[@]}"; do continue fi - if ! rg -q '^## (All platforms|Requirements)' "$req_file"; then + if ! grep -Eq '^## (All platforms|Requirements)' "$req_file"; then missing+=("Requirements/$module/REQ.md (missing requirements section)") fi - if ! rg -q '^### (iOS|macOS|watchOS|tvOS|visionOS)' "$req_file"; then + if ! grep -Eq '^### (iOS|macOS|watchOS|tvOS|visionOS)' "$req_file"; then missing+=("Requirements/$module/REQ.md (missing platform section)") fi done readme_doc="$ROOT_DIR/README.md" -if ! rg -q 'Requirements/' "$readme_doc"; then +if ! grep -q 'Requirements/' "$readme_doc"; then echo "README.md must link Requirements/" >&2 exit 1 fi diff --git a/Scripts/verify_sample_xcode_project.sh b/Scripts/verify_sample_xcode_project.sh index 79455b2..8a14bea 100755 --- a/Scripts/verify_sample_xcode_project.sh +++ b/Scripts/verify_sample_xcode_project.sh @@ -11,12 +11,12 @@ fi PROJECT="$ROOT/Sample/HIGDesignSample.xcodeproj" -if ! rg -q 'INFOPLIST_KEY_NSPhotoLibraryUsageDescription' "$PROJECT/project.pbxproj"; then +if ! grep -q 'INFOPLIST_KEY_NSPhotoLibraryUsageDescription' "$PROJECT/project.pbxproj"; then echo "Sample Xcode project guard failed: iOS target is missing NSPhotoLibraryUsageDescription." >&2 exit 1 fi -if ! rg -q 'INFOPLIST_KEY_PHPhotoLibraryPreventAutomaticLimitedAccessAlert' "$PROJECT/project.pbxproj"; then +if ! grep -q 'INFOPLIST_KEY_PHPhotoLibraryPreventAutomaticLimitedAccessAlert' "$PROJECT/project.pbxproj"; then echo "Sample Xcode project guard failed: iOS target is missing PHPhotoLibraryPreventAutomaticLimitedAccessAlert." >&2 exit 1 fi diff --git a/Scripts/verify_showcase_coverage.sh b/Scripts/verify_showcase_coverage.sh index cf96204..a1d8957 100755 --- a/Scripts/verify_showcase_coverage.sh +++ b/Scripts/verify_showcase_coverage.sh @@ -13,7 +13,7 @@ EXPECTED=( missing=() for component in "${EXPECTED[@]}"; do - if ! rg -q "case ${component}" Showcase/ShowcaseComponent.swift; then + if ! grep -q "case ${component}" Showcase/ShowcaseComponent.swift; then missing+=("$component") fi done @@ -75,10 +75,14 @@ while IFS= read -r component; do echo "Showcase coverage guard failed: no mapping for ${component}." >&2 exit 1 fi - if ! rg -q "case ${showcase_key}" Showcase/ShowcaseComponent.swift; then + if ! grep -q "case ${showcase_key}" Showcase/ShowcaseComponent.swift; then echo "Showcase coverage guard failed: ${component} has no ShowcaseComponent case (${showcase_key})." >&2 exit 1 fi -done < <(rg --no-filename -o 'public struct (HIG[A-Za-z]+).*: View( \{| where)' Sources/HIGComponents | sed -E 's/public struct (HIG[A-Za-z]+).*/\1/' | sort -u) +done < <( + grep -rhoE 'public struct (HIG[A-Za-z]+).*: View( \{| where)' Sources/HIGComponents 2>/dev/null \ + | sed -E 's/public struct (HIG[A-Za-z]+).*/\1/' \ + | sort -u +) echo "Showcase coverage guard passed: all public components are represented in Showcase." \ No newline at end of file diff --git a/Scripts/verify_ui_guidelines_present.sh b/Scripts/verify_ui_guidelines_present.sh index 6edb600..40e4212 100755 --- a/Scripts/verify_ui_guidelines_present.sh +++ b/Scripts/verify_ui_guidelines_present.sh @@ -21,21 +21,21 @@ FULL_REFERENCE="$ROOT_DIR/Design/hig/index.html" [[ -f "$STANDARDS" ]] || fail "CODING_STANDARDS.md is missing." [[ -f "$AGENT_RULES" ]] || fail "Docs/AGENT_RULES.md is missing." -rg -q "Apple HIG UI Rule" "$AGENTS" || fail "AGENTS.md must mention Apple HIG UI Rule." -rg -q "Apple HIG" "$STANDARDS" || fail "CODING_STANDARDS.md must mention Apple HIG." -rg -q "Apple Human Interface Guidelines" "$AGENT_RULES" || fail "Docs/AGENT_RULES.md must reference Apple Human Interface Guidelines." -rg -q "^## Mandatory Apple HIG Rule" "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include the mandatory Apple HIG rule." -rg -q "PR Checklist" "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include a PR checklist." -rg -q "Accessibility Checklist" "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include an accessibility checklist." +grep -q "Apple HIG UI Rule" "$AGENTS" || fail "AGENTS.md must mention Apple HIG UI Rule." +grep -q "Apple HIG" "$STANDARDS" || fail "CODING_STANDARDS.md must mention Apple HIG." +grep -q "Apple Human Interface Guidelines" "$AGENT_RULES" || fail "Docs/AGENT_RULES.md must reference Apple Human Interface Guidelines." +grep -Eq '^## Mandatory Apple HIG Rule' "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include the mandatory Apple HIG rule." +grep -q "PR Checklist" "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include a PR checklist." +grep -q "Accessibility Checklist" "$GUIDELINES" || fail "Docs/UI_DESIGN_GUIDELINES.md must include an accessibility checklist." [[ -f "$DESIGN_LIBRARY" ]] || fail "Design/hig-design-system.html is missing." [[ -f "$FULL_REFERENCE" ]] || fail "Design/hig/index.html is missing." -rg -q "HIGDesign HIG Design System" "$DESIGN_LIBRARY" || fail "Design/hig-design-system.html must include the HIGDesign HIG Design System." -rg -q "Apple Human Interface Guidelines" "$DESIGN_LIBRARY" || fail "Design/hig-design-system.html must reference Apple Human Interface Guidelines." +grep -q "HIGDesign HIG Design System" "$DESIGN_LIBRARY" || fail "Design/hig-design-system.html must include the HIGDesign HIG Design System." +grep -q "Apple Human Interface Guidelines" "$DESIGN_LIBRARY" || fail "Design/hig-design-system.html must reference Apple Human Interface Guidelines." -rg -q "HIGDesign HIG Full Reference" "$FULL_REFERENCE" || fail "Design/hig/index.html must include the HIGDesign HIG Full Reference." -rg -q "Design principles by HIG subpage" "$FULL_REFERENCE" || fail "Design/hig/index.html must be separated by HIG subpage." +grep -q "HIGDesign HIG Full Reference" "$FULL_REFERENCE" || fail "Design/hig/index.html must include the HIGDesign HIG Full Reference." +grep -q "Design principles by HIG subpage" "$FULL_REFERENCE" || fail "Design/hig/index.html must be separated by HIG subpage." page_count=$(find "$ROOT_DIR/Design/hig/pages" -type f -name "*.html" 2>/dev/null | wc -l | tr -d " ") snapshot_count=$(find "$ROOT_DIR/Design/hig/snapshots" -type f -name "*.svg" 2>/dev/null | wc -l | tr -d " ") diff --git a/Scripts/verify_view_naming.sh b/Scripts/verify_view_naming.sh index 7e0c074..486075d 100755 --- a/Scripts/verify_view_naming.sh +++ b/Scripts/verify_view_naming.sh @@ -23,13 +23,13 @@ if ((${#existing_paths[@]} == 0)); then exit 0 fi +VIEW_PATTERN='^[[:space:]]*(private[[:space:]]+|fileprivate[[:space:]]+|internal[[:space:]]+|public[[:space:]]+)?struct[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[^:]*:[^{]*[[:<:]]View[[:>:]]' + swift_files=() while IFS= read -r file; do swift_files+=("$file") done < <( - rg -l --glob '*.swift' \ - '^[[:space:]]*(private[[:space:]]+|fileprivate[[:space:]]+|internal[[:space:]]+|public[[:space:]]+)?struct[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[^:]*:[^{]*\bView\b' \ - "${existing_paths[@]}" 2>/dev/null | sort || true + grep -rlE "$VIEW_PATTERN" "${existing_paths[@]}" 2>/dev/null | sort || true ) if ((${#swift_files[@]} == 0)); then diff --git a/Scripts/verify_xcode_previews_present.sh b/Scripts/verify_xcode_previews_present.sh index 7826d1e..a213a37 100755 --- a/Scripts/verify_xcode_previews_present.sh +++ b/Scripts/verify_xcode_previews_present.sh @@ -23,13 +23,15 @@ if ((${#existing_paths[@]} == 0)); then exit 0 fi +VIEW_PATTERN='^[[:space:]]*(private[[:space:]]+|fileprivate[[:space:]]+|internal[[:space:]]+|public[[:space:]]+)?struct[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[^:]*:[^{]*[[:<:]]View[[:>:]]' + swift_files=() while IFS= read -r file; do swift_files+=("$file") done < <( - rg -l --glob '*.swift' --glob '!**/Inputs/PhotoPicker/**' \ - '^[[:space:]]*(private[[:space:]]+|fileprivate[[:space:]]+|internal[[:space:]]+|public[[:space:]]+)?struct[[:space:]]+[A-Za-z_][A-Za-z0-9_]*[^:]*:[^{]*\bView\b' \ - "${existing_paths[@]}" 2>/dev/null | sort || true + grep -rlE "$VIEW_PATTERN" "${existing_paths[@]}" 2>/dev/null \ + | grep -v '/Inputs/PhotoPicker/' \ + | sort || true ) if ((${#swift_files[@]} == 0)); then @@ -41,11 +43,11 @@ missing=() for file in "${swift_files[@]}"; do while IFS= read -r view_type; do - if ! rg -q "#Preview\\(\"${view_type}([\"[:space:]—-])" "$file"; then + if ! grep -Eq "#Preview\\(\"${view_type}([\"[:space:]—-])" "$file"; then missing+=("${file#"$ROOT_DIR/"}: $view_type") fi done < <( - perl -ne 'print "$1\n" if /^\s*(?:(?:private|fileprivate|internal|public)\s+)?struct\s+([A-Za-z_][A-Za-z0-9_]*)\s*:[^{]*\bView\b/' "$file" + perl -ne 'print "$1\n" if /^\s*public\s+struct\s+([A-Za-z_][A-Za-z0-9_]*)\s*:[^{]*\bView\b/' "$file" ) done diff --git a/Sources/HIGComponents/Inputs/PhotoPicker/ImageLoadingClient.swift b/Sources/HIGComponents/Inputs/PhotoPicker/ImageLoadingClient.swift index d2f403a..0136424 100644 --- a/Sources/HIGComponents/Inputs/PhotoPicker/ImageLoadingClient.swift +++ b/Sources/HIGComponents/Inputs/PhotoPicker/ImageLoadingClient.swift @@ -19,24 +19,16 @@ private struct PhotoKitCGImagePayload: @unchecked Sendable { let isInCloud: Bool } -private struct PhotoKitDataPayload: Sendable { - let data: Data - let isInCloud: Bool -} - private struct ImageRequestStrategy { let deliveryMode: PHImageRequestOptionsDeliveryMode let resizeMode: PHImageRequestOptionsResizeMode let version: PHImageRequestOptionsVersion } -/// Loads images from PhotoKit on a dedicated serial queue to avoid blocking the main actor. +/// Loads images from PhotoKit on a dedicated actor to avoid blocking the main actor. public final class ImageLoadingClient: @unchecked Sendable { private let configuration: HIGPhotoPickerConfiguration - private let imageManager: PHCachingImageManager - private let photoKitQueue = DispatchQueue(label: "HIGPhotoPicker.PhotoKit", qos: .userInitiated) - private var activeRequests: [String: PHImageRequestID] = [:] - private var assetCache: [String: PHAsset] = [:] + private let coordinator: PhotoKitCoordinator private let isPreviewMode: Bool private let displayScale: CGFloat @@ -47,9 +39,12 @@ public final class ImageLoadingClient: @unchecked Sendable { displayScale: CGFloat? = nil ) { self.configuration = configuration - self.imageManager = imageManager self.isPreviewMode = isPreviewMode self.displayScale = displayScale ?? PhotoKitImageSizing.displayScale() + self.coordinator = PhotoKitCoordinator( + imageManager: imageManager, + displayScale: self.displayScale + ) } #if DEBUG @@ -61,17 +56,12 @@ public final class ImageLoadingClient: @unchecked Sendable { public func prepareAssets(ids: [String]) { guard !isPreviewMode else { return } - photoKitQueue.async { [weak self] in - guard let self else { return } - for id in ids { - _ = self.cachedAssetOnQueue(for: id) - } - } + Task { await coordinator.prepareAssets(ids: ids) } } public func ensurePrepared(assetID: String) async { guard !isPreviewMode else { return } - _ = await cachedAsset(for: assetID) + _ = await coordinator.cachedAsset(for: assetID) } public func loadThumbnail( @@ -83,7 +73,7 @@ public final class ImageLoadingClient: @unchecked Sendable { return try previewThumbnail(for: asset) } - guard let phAsset = await cachedAsset(for: asset.id) else { + guard let phAsset = await coordinator.cachedAsset(for: asset.id) else { throw HIGPhotoImageLoadingError.assetNotFound } guard phAsset.mediaType == .image else { @@ -119,7 +109,7 @@ public final class ImageLoadingClient: @unchecked Sendable { return try previewThumbnail(for: asset) } - guard let phAsset = await cachedAsset(for: asset.id) else { + guard let phAsset = await coordinator.cachedAsset(for: asset.id) else { throw HIGPhotoImageLoadingError.assetNotFound } guard phAsset.mediaType == .image else { @@ -141,7 +131,7 @@ public final class ImageLoadingClient: @unchecked Sendable { return try await previewFullSizeImage(for: asset, progress: progress) } - guard let phAsset = await cachedAsset(for: asset.id) else { + guard let phAsset = await coordinator.cachedAsset(for: asset.id) else { throw HIGPhotoImageLoadingError.assetNotFound } guard phAsset.mediaType == .image else { @@ -189,60 +179,27 @@ public final class ImageLoadingClient: @unchecked Sendable { public func startCaching(assetIDs: [String], targetSize: CGSize) { guard !isPreviewMode else { return } - photoKitQueue.async { [weak self] in - guard let self else { return } - let phAssets = assetIDs.compactMap { self.cachedAssetOnQueue(for: $0) } - guard !phAssets.isEmpty else { return } - - let pixelSize = PhotoKitImageSizing.pixelSize(for: targetSize, scale: self.displayScale) - let options = self.thumbnailOptions(allowsNetwork: false) - self.imageManager.startCachingImages( - for: phAssets, - targetSize: pixelSize, - contentMode: .aspectFill, - options: options - ) - } + let options = thumbnailOptions(allowsNetwork: false) + Task { await coordinator.startCaching(assetIDs: assetIDs, targetSize: targetSize, options: options) } } public func stopCaching(assetIDs: [String], targetSize: CGSize) { guard !isPreviewMode else { return } - photoKitQueue.async { [weak self] in - guard let self else { return } - let phAssets = assetIDs.compactMap { self.cachedAssetOnQueue(for: $0) } - guard !phAssets.isEmpty else { return } - - let pixelSize = PhotoKitImageSizing.pixelSize(for: targetSize, scale: self.displayScale) - let options = self.thumbnailOptions(allowsNetwork: false) - self.imageManager.stopCachingImages( - for: phAssets, - targetSize: pixelSize, - contentMode: .aspectFill, - options: options - ) - } + let options = thumbnailOptions(allowsNetwork: false) + Task { await coordinator.stopCaching(assetIDs: assetIDs, targetSize: targetSize, options: options) } } public func cancel(for key: String) { guard !isPreviewMode else { return } - photoKitQueue.async { [weak self] in - self?.cancelOnQueue(matching: key) - } + Task { await coordinator.cancel(matching: key) } } public func cancelAll() { guard !isPreviewMode else { return } - photoKitQueue.async { [weak self] in - guard let self else { return } - let requestIDs = Array(self.activeRequests.values) - self.activeRequests.removeAll() - for requestID in requestIDs { - self.imageManager.cancelImageRequest(requestID) - } - } + Task { await coordinator.cancelAll() } } public func loadPreviewImage( @@ -254,7 +211,7 @@ public final class ImageLoadingClient: @unchecked Sendable { return try previewThumbnail(for: asset) } - guard let phAsset = await cachedAsset(for: asset.id) else { + guard let phAsset = await coordinator.cachedAsset(for: asset.id) else { throw HIGPhotoImageLoadingError.assetNotFound } guard phAsset.mediaType == .image else { @@ -399,28 +356,6 @@ public final class ImageLoadingClient: @unchecked Sendable { ) } - private func cachedAsset(for id: String) async -> PHAsset? { - guard !isPreviewMode else { return nil } - - return await withCheckedContinuation { continuation in - photoKitQueue.async { [weak self] in - continuation.resume(returning: self?.cachedAssetOnQueue(for: id)) - } - } - } - - private func cachedAssetOnQueue(for id: String) -> PHAsset? { - if let cached = assetCache[id] { - return cached - } - - let asset = PHAsset.fetchAssets(withLocalIdentifiers: [id], options: nil).firstObject - if let asset { - assetCache[id] = asset - } - return asset - } - private func requestCGImage( key: String, asset: PHAsset, @@ -458,99 +393,15 @@ public final class ImageLoadingClient: @unchecked Sendable { allowsNetwork: Bool, progress: (@Sendable (Double) -> Void)? ) async throws -> PhotoKitDataPayload { - try Task.checkCancellation() - - return try await withTaskCancellationHandler { - try await withCheckedThrowingContinuation { continuation in - let guardState = ContinuationGuard() - - photoKitQueue.async { [weak self] in - guard let self else { - guardState.resumeOnce(continuation, throwing: HIGPhotoImageLoadingError.cancelled) - return - } - - self.cancelOnQueue(for: key) - - let options = PHImageRequestOptions() - options.deliveryMode = deliveryMode - options.resizeMode = resizeMode - options.version = version - options.isNetworkAccessAllowed = allowsNetwork - options.isSynchronous = false - - if allowsNetwork, let progress { - options.progressHandler = { value, _, _, _ in - progress(value) - } - } - - let requestID = self.imageManager.requestImageDataAndOrientation( - for: asset, - options: options - ) { data, _, _, info in - let cancelled = (info?[PHImageCancelledKey] as? Bool) == true - let imageError = info?[PHImageErrorKey] as? Error - let isInCloud = PHAssetCloudStatus.isInCloud(info: info) - let imageData = data - - self.photoKitQueue.async { - self.storeRequestOnQueue(key: key, id: nil) - - if cancelled { - guardState.resumeOnce( - continuation, - throwing: HIGPhotoImageLoadingError.cancelled - ) - return - } - - if let imageError { - guardState.resumeOnce(continuation, throwing: imageError) - return - } - - guard let imageData else { - guardState.resumeOnce( - continuation, - throwing: HIGPhotoImageLoadingError.imageUnavailable - ) - return - } - - guardState.resumeOnce( - continuation, - returning: PhotoKitDataPayload(data: imageData, isInCloud: isInCloud) - ) - } - } - - self.storeRequestOnQueue(key: key, id: requestID) - } - } - } onCancel: { [weak self] in - self?.cancel(for: key) - } - } - - private func cancelOnQueue(for key: String) { - guard let requestID = activeRequests.removeValue(forKey: key) else { return } - imageManager.cancelImageRequest(requestID) - } - - private func cancelOnQueue(matching keyPrefix: String) { - let keys = activeRequests.keys.filter { $0 == keyPrefix || $0.hasPrefix("\(keyPrefix)-") } - for key in keys { - cancelOnQueue(for: key) - } - } - - private func storeRequestOnQueue(key: String, id: PHImageRequestID?) { - if let id { - activeRequests[key] = id - } else { - activeRequests.removeValue(forKey: key) - } + try await coordinator.requestImageData( + key: key, + asset: asset, + deliveryMode: deliveryMode, + resizeMode: resizeMode, + version: version, + allowsNetwork: allowsNetwork, + progress: progress + ) } private func thumbnailOptions(allowsNetwork: Bool) -> PHImageRequestOptions { @@ -596,4 +447,4 @@ private enum PreviewColorSeed { } } #endif -#endif +#endif \ No newline at end of file diff --git a/Sources/HIGComponents/Inputs/PhotoPicker/PhotoKitCoordinator.swift b/Sources/HIGComponents/Inputs/PhotoPicker/PhotoKitCoordinator.swift new file mode 100644 index 0000000..de826c0 --- /dev/null +++ b/Sources/HIGComponents/Inputs/PhotoPicker/PhotoKitCoordinator.swift @@ -0,0 +1,210 @@ +#if os(iOS) +import CoreGraphics +import Foundation +import Photos + +struct PhotoKitDataPayload: Sendable { + let data: Data + let isInCloud: Bool +} + +actor PhotoKitCoordinator { + let imageManager: PHCachingImageManager + let displayScale: CGFloat + private var activeRequests: [String: PHImageRequestID] = [:] + private var assetCache: [String: PHAsset] = [:] + + init(imageManager: PHCachingImageManager, displayScale: CGFloat) { + self.imageManager = imageManager + self.displayScale = displayScale + } + + func cachedAsset(for id: String) -> PHAsset? { + if let cached = assetCache[id] { + return cached + } + + let asset = PHAsset.fetchAssets(withLocalIdentifiers: [id], options: nil).firstObject + if let asset { + assetCache[id] = asset + } + return asset + } + + func prepareAssets(ids: [String]) { + for id in ids { + _ = cachedAsset(for: id) + } + } + + func startCaching(assetIDs: [String], targetSize: CGSize, options: PHImageRequestOptions) { + let phAssets = assetIDs.compactMap { cachedAsset(for: $0) } + guard !phAssets.isEmpty else { return } + + let pixelSize = PhotoKitImageSizing.pixelSize(for: targetSize, scale: displayScale) + imageManager.startCachingImages( + for: phAssets, + targetSize: pixelSize, + contentMode: .aspectFill, + options: options + ) + } + + func stopCaching(assetIDs: [String], targetSize: CGSize, options: PHImageRequestOptions) { + let phAssets = assetIDs.compactMap { cachedAsset(for: $0) } + guard !phAssets.isEmpty else { return } + + let pixelSize = PhotoKitImageSizing.pixelSize(for: targetSize, scale: displayScale) + imageManager.stopCachingImages( + for: phAssets, + targetSize: pixelSize, + contentMode: .aspectFill, + options: options + ) + } + + func cancel(for key: String) { + cancelRequest(for: key) + } + + func cancel(matching keyPrefix: String) { + let keys = activeRequests.keys.filter { $0 == keyPrefix || $0.hasPrefix("\(keyPrefix)-") } + for key in keys { + cancelRequest(for: key) + } + } + + func cancelAll() { + let requestIDs = Array(activeRequests.values) + activeRequests.removeAll() + for requestID in requestIDs { + imageManager.cancelImageRequest(requestID) + } + } + + func requestImageData( + key: String, + asset: PHAsset, + deliveryMode: PHImageRequestOptionsDeliveryMode, + resizeMode: PHImageRequestOptionsResizeMode, + version: PHImageRequestOptionsVersion, + allowsNetwork: Bool, + progress: (@Sendable (Double) -> Void)? + ) async throws -> PhotoKitDataPayload { + try Task.checkCancellation() + + return try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { continuation in + let guardState = ContinuationGuard() + + Task { + await self.beginImageDataRequest( + key: key, + asset: asset, + deliveryMode: deliveryMode, + resizeMode: resizeMode, + version: version, + allowsNetwork: allowsNetwork, + progress: progress, + continuation: continuation, + guardState: guardState + ) + } + } + } onCancel: { + Task { await self.cancel(matching: key) } + } + } + + private func beginImageDataRequest( + key: String, + asset: PHAsset, + deliveryMode: PHImageRequestOptionsDeliveryMode, + resizeMode: PHImageRequestOptionsResizeMode, + version: PHImageRequestOptionsVersion, + allowsNetwork: Bool, + progress: (@Sendable (Double) -> Void)?, + continuation: CheckedContinuation, + guardState: ContinuationGuard + ) { + cancelRequest(for: key) + + let options = PHImageRequestOptions() + options.deliveryMode = deliveryMode + options.resizeMode = resizeMode + options.version = version + options.isNetworkAccessAllowed = allowsNetwork + options.isSynchronous = false + + if allowsNetwork, let progress { + options.progressHandler = { value, _, _, _ in + progress(value) + } + } + + let requestID = imageManager.requestImageDataAndOrientation( + for: asset, + options: options + ) { data, _, _, info in + Task { + await self.finishImageDataRequest( + key: key, + data: data, + info: info, + continuation: continuation, + guardState: guardState + ) + } + } + + storeRequest(key: key, id: requestID) + } + + private func finishImageDataRequest( + key: String, + data: Data?, + info: [AnyHashable: Any]?, + continuation: CheckedContinuation, + guardState: ContinuationGuard + ) { + storeRequest(key: key, id: nil) + + let cancelled = (info?[PHImageCancelledKey] as? Bool) == true + let imageError = info?[PHImageErrorKey] as? Error + let isInCloud = PHAssetCloudStatus.isInCloud(info: info) + + if cancelled { + guardState.resumeOnce(continuation, throwing: HIGPhotoImageLoadingError.cancelled) + return + } + + if let imageError { + guardState.resumeOnce(continuation, throwing: imageError) + return + } + + guard let data else { + guardState.resumeOnce(continuation, throwing: HIGPhotoImageLoadingError.imageUnavailable) + return + } + + guardState.resumeOnce( + continuation, + returning: PhotoKitDataPayload(data: data, isInCloud: isInCloud) + ) + } + + private func cancelRequest(for key: String) { + guard let requestID = activeRequests.removeValue(forKey: key) else { return } + imageManager.cancelImageRequest(requestID) + } + + private func storeRequest(key: String, id: PHImageRequestID?) { + if let id { + activeRequests[key] = id + } else { + activeRequests.removeValue(forKey: key) + } + } +} +#endif \ No newline at end of file diff --git a/Sources/HIGComponents/Inputs/PhotoPicker/PhotosLibraryClient.swift b/Sources/HIGComponents/Inputs/PhotoPicker/PhotosLibraryClient.swift index a9110be..20da685 100644 --- a/Sources/HIGComponents/Inputs/PhotoPicker/PhotosLibraryClient.swift +++ b/Sources/HIGComponents/Inputs/PhotoPicker/PhotosLibraryClient.swift @@ -29,8 +29,14 @@ public protocol PhotosLibraryClientProtocol: Sendable { func fetchAsset(localIdentifier: String) async -> HIGPhotoAsset? } +private actor PhotosLibraryExecutor { + func perform(_ work: @Sendable () -> T) -> T { + work() + } +} + public struct PhotosLibraryClient: PhotosLibraryClientProtocol { - private static let libraryQueue = DispatchQueue(label: "HIGPhotoPicker.Library", qos: .userInitiated) + private static let executor = PhotosLibraryExecutor() public init() {} @@ -39,7 +45,7 @@ public struct PhotosLibraryClient: PhotosLibraryClientProtocol { guard HIGPhotoPickerRuntime.shouldAccessPhotoKit else { return [] } #endif - return await performLibraryWork { + return await Self.executor.perform { var albums: [HIGPhotoAlbum] = [] for type in [PHAssetCollectionType.smartAlbum, .album] { @@ -64,7 +70,7 @@ public struct PhotosLibraryClient: PhotosLibraryClientProtocol { guard HIGPhotoPickerRuntime.shouldAccessPhotoKit else { return [] } #endif - return await performLibraryWork { + return await Self.executor.perform { let collections = PHAssetCollection.fetchAssetCollections( withLocalIdentifiers: [albumID], options: nil @@ -81,7 +87,7 @@ public struct PhotosLibraryClient: PhotosLibraryClientProtocol { guard HIGPhotoPickerRuntime.shouldAccessPhotoKit else { return nil } #endif - return await performLibraryWork { + return await Self.executor.perform { guard let phAsset = PHAsset.fetchAssets(withLocalIdentifiers: [localIdentifier], options: nil).firstObject else { return nil } @@ -113,12 +119,5 @@ public struct PhotosLibraryClient: PhotosLibraryClientProtocol { } return assets } - - private func performLibraryWork(_ work: @escaping @Sendable () -> T) async -> T { - await withCheckedContinuation { continuation in - Self.libraryQueue.async { - continuation.resume(returning: work()) - } - } - } -}#endif +} +#endif \ No newline at end of file diff --git a/Sources/HIGComponents/Inputs/PhotoPicker/Views/PhotoPreviewView.swift b/Sources/HIGComponents/Inputs/PhotoPicker/Views/PhotoPreviewView.swift index cdffc2d..18ee8cd 100644 --- a/Sources/HIGComponents/Inputs/PhotoPicker/Views/PhotoPreviewView.swift +++ b/Sources/HIGComponents/Inputs/PhotoPicker/Views/PhotoPreviewView.swift @@ -74,7 +74,7 @@ struct PhotoPreviewView: View { } if showsCropMask { - PreviewCompositionGridOverlay() + PreviewCompositionGridOverlayView() .frame(width: width, height: height) } @@ -269,7 +269,7 @@ struct PhotoPreviewView: View { } } -private struct PreviewCompositionGridOverlay: View { +private struct PreviewCompositionGridOverlayView: View { @Environment(\.higTheme) private var theme private var tokens: any HIGPhotoPickerTokens { theme.photoPicker } @@ -396,7 +396,7 @@ private struct ZoomableSwiftUIImageView: View { let tokens = HIGSystemPhotoPickerTokens() ZStack { tokens.previewBackground - PreviewCompositionGridOverlay() + PreviewCompositionGridOverlayView() } .frame(width: tokens.fallbackLayoutWidth * 0.75, height: tokens.fallbackLayoutWidth * 0.75) } diff --git a/Sources/HIGComponents/Inputs/PhotoPicker/Views/PickerChromeIconButton.swift b/Sources/HIGComponents/Inputs/PhotoPicker/Views/PickerChromeIconButton.swift index ce84e9a..278cfe6 100644 --- a/Sources/HIGComponents/Inputs/PhotoPicker/Views/PickerChromeIconButton.swift +++ b/Sources/HIGComponents/Inputs/PhotoPicker/Views/PickerChromeIconButton.swift @@ -90,6 +90,7 @@ struct PickerChromeLabelButtonView: View { } private enum PickerChromeButtonStyleSupport { + @MainActor static func applyIconStyle( to content: Content, sizing: PickerChromeButtonSizing @@ -99,7 +100,7 @@ private enum PickerChromeButtonStyleSupport { if #available(iOS 26.0, *) { return AnyView( content - .buttonStyle(.glass) + .buttonStyle(.glass as GlassButtonStyle) .controlSize(controlSize) .buttonBorderShape(.circle) .labelStyle(.iconOnly) @@ -115,6 +116,7 @@ private enum PickerChromeButtonStyleSupport { ) } + @MainActor static func applyLabelStyle( to content: Content, sizing: PickerChromeButtonSizing @@ -124,7 +126,7 @@ private enum PickerChromeButtonStyleSupport { if #available(iOS 26.0, *) { return AnyView( content - .buttonStyle(.glass) + .buttonStyle(.glass as GlassButtonStyle) .controlSize(controlSize) .buttonBorderShape(.capsule) .labelStyle(.titleAndIcon) @@ -144,6 +146,7 @@ private enum PickerChromeButtonStyleSupport { private struct PickerChromeIconButtonViewStyle: ViewModifier { let sizing: PickerChromeButtonSizing + @MainActor func body(content: Content) -> some View { PickerChromeButtonStyleSupport.applyIconStyle(to: content, sizing: sizing) } @@ -152,6 +155,7 @@ private struct PickerChromeIconButtonViewStyle: ViewModifier { private struct PickerChromeLabelButtonViewStyle: ViewModifier { let sizing: PickerChromeButtonSizing + @MainActor func body(content: Content) -> some View { PickerChromeButtonStyleSupport.applyLabelStyle(to: content, sizing: sizing) }