Skip to content

feat: show feedback widget on NFC scan issues #948

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 25, 2025
Merged

feat: show feedback widget on NFC scan issues #948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
caa54ad
feat: show feedback widget on NFC scan issues
transphorm Aug 25, 2025
7bc4d8d
fix ref
transphorm Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions app/src/hooks/useFeedbackModal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import {
import type { FeedbackModalScreenParams } from '@/components/FeedbackModalScreen';
import { captureFeedback } from '@/Sentry';

export type FeedbackType = 'button' | 'widget' | 'custom' | 'modal';
export type FeedbackType = 'button' | 'widget' | 'custom';

export const useFeedbackModal = () => {
const timeoutRef = useRef<NodeJS.Timeout | null>(null);
const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const [isVisible, setIsVisible] = useState(false);
const [isModalVisible, setIsModalVisible] = useState(false);
const [modalParams, setModalParams] =
Expand All @@ -35,8 +35,6 @@ export const useFeedbackModal = () => {
case 'custom':
setIsVisible(true);
break;
case 'modal':
break;
default:
showFeedbackButton();
}
Expand Down
46 changes: 39 additions & 7 deletions app/src/screens/passport/PassportNFCScanScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import TextsContainer from '@/components/TextsContainer';
import { BodyText } from '@/components/typography/BodyText';
import { Title } from '@/components/typography/Title';
import { PassportEvents } from '@/consts/analytics';
import { useFeedbackAutoHide } from '@/hooks/useFeedbackAutoHide';
import useHapticNavigation from '@/hooks/useHapticNavigation';
import NFC_IMAGE from '@/images/nfc.png';
import { ExpandableBottomLayout } from '@/layouts/ExpandableBottomLayout';
Expand Down Expand Up @@ -79,7 +80,8 @@ type PassportNFCScanRoute = RouteProp<
const PassportNFCScanScreen: React.FC = () => {
const navigation = useNavigation();
const route = useRoute<PassportNFCScanRoute>();
const { showModal } = useFeedback();
const { showModal, showFeedbackModal } = useFeedback();
useFeedbackAutoHide();
const {
passportNumber,
dateOfBirth,
Expand All @@ -93,6 +95,7 @@ const PassportNFCScanScreen: React.FC = () => {
const [isNfcSheetOpen, setIsNfcSheetOpen] = useState(false);
const [dialogMessage, setDialogMessage] = useState('');
const [nfcMessage, setNfcMessage] = useState<string | null>(null);
const scanTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);

const animationRef = useRef<LottieView>(null);

Expand Down Expand Up @@ -141,6 +144,10 @@ ${deviceInfo.map(([k, v]) => `${k}=${v}`).join('\n')}
);
}, []);

const onReportIssue = useCallback(() => {
showFeedbackModal('widget');
}, [showFeedbackModal]);

const openErrorModal = useCallback(
(message: string) => {
showModal({
Expand Down Expand Up @@ -198,6 +205,16 @@ ${deviceInfo.map(([k, v]) => `${k}=${v}`).join('\n')}
setIsNfcSheetOpen(true);
// Add timestamp when scan starts
const scanStartTime = Date.now();
if (scanTimeoutRef.current) {
clearTimeout(scanTimeoutRef.current);
scanTimeoutRef.current = null;
}
scanTimeoutRef.current = setTimeout(() => {
trackEvent(PassportEvents.NFC_SCAN_FAILED, { error: 'timeout' });
openErrorModal('Scan timed out. Please try again.');
showFeedbackModal('widget');
setIsNfcSheetOpen(false);
}, 30000);
Comment on lines +213 to +217
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid stacking Sentry widget on top of an error modal. Queue it for after modal dismissal.

Opening a modal and the Sentry widget simultaneously risks UI overlap, focus traps, or inaccessible flows. Trigger the widget after the error modal closes (or make it the primary action), not immediately on error/timeout.

       scanTimeoutRef.current = setTimeout(() => {
         trackEvent(PassportEvents.NFC_SCAN_FAILED, { error: 'timeout' });
         openErrorModal('Scan timed out. Please try again.');
-        showFeedbackModal('widget');
         setIsNfcSheetOpen(false);
       }, 30000);
-        openErrorModal(message);
-        showFeedbackModal('widget');
+        openErrorModal(message);

And change the modal to trigger the widget on dismiss:

   const openErrorModal = useCallback(
     (message: string) => {
       showModal({
         titleText: 'NFC Scan Error',
         bodyText: message,
         buttonText: 'Send Feedback',
         secondaryButtonText: 'Help',
         preventDismiss: false,
-        onButtonPress: () => sendFeedbackEmail(message),
+        onButtonPress: () => sendFeedbackEmail(message),
         onSecondaryButtonPress: goToNFCTrouble,
-        onModalDismiss: () => {},
+        onModalDismiss: () => showFeedbackModal('widget'),
       });
     },
-    [showModal, goToNFCTrouble, sendFeedbackEmail],
+    [showModal, goToNFCTrouble, sendFeedbackEmail, showFeedbackModal],
   );

Also applies to: 327-327

Copy link
Member Author

@transphorm transphorm Aug 25, 2025

Choose a reason for hiding this comment

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

why are we showing the feedback modal again onModalDismiss? this seems like an anti pattern to me

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

@transphorm You're absolutely right - automatically showing a feedback modal on dismiss is indeed an anti-pattern. Users dismiss modals to return to the main flow, not to be confronted with another modal. This violates user agency and creates an intrusive experience.

Looking at the code, you already have a "Report issue" button in the main UI that triggers showFeedbackModal('widget'), which is the proper user-initiated approach.

For the error cases (timeout and scan failures), I'd suggest one of these cleaner alternatives:

Option 1: Remove automatic feedback entirely

scanTimeoutRef.current = setTimeout(() => {
  trackEvent(PassportEvents.NFC_SCAN_FAILED, { error: 'timeout' });
  openErrorModal('Scan timed out. Please try again.');
  setIsNfcSheetOpen(false);
}, 30000);

Option 2: Replace the "Send Feedback" button with widget feedback

const openErrorModal = useCallback(
  (message: string) => {
    showModal({
      titleText: 'NFC Scan Error',
      bodyText: message,
      buttonText: 'Report Issue',
      secondaryButtonText: 'Help',
      preventDismiss: false,
      onButtonPress: () => showFeedbackModal('widget'),
      onSecondaryButtonPress: goToNFCTrouble,
      onModalDismiss: () => {},
    });
  },
  [showModal, goToNFCTrouble, showFeedbackModal],
);

The existing "Report issue" button provides a clear, user-controlled path for feedback without being pushy about it.

Copy link
Member Author

@transphorm transphorm Aug 25, 2025

Choose a reason for hiding this comment

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

ok nice at least we are on the same page now. can you provide a full prompt for me?

Copy link
Contributor

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

@transphorm Here's the complete code solution to remove the anti-pattern and provide a clean feedback flow:

1. Update the timeout handler:

scanTimeoutRef.current = setTimeout(() => {
  trackEvent(PassportEvents.NFC_SCAN_FAILED, { error: 'timeout' });
  openErrorModal('Scan timed out. Please try again.');
  setIsNfcSheetOpen(false);
}, 30000);

2. Update the error catch block:

} catch (e: unknown) {
  const scanDurationSeconds = (
    (Date.now() - scanStartTime) /
    1000
  ).toFixed(2);
  console.error('NFC Scan Unsuccessful:', e);
  const message = e instanceof Error ? e.message : String(e);
  trackEvent(PassportEvents.NFC_SCAN_FAILED, {
    error: message,
    duration_seconds: parseFloat(scanDurationSeconds),
  });
  openErrorModal(message);
} finally {
  if (scanTimeoutRef.current) {
    clearTimeout(scanTimeoutRef.current);
    scanTimeoutRef.current = null;
  }
  setIsNfcSheetOpen(false);
}

3. Update the openErrorModal function:

const openErrorModal = useCallback(
  (message: string) => {
    showModal({
      titleText: 'NFC Scan Error',
      bodyText: message,
      buttonText: 'Report Issue',
      secondaryButtonText: 'Help',
      preventDismiss: false,
      onButtonPress: () => showFeedbackModal('widget'),
      onSecondaryButtonPress: goToNFCTrouble,
      onModalDismiss: () => {},
    });
  },
  [showModal, goToNFCTrouble, showFeedbackModal],
);

4. Remove the sendFeedbackEmail callback dependency from onVerifyPress:

const onVerifyPress = useCallback(async () => {
  // ... existing code
}, [
  isNfcEnabled,
  isNfcSupported,
  route.params,
  passportNumber,
  dateOfBirth,
  dateOfExpiry,
  isPacePolling,
  navigation,
  openErrorModal,
  showFeedbackModal,
]);

This approach gives users clear control over when they want to provide feedback while avoiding the intrusive automatic modal stacking.


try {
const { canNumber, useCan, skipPACE, skipCA, extendedMode } =
Expand Down Expand Up @@ -307,7 +324,12 @@ ${deviceInfo.map(([k, v]) => `${k}=${v}`).join('\n')}
duration_seconds: parseFloat(scanDurationSeconds),
});
openErrorModal(message);
showFeedbackModal('widget');
} finally {
if (scanTimeoutRef.current) {
clearTimeout(scanTimeoutRef.current);
scanTimeoutRef.current = null;
}
setIsNfcSheetOpen(false);
}
} else if (isNfcSupported) {
Expand All @@ -327,6 +349,7 @@ ${deviceInfo.map(([k, v]) => `${k}=${v}`).join('\n')}
isPacePolling,
navigation,
openErrorModal,
showFeedbackModal,
]);

const navigateToLaunch = useHapticNavigation('Launch', {
Expand Down Expand Up @@ -448,12 +471,21 @@ ${deviceInfo.map(([k, v]) => `${k}=${v}`).join('\n')}
gap="$1.5"
>
<Title>Verify your ID</Title>
<Button
unstyled
onPress={goToNFCTrouble}
icon={<CircleHelp size={28} color={slate500} />}
aria-label="Help"
/>
<XStack alignItems="center" gap="$2">
<Button
unstyled
onPress={goToNFCTrouble}
icon={<CircleHelp size={28} color={slate500} />}
aria-label="Help"
/>
<Button
unstyled
onPress={onReportIssue}
aria-label="Report issue"
>
<BodyText color={slate500}>Report issue</BodyText>
</Button>
</XStack>
</XStack>
</GestureDetector>
{isNfcEnabled ? (
Expand Down
9 changes: 8 additions & 1 deletion app/src/screens/passport/PassportNFCTroubleScreen.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@

import React, { useEffect } from 'react';
import { Gesture, GestureDetector } from 'react-native-gesture-handler';
import { YStack } from 'tamagui';
import { Button, YStack } from 'tamagui';

import type { TipProps } from '@/components/Tips';
import Tips from '@/components/Tips';
import { Caption } from '@/components/typography/Caption';
import { useFeedbackAutoHide } from '@/hooks/useFeedbackAutoHide';
import useHapticNavigation from '@/hooks/useHapticNavigation';
import SimpleScrolledTitleLayout from '@/layouts/SimpleScrolledTitleLayout';
import { useFeedback } from '@/providers/feedbackProvider';
import analytics from '@/utils/analytics';
import { slate500 } from '@/utils/colors';

Expand Down Expand Up @@ -46,6 +48,8 @@ const PassportNFCTrouble: React.FC = () => {
const goToNFCMethodSelection = useHapticNavigation(
'PassportNFCMethodSelection',
);
const { showFeedbackModal } = useFeedback();
useFeedbackAutoHide();

// error screen, flush analytics
useEffect(() => {
Expand Down Expand Up @@ -84,6 +88,9 @@ const PassportNFCTrouble: React.FC = () => {
device supports NFC and that your passport's RFID is functioning
properly.
</Caption>
<Button onPress={() => showFeedbackModal('widget')}>
Report issue
</Button>
</YStack>
</SimpleScrolledTitleLayout>
);
Expand Down
Loading