Skip to content

Conversation

philprime
Copy link
Member

@philprime philprime commented Sep 12, 2025

Summary

  • Extracted LinkDetailNFCSharingViewModel, NFCDeviceDelegate, NFCSharingState; container view is view-only with .sentryTrace("LINK_DETAIL_NFC_SHARING").
  • Fixed CoreNFC write flow: retain delegate; handle multi-tag via restartPolling(); check NDEF status/capacity; set alertMessage and invalidate() on success/error.
  • Preserved error handling pattern and Sentry breadcrumbs/events.
  • Entitlements (com.apple.developer.nfc.readersession.formats) and NFCReaderUsageDescription verified.

Why

  • Previous implementation could drop callbacks due to deallocated delegate and didn't follow Apple guidance, causing writes to fail.

References

Testing

  • Requires physical device with NFC (not simulator).
  • Present only one tag at a time; ensure tag supports NDEF and has sufficient capacity.
  • iOS 13+ required for CoreNFC NDEF writing.

@Copilot Copilot AI review requested due to automatic review settings September 12, 2025 13:54
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the NFC sharing functionality to follow MVVM architecture pattern and fixes the CoreNFC write flow by implementing proper tag handling and error management.

Key changes:

  • Extracts business logic from the container view into a dedicated ViewModel pattern
  • Implements proper CoreNFC delegate with tag capacity checks and multi-tag handling
  • Adds comprehensive error handling with specific NFC error cases and Sentry integration

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
NFCSharingState.swift Defines state enum for NFC sharing operations (ready, scanning, success, error)
NFCDeviceDelegate.swift New CoreNFC delegate handling tag detection, connection, and NDEF writing with proper error cases
LinkDetailNFCSharingViewModel.swift ViewModel managing NFC session lifecycle, delegate coordination, and analytics tracking
LinkDetailNFCSharingContainerView.swift Refactored container view using MVVM pattern, removing inline NFC logic
App.entitlements Adds NDEF format to NFC reader session entitlements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@philprime philprime changed the title NFC: Refactor LinkDetailNFCSharing into MVVM and fix CoreNFC write flow \n\n- Extract ViewModel and delegate; container is view-only\n- Retain delegate; handle multi-tag, capacity checks, proper alerts\n- Align with Apple CoreNFC guidance; add Sentry breadcrumbs/events\n- Keep entitlements NDEF/TAG formats enabled fix: Change NFC tag writing Sep 12, 2025
- Extract ViewModel and delegate; container is view-only
- Retain delegate; handle multi-tag, capacity checks, proper alerts
- Align with Apple CoreNFC guidance; add Sentry breadcrumbs/events
- Keep entitlements NDEF/TAG formats enabled
@philprime philprime enabled auto-merge (squash) September 12, 2025 13:59
@philprime philprime disabled auto-merge September 12, 2025 14:03
@philprime philprime merged commit c2385ec into main Sep 12, 2025
2 of 3 checks passed
@philprime philprime deleted the nfc-writing branch September 12, 2025 14:04
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.

1 participant