From cda082d555e68783f8dc1bb9bc86ca20bedf756e Mon Sep 17 00:00:00 2001 From: Ryan Stelly Date: Fri, 16 Jan 2026 15:33:29 -0600 Subject: [PATCH] additional user context for feedback (#3324) * additional context when submitting user feedback * remove log * add local obs count * add a generic breakpoint mapper as a shared function and migrate media query breakpoints to use it * remove original getBreakpoint impl after test validated * switch feedback logging to structured data --- src/components/Menu/Menu.tsx | 71 ++++++++++++++--- src/components/Settings/AdvancedSettings.tsx | 8 +- src/realmModels/User.ts | 2 + src/realmModels/index.ts | 2 +- src/sharedHelpers/breakpoint.ts | 81 ++++++++++++++++---- tests/unit/helpers/breakpoint.test.js | 55 +++++++++++++ 6 files changed, 185 insertions(+), 34 deletions(-) create mode 100644 tests/unit/helpers/breakpoint.test.js diff --git a/src/components/Menu/Menu.tsx b/src/components/Menu/Menu.tsx index e6fe52e7d..5eafc1c4c 100644 --- a/src/components/Menu/Menu.tsx +++ b/src/components/Menu/Menu.tsx @@ -16,20 +16,18 @@ import { RealmContext } from "providers/contexts"; import React, { useCallback, useState } from "react"; import { Alert } from "react-native"; import { useSafeAreaInsets } from "react-native-safe-area-context"; +import Observation from "realmModels/Observation"; import User from "realmModels/User"; +import { valueToBreakpoint } from "sharedHelpers/breakpoint"; import { log } from "sharedHelpers/logger"; -import { useCurrentUser, useTranslation } from "sharedHooks"; -import useStore, { zustandStorage } from "stores/useStore"; +import { useCurrentUser, useLayoutPrefs, useTranslation } from "sharedHooks"; +import { zustandStorage } from "stores/useStore"; import colors from "styles/tailwindColors"; import MenuItem from "./MenuItem"; const { useRealm } = RealmContext; -function isDefaultMode( ) { - return useStore.getState( ).layout.isDefaultMode === true; -} - interface BaseMenuOption { label: string; icon: string; @@ -72,6 +70,7 @@ const Menu = ( ) => { const { isConnected } = useNetInfo( ); + const layoutPrefs = useLayoutPrefs(); const [modalState, setModalState] = useState( null ); const menuItems: Record = { @@ -155,19 +154,67 @@ const Menu = ( ) => { navigation.goBack( ); }; - const onSubmitFeedback = useCallback( ( text: string ) => { + const onSubmitFeedback = useCallback( ( feedbackText: string ) => { if ( !isConnected ) { showOfflineAlert( t ); return false; } - const mode = isDefaultMode( ) - ? "DEFAULT:" - : "ADVANCED:"; - feedbackLogger.info( mode, text ); + const locallySavedOnlyObservations = Observation.filterUnsyncedObservations( realm ).length; + const getCountBreakpoint = ( count: number ) => valueToBreakpoint( count, [ + [0, "0"], + [1, "1-9"], + [10, "10-99"], + [100, "100-999"], + [1000, "1000+"], + ] ); + const { + isDefaultMode, + isAllAddObsOptionsMode, + screenAfterPhotoEvidence, + } = layoutPrefs; + const modeContext = ( isDefaultMode + ? { + mode: "default", + observationButtonMode: "default", + screenAfterPhotoEvidence: "default", + } + : { + mode: "advanced", + observationButtonMode: isAllAddObsOptionsMode + ? "Obs Sheet" + : "AI Camera", + screenAfterPhotoEvidence, + } ); + const loggedInContext = currentUser + ? { + loggedIn: "Yes", + username: currentUser.login, + identifications: typeof currentUser.identifications_count === "number" + ? getCountBreakpoint( currentUser.identifications_count ) + : "NA", + remoteObservations: typeof currentUser.observations_count === "number" + ? getCountBreakpoint( currentUser.observations_count ) + : "NA", + } + : { + loggedIn: "No", + username: "loggedout", + identifications: "loggedout", + remoteObservations: "loggedout", + }; + const feedbackWithContext = { + text: feedbackText, + ...modeContext, + ...loggedInContext, + // can have unsynced obs when logged out + locallySavedOnlyObservations, + }; + // we're logging structured data here that is parsed in Grafana + feedbackLogger.info( feedbackWithContext ); Alert.alert( t( "Feedback-Submitted" ), t( "Thank-you-for-sharing-your-feedback" ) ); setModalState( null ); return true; - }, [isConnected, t] ); + }, [currentUser, isConnected, layoutPrefs, realm, t] ); return ( { setScreenAfterPhotoEvidence, } = useLayoutPrefs(); - const renderSettingDescription = description => ( - {description} - ); - return ( <> - {renderSettingDescription( t( "When-tapping-the-green-observation-button" ) )} + { t( "When-tapping-the-green-observation-button" ) } { /> - {renderSettingDescription( t( "After-capturing-or-importing-photos-show" ) )} + { t( "After-capturing-or-importing-photos-show" ) } { + const breakpointCount = breakpointToLabelTuples.length; + if ( breakpointToLabelTuples.length < 2 ) { + throw Error( "must provide at least 2 breakpoints" ); + } + + // tuples are more ergonomic for the caller, but paired collections + // are cleaner for the implementation + const breakpoints = []; + const labels = []; + for ( const [breakpoint, label] of breakpointToLabelTuples ) { + breakpoints.push( breakpoint ); + labels.push( label ); + } + + let lastLowestBreakpoint = null; + for ( const breakpoint of breakpoints ) { + if ( lastLowestBreakpoint !== null && breakpoint <= lastLowestBreakpoint ) { + throw Error( "breakpoints must be unique and ascending" ); + } + lastLowestBreakpoint = breakpoint; + } + + if ( value < breakpoints[0] ) { + throw Error( "value cannot be lower than lowest breakpoint" ); + } + + for ( const index of breakpoints.keys() ) { + const atFinalBreakpoint = index === breakpointCount - 1; + const valueIsBelowNextThresold = value < breakpoints[index + 1]; + if ( atFinalBreakpoint || valueIsBelowNextThresold ) { + return labels[index]; + } + } + throw Error( "breakpoint unresolvable" ); +}; + export const BREAKPOINTS = reduce( screens, ( memo, widthString, breakpoint ) => { if ( typeof widthString !== "string" ) { throw new Error( `Unexpected breakpoint value: ${widthString}` ); @@ -9,20 +65,15 @@ export const BREAKPOINTS = reduce( screens, ( memo, widthString, breakpoint ) => return memo; }, { } as Record ); -const getBreakpoint = ( screenWidth: number ) => { - if ( screenWidth >= BREAKPOINTS["2xl"] ) { - return "2xl"; - } - if ( screenWidth >= BREAKPOINTS.xl ) { - return "xl"; - } - if ( screenWidth >= BREAKPOINTS.lg ) { - return "lg"; - } - if ( screenWidth >= BREAKPOINTS.md ) { - return "md"; - } - return "sm"; -}; +const getBreakpoint = ( screenWidth: number ) => valueToBreakpoint( screenWidth, [ + // duplicate "sm" here to maintain "sm" as the minimum, but leave room for maybe "xs" + // to be added as a new breakpoint which would have its own breakpoint and replace "sm" as the 0 + [0, "sm"], + [BREAKPOINTS.sm, "sm"], + [BREAKPOINTS.md, "md"], + [BREAKPOINTS.lg, "lg"], + [BREAKPOINTS.xl, "xl"], + [BREAKPOINTS["2xl"], "2xl"], +] ); export default getBreakpoint; diff --git a/tests/unit/helpers/breakpoint.test.js b/tests/unit/helpers/breakpoint.test.js new file mode 100644 index 000000000..835f52e25 --- /dev/null +++ b/tests/unit/helpers/breakpoint.test.js @@ -0,0 +1,55 @@ +import getBreakpoint, { valueToBreakpoint } from "sharedHelpers/breakpoint"; + +describe( "breakpoint helpers", () => { + describe( "valueToBreakpoint", () => { + test.each( [ + [0, "0"], + [1, "1-9"], + [9, "1-9"], + [10, "10-49"], + [49, "10-49"], + [50, "50-99"], + [99, "50-99"], + [100, "100+"], + [101, "100+"], + [1000, "100+"], + // one-off randomish case in addition to boundary cases above + [62, "50-99"], + ] )( "should return appropriate segment label for input", ( input, expected ) => { + const result = valueToBreakpoint( input, [ + [0, "0"], + [1, "1-9"], + [10, "10-49"], + [50, "50-99"], + [100, "100+"], + ] ); + + expect( result ).toBe( expected ); + } ); + } ); + + describe( "getBreakpoint", () => { + test.each( [ + // main focus on sm => md because of sm's role + // as a non-zero breakpoint _but also_ as the default breakpoint / floor breakpoint + [0, "sm"], + [239, "sm"], + [240, "sm"], + [241, "sm"], + [319, "sm"], + [320, "md"], + [321, "md"], + [321, "md"], + [1365, "xl"], + [1366, "2xl"], + [1367, "2xl"], + ] )( + "should return appropriate media query label for screenWidth", + ( screenWidth, expected ) => { + const result = getBreakpoint( screenWidth ); + + expect( result ).toBe( expected ); + }, + ); + } ); +} );