Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce haptic feedback to selfie capture flow #256

Closed

Conversation

tobitech
Copy link
Contributor

Story: https://app.shortcut.com/smileid/story/13080/add-active-liveness-smartselfie-v2-ios

Summary

  • Add a haptic manager and play haptics at different points in the selfie capture process.

Known Issues

N/A.

Test Instructions

N/A.

Screenshot

N/A.

@tobitech tobitech self-assigned this Nov 15, 2024
@tobitech tobitech requested a review from a team as a code owner November 15, 2024 16:00
@prfectionist
Copy link

prfectionist bot commented Nov 15, 2024

Title

Introduce haptic feedback to selfie capture flow


User description

Story: https://app.shortcut.com/smileid/story/13080/add-active-liveness-smartselfie-v2-ios

Summary

  • Add a haptic manager and play haptics at different points in the selfie capture process.

Known Issues

N/A.

Test Instructions

N/A.

Screenshot

N/A.


PR Type

Enhancement


Description

  • Implemented new HapticManager singleton class to centralize haptic feedback control
  • Added tactile feedback at key moments in the selfie capture process:
    • Face detection success
    • Liveness task completion
    • Liveness challenge completion
    • API submission success/failure
  • Improved user experience by providing immediate tactile confirmation of important events

Changes walkthrough 📝

Relevant files
Enhancement
HapticManager.swift
Implement HapticManager for device feedback control           

Sources/SmileID/Classes/Helpers/HapticManager.swift

  • Created new HapticManager singleton class
  • Implemented notification feedback method for success/warning/error
  • Implemented impact feedback method with customizable style
  • +23/-0   
    SelfieViewModelV2.swift
    Add haptic feedback integration to selfie capture flow     

    Sources/SmileID/Classes/SelfieCapture/SelfieViewModelV2.swift

  • Added haptic feedback for successful face detection
  • Integrated haptic feedback for liveness check completion
  • Added haptic notifications for submission success/failure
  • +5/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @prfectionist
    Copy link

    prfectionist bot commented Nov 15, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Memory Management
    The UINotificationFeedbackGenerator and UIImpactFeedbackGenerator instances are created and immediately discarded. Consider reusing these generators by storing them as properties for better performance.

    Duplicate Code
    Multiple identical haptic success notifications are triggered in different places. Consider extracting this into a dedicated method to improve code maintainability.

    Copy link

    github-actions bot commented Nov 15, 2024

    Warnings
    ⚠️ The source files were changed, but the tests remain unmodified. Consider updating or adding to the tests to match the source changes.

    Generated by 🚫 Danger Swift against d7c6ddf

    @prfectionist
    Copy link

    prfectionist bot commented Nov 15, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Performance
    Cache and reuse feedback generators instead of creating new instances for each haptic feedback

    Store the generators as properties to reuse them instead of creating new instances
    for each feedback, which improves performance and reduces memory allocation.

    Sources/SmileID/Classes/Helpers/HapticManager.swift [3-6]

     class HapticManager {
         static let shared = HapticManager()
    +    private let notificationGenerator = UINotificationFeedbackGenerator()
    +    private let impactGenerator = UIImpactFeedbackGenerator(style: .medium)
         private init() {}
    Suggestion importance[1-10]: 9

    Why: Creating new generator instances for each feedback is inefficient. Caching generators as properties will improve performance and reduce memory allocations, especially given the frequent haptic feedback usage in the code.

    9
    Prepare haptic feedback generators before triggering to ensure consistent timing and responsiveness

    Prepare the haptic generator before triggering feedback to ensure consistent haptic
    response timing. Call prepare() before triggering the feedback.

    Sources/SmileID/Classes/Helpers/HapticManager.swift [12-15]

     func notification(type: UINotificationFeedbackGenerator.FeedbackType) {
         let generator = UINotificationFeedbackGenerator()
    +    generator.prepare()
         generator.notificationOccurred(type)
     }
    Suggestion importance[1-10]: 8

    Why: Calling prepare() on haptic generators is a recommended practice that significantly improves the timing consistency of haptic feedback, leading to a better user experience.

    8
    Enhancement
    Use appropriate haptic feedback types that match the user interaction context

    Consider using impact feedback instead of notification feedback for the face
    detection success to provide a more subtle and appropriate haptic response.

    Sources/SmileID/Classes/SelfieCapture/SelfieViewModelV2.swift [155-157]

     if hasDetectedValidFace && selfieImage == nil {
    -    HapticManager.shared.notification(type: .success)
    +    HapticManager.shared.impact(style: .light)
         captureSelfieImage(imageBuffer)
    Suggestion importance[1-10]: 6

    Why: Using impact feedback instead of notification feedback for face detection provides a more subtle and contextually appropriate haptic response, improving the user experience.

    6

    @tobitech
    Copy link
    Contributor Author

    tobitech commented Dec 4, 2024

    Closing this because the changes in this PR have been integrated into the liveness-timeout branch.

    @tobitech tobitech closed this Dec 4, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants