diff --git a/redux/PlayersSlice.test.ts b/redux/PlayersSlice.test.ts index 16ffc857..5b796be6 100644 --- a/redux/PlayersSlice.test.ts +++ b/redux/PlayersSlice.test.ts @@ -4,6 +4,7 @@ import playersReducer, { playerAdd, playerRoundScoreIncrement, playerRoundScoreSet, + selectAllPlayerNames, selectAllPlayers, selectPlayerGrandTotalScore, selectPlayerRoundStats, @@ -230,6 +231,32 @@ describe('player sort order', () => { }); }); +describe('selectAllPlayerNames', () => { + it('should return unique player names with a stable memoized reference', () => { + const players = playersReducer(undefined, playerAdd({ + id: '1', + playerName: 'Alice', + scores: [], + })); + const withDuplicateName = playersReducer(players, playerAdd({ + id: '2', + playerName: 'Alice', + scores: [], + })); + const withOtherName = playersReducer(withDuplicateName, playerAdd({ + id: '3', + playerName: 'Bob', + scores: [], + })); + const state = { players: withOtherName } as RootState; + + const result = selectAllPlayerNames(state); + + expect(result).toEqual(['Alice', 'Bob']); + expect(selectAllPlayerNames(state)).toBe(result); + }); +}); + describe('selectPlayerScoreByRound', () => { it('should return the score for the requested round', () => { expect(selectPlayerScoreByRound(stateWithScores([10, 20, 30]), '1', 1)).toEqual(20); diff --git a/redux/PlayersSlice.ts b/redux/PlayersSlice.ts index ea1c21d9..27db67e2 100644 --- a/redux/PlayersSlice.ts +++ b/redux/PlayersSlice.ts @@ -105,6 +105,17 @@ export const selectPlayerNameById = createSelector( (player) => player?.playerName ); +export const selectAllPlayerNames = createSelector( + [(state: RootState) => state.players.entities], + (entities) => { + const names: string[] = []; + for (const player of Object.values(entities)) { + if (player) names.push(player.playerName); + } + return [...new Set(names)]; + } +); + export const selectPlayerScoreByRound = createSelector( [ (state: RootState) => state.players.entities, diff --git a/src/components/EditGame.test.tsx b/src/components/EditGame.test.tsx index 71254bf7..06405cf9 100644 --- a/src/components/EditGame.test.tsx +++ b/src/components/EditGame.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { configureStore } from '@reduxjs/toolkit'; import { fireEvent, render } from '@testing-library/react-native'; +import { Platform } from 'react-native'; import { Provider } from 'react-redux'; import gamesReducer from '../../redux/GamesSlice'; @@ -11,28 +12,45 @@ import settingsReducer from '../../redux/SettingsSlice'; import EditGame from './EditGame'; // Mock react-native-elements -jest.mock('react-native-elements', () => ({ - Input: ({ defaultValue, onChangeText, onEndEditing, onBlur, placeholder, testID }: { - defaultValue: string; - onChangeText: (text: string) => void; - onEndEditing: (e: { nativeEvent: { text: string } }) => void; - onBlur: (e: { nativeEvent: { text: string } }) => void; - placeholder: string; - testID?: string; - }) => { - const { TextInput } = jest.requireActual('react-native'); - return ( - onEndEditing({ nativeEvent: { text: e.nativeEvent.text } })} - onBlur={(e: { nativeEvent: { text: string } }) => onBlur({ nativeEvent: { text: e.nativeEvent.text } })} - placeholder={placeholder} - testID={testID || 'game-title-input'} - /> - ); - }, -})); +jest.mock('react-native-elements', () => { + const React = jest.requireActual('react'); + const { Pressable, TextInput } = jest.requireActual('react-native'); + + return { + Input: React.forwardRef(({ value, onChangeText, onEndEditing, onSubmitEditing, onBlur, placeholder, testID, rightIcon, ...props }: { + value: string; + onChangeText: (text: string) => void; + onEndEditing: (e: { nativeEvent: { text: string } }) => void; + onSubmitEditing: (e: { nativeEvent: { text: string } }) => void; + onBlur: () => void; + placeholder: string; + rightIcon?: { onPress: () => void; name: string }; + testID?: string; + }, ref: React.Ref<{ focus: () => void }>) => { + React.useImperativeHandle(ref, () => ({ + focus: jest.fn(), + })); + + return ( + <> + onEndEditing({ nativeEvent: { text: e.nativeEvent.text } })} + onSubmitEditing={(e: { nativeEvent: { text: string } }) => onSubmitEditing({ nativeEvent: { text: e.nativeEvent.text } })} + placeholder={placeholder} + testID={testID || 'game-title-input'} + value={value} + {...props} + /> + {rightIcon != null && ( + + )} + + ); + }), + }; +}); // Mock @react-navigation/native jest.mock('@react-navigation/native', () => ({ @@ -89,6 +107,10 @@ describe('EditGame', () => { beforeEach(() => { jest.clearAllMocks(); + Object.defineProperty(Platform, 'OS', { + configurable: true, + value: 'ios', + }); }); it('should render null when no current game is set', () => { @@ -427,7 +449,71 @@ describe('EditGame', () => { ); const input = getByTestId('game-title-input'); - expect(input.props.defaultValue).toBe(''); + expect(input.props.value).toBe(''); + }); + + it('should use native replacement affordances', () => { + const store = createMockStore({ + settings: { + currentGameId: 'game-1', + }, + games: { + entities: { + 'game-1': mockGame, + }, + ids: ['game-1'], + }, + players: { + entities: mockPlayers, + ids: ['player-1', 'player-2'], + }, + }); + + const { getByTestId } = render( + + + + ); + + const input = getByTestId('game-title-input'); + + expect(input.props.clearButtonMode).toBe('while-editing'); + expect(input.props.returnKeyType).toBe('done'); + expect(input.props.selectTextOnFocus).toBe(true); + }); + + it('should clear the game title locally with the Android clear button', () => { + Object.defineProperty(Platform, 'OS', { + configurable: true, + value: 'android', + }); + + const store = createMockStore({ + settings: { + currentGameId: 'game-1', + }, + games: { + entities: { + 'game-1': mockGame, + }, + ids: ['game-1'], + }, + players: { + entities: mockPlayers, + ids: ['player-1', 'player-2'], + }, + }); + + const { getByDisplayValue, getByTestId } = render( + + + + ); + + fireEvent.press(getByTestId('game-title-clear-button')); + + expect(getByDisplayValue('')).toBeTruthy(); + expect(store.getState().games.entities['game-1']?.title).toBe('Test Game'); }); it('should handle very long titles within character limit', () => { diff --git a/src/components/EditGame.tsx b/src/components/EditGame.tsx index 1e89fa0d..28ab65ed 100644 --- a/src/components/EditGame.tsx +++ b/src/components/EditGame.tsx @@ -1,7 +1,16 @@ import React, { useEffect, useState } from 'react'; import { useNavigation } from '@react-navigation/native'; -import { NativeSyntheticEvent, StyleSheet, Text, TextInputEndEditingEventData, View } from 'react-native'; +import { + NativeSyntheticEvent, + Platform, + StyleSheet, + Text, + TextInput, + TextInputEndEditingEventData, + TextInputSubmitEditingEventData, + View +} from 'react-native'; import { Input } from 'react-native-elements'; import { updateGame } from '../../redux/GamesSlice'; @@ -18,6 +27,7 @@ const EditGame = ({ }) => { const dispatch = useAppDispatch(); const currentGame = useAppSelector(selectCurrentGame); const [localTitle, setLocalTitle] = useState(currentGame?.title ?? ''); + const inputRef = React.useRef(null); const navigation = useNavigation(); @@ -48,18 +58,30 @@ const EditGame = ({ }) => { commitTitle(text); }; + const onSubmitEditingHandler = (e: NativeSyntheticEvent) => { + const text = e.nativeEvent.text; + commitTitle(text); + }; + const onChangeTextHandler = (text: string) => { setLocalTitle(text); }; + const clearTitle = () => { + setLocalTitle(''); + inputRef.current?.focus(); + }; + return ( <> { if (localTitle == '') { commitTitle(UNTITLED); @@ -67,8 +89,20 @@ const EditGame = ({ }) => { }} placeholder={UNTITLED} renderErrorMessage={false} + returnKeyType="done" + rightIcon={Platform.OS === 'ios' ? undefined : { + style: { padding: 8 }, + disabled: localTitle == '', + disabledStyle: { display: 'none' }, + color: theme.textTertiary, + size: 15, + name: 'close', + onPress: clearTitle, + }} + selectTextOnFocus={true} style={{ color: theme.inputText }} inputContainerStyle={{ borderBottomWidth: 0 }} + value={localTitle} /> diff --git a/src/components/FloatingActionButton.test.tsx b/src/components/FloatingActionButton.test.tsx new file mode 100644 index 00000000..dfe89d47 --- /dev/null +++ b/src/components/FloatingActionButton.test.tsx @@ -0,0 +1,114 @@ +import React from 'react'; + +import type { ParamListBase } from '@react-navigation/native'; +import type { NativeStackNavigationProp } from '@react-navigation/native-stack'; +import { configureStore } from '@reduxjs/toolkit'; +import { act, render } from '@testing-library/react-native'; +import { Keyboard } from 'react-native'; +import { Provider } from 'react-redux'; + +import gamesReducer from '../../redux/GamesSlice'; +import playersReducer from '../../redux/PlayersSlice'; +import settingsReducer, { initialState as initialSettingsState } from '../../redux/SettingsSlice'; + +import FloatingActionButton from './FloatingActionButton'; + +jest.mock('expo-crypto', () => ({ + randomUUID: jest.fn(() => 'mock-uuid'), +})); + +jest.mock('../Analytics', () => ({ + logEvent: jest.fn(), +})); + +jest.mock('../ColorPalette', () => ({ + getPalette: jest.fn(() => ['#ffffff']), +})); + +jest.mock('react-native-elements', () => ({ + Icon: () => null, +})); + +jest.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ top: 0, bottom: 0, left: 0, right: 0 }), +})); + +let capturedOnOpenMenu: (() => void) | undefined; +let capturedOnPressAction: ((event: { nativeEvent: { event: string } }) => void) | undefined; + +jest.mock('@react-native-menu/menu', () => { + const { View } = jest.requireActual('react-native'); + + return { + MenuView: ({ children, onOpenMenu, onPressAction }: { + children: React.ReactNode; + onOpenMenu?: () => void; + onPressAction?: (event: { nativeEvent: { event: string } }) => void; + }) => { + capturedOnOpenMenu = onOpenMenu; + capturedOnPressAction = onPressAction; + return {children}; + }, + }; +}); + +const createMockStore = () => configureStore({ + reducer: { + settings: settingsReducer, + games: gamesReducer, + players: playersReducer, + }, + preloadedState: { + settings: { + ...initialSettingsState, + currentGameId: undefined, + }, + games: { + entities: {}, + ids: [], + }, + players: { + entities: {}, + ids: [], + }, + }, +}); + +const mockNavigation = { + navigate: jest.fn(), +} as unknown as NativeStackNavigationProp; + +describe('FloatingActionButton', () => { + beforeEach(() => { + jest.clearAllMocks(); + capturedOnOpenMenu = undefined; + capturedOnPressAction = undefined; + jest.spyOn(Keyboard, 'dismiss').mockImplementation(() => undefined); + }); + + it('dismisses the keyboard when the player count menu opens', () => { + render( + + + + ); + + capturedOnOpenMenu?.(); + + expect(Keyboard.dismiss).toHaveBeenCalledTimes(1); + }); + + it('dismisses the keyboard before creating a game from the menu', async () => { + render( + + + + ); + + await act(async () => { + capturedOnPressAction?.({ nativeEvent: { event: '2' } }); + }); + + expect(Keyboard.dismiss).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/components/FloatingActionButton.tsx b/src/components/FloatingActionButton.tsx index e4943489..d213aa42 100644 --- a/src/components/FloatingActionButton.tsx +++ b/src/components/FloatingActionButton.tsx @@ -3,7 +3,7 @@ import React from 'react'; import { MenuAction, MenuView } from '@react-native-menu/menu'; import { ParamListBase } from '@react-navigation/native'; import { NativeStackNavigationProp } from '@react-navigation/native-stack'; -import { StyleSheet, View } from 'react-native'; +import { Keyboard, StyleSheet, View } from 'react-native'; import { Icon } from 'react-native-elements'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; @@ -51,7 +51,11 @@ const FloatingActionButton: React.FunctionComponent = ({ navigation }) => }]}> { + Keyboard.dismiss(); + }} onPressAction={async ({ nativeEvent }) => { + Keyboard.dismiss(); const playerNumber = parseInt(nativeEvent.event); addGameHandler(playerNumber); }} diff --git a/src/screens/EditPlayerScreen.test.tsx b/src/screens/EditPlayerScreen.test.tsx index 6da3070f..f6b2ccfa 100644 --- a/src/screens/EditPlayerScreen.test.tsx +++ b/src/screens/EditPlayerScreen.test.tsx @@ -3,6 +3,7 @@ import React from 'react'; import { configureStore } from '@reduxjs/toolkit'; import { act, fireEvent, render, waitFor } from '@testing-library/react-native'; +import { Keyboard, Platform } from 'react-native'; import { Provider } from 'react-redux'; import gamesReducer from '../../redux/GamesSlice'; @@ -99,6 +100,10 @@ describe('EditPlayerScreen', () => { beforeEach(() => { jest.clearAllMocks(); + Object.defineProperty(Platform, 'OS', { + configurable: true, + value: 'ios', + }); }); it('should render null when playerId is null', () => { @@ -300,7 +305,7 @@ describe('EditPlayerScreen', () => { expect(state.players.entities['player-1']?.playerName).toBe('New Player Name'); }); - it('should revert to original name when input is empty on change', () => { + it('should allow empty local text without saving blank while editing', () => { const store = createMockStore({ settings: { currentGameId: 'game-1', @@ -335,7 +340,9 @@ describe('EditPlayerScreen', () => { const input = getByDisplayValue('Test Player'); fireEvent.changeText(input, ''); - // Check that the store was updated with the original name + expect(getByDisplayValue('')).toBeTruthy(); + + // Empty text is only local while editing; Redux keeps the last valid name. const state = store.getState(); expect(state.players.entities['player-1']?.playerName).toBe('Test Player'); }); @@ -386,6 +393,47 @@ describe('EditPlayerScreen', () => { expect(state.players.entities['player-1']?.playerName).toBe('Test Player'); }); + it('should revert to original name when empty input blurs', () => { + const store = createMockStore({ + settings: { + currentGameId: 'game-1', + }, + games: { + entities: { + 'game-1': mockGame, + }, + ids: ['game-1'], + }, + players: { + entities: { + 'player-1': mockPlayer, + }, + ids: ['player-1'], + }, + }); + + const mockRoute = { + params: { + index: 0, + playerId: 'player-1', + }, + }; + + const { getByDisplayValue } = render( + + + + ); + + const input = getByDisplayValue('Test Player'); + + fireEvent.changeText(input, ''); + fireEvent(input, 'blur'); + + expect(getByDisplayValue('Test Player')).toBeTruthy(); + expect(store.getState().players.entities['player-1']?.playerName).toBe('Test Player'); + }); + it('should save new name when input has text on end editing', () => { const store = createMockStore({ settings: { @@ -427,6 +475,11 @@ describe('EditPlayerScreen', () => { }); it('should clear input when clear button is pressed', () => { + Object.defineProperty(Platform, 'OS', { + configurable: true, + value: 'android', + }); + const store = createMockStore({ settings: { currentGameId: 'game-1', @@ -452,15 +505,67 @@ describe('EditPlayerScreen', () => { }, }; - const { getByTestId } = render( + const { getByDisplayValue, getByTestId } = render( ); - // Should render the input field successfully const input = getByTestId('RNE__Input__text-input'); - expect(input).toBeTruthy(); + const clearButton = input.parent?.parent?.findByProps({ name: 'close' }); + + if (clearButton == null) { + throw new Error('Expected Android clear button to render'); + } + + fireEvent.press(clearButton); + + expect(getByDisplayValue('')).toBeTruthy(); + expect(store.getState().players.entities['player-1']?.playerName).toBe('Test Player'); + }); + + it('should dismiss the keyboard before navigating back', async () => { + const dismissSpy = jest.spyOn(Keyboard, 'dismiss').mockImplementation(() => undefined); + const store = createMockStore({ + settings: { + currentGameId: 'game-1', + }, + games: { + entities: { + 'game-1': mockGame, + }, + ids: ['game-1'], + }, + players: { + entities: { + 'player-1': mockPlayer, + }, + ids: ['player-1'], + }, + }); + + const mockRoute = { + params: { + index: 0, + playerId: 'player-1', + }, + }; + + render( + + + + ); + + const options = mockNavigation.setOptions.mock.calls[0][0]; + const backButton = options.headerLeft({ tintColor: '#fff' }); + + await act(async () => { + await backButton.props.onPress(); + }); + + expect(dismissSpy).toHaveBeenCalled(); + expect(mockNavigation.goBack).toHaveBeenCalled(); }); it('should limit input to 15 characters', () => { @@ -597,7 +702,7 @@ describe('EditPlayerScreen', () => { expect(getByDisplayValue('')).toBeTruthy(); }); - it('should hide clear button when input is empty', () => { + it('should use the native iOS clear button mode', () => { const playerWithEmptyName = { id: 'player-2', playerName: '', @@ -636,10 +741,9 @@ describe('EditPlayerScreen', () => { ); const input = getByDisplayValue(''); - - // The clear button should be disabled and hidden when input is empty - const clearButton = input.parent?.parent?.findByProps({ name: 'close' }); - expect(clearButton?.props.disabled).toBe(true); + + expect(input.props.clearButtonMode).toBe('while-editing'); + expect(input.props.returnKeyType).toBe('done'); }); describe('suggestion feature', () => { diff --git a/src/screens/EditPlayerScreen.tsx b/src/screens/EditPlayerScreen.tsx index d04ab210..7beef574 100644 --- a/src/screens/EditPlayerScreen.tsx +++ b/src/screens/EditPlayerScreen.tsx @@ -1,21 +1,24 @@ -import React, { useLayoutEffect, useMemo, useRef, useState } from 'react'; +import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'; import { ParamListBase, RouteProp } from '@react-navigation/native'; import { NativeStackNavigationProp } from '@react-navigation/native-stack'; import { NativeSyntheticEvent, + Keyboard, + Platform, ScrollView, StyleSheet, Text, TextInput, TextInputEndEditingEventData, + TextInputSubmitEditingEventData, TouchableOpacity, View } from 'react-native'; import { Input } from 'react-native-elements'; import { useAppDispatch, useAppSelector } from '../../redux/hooks'; -import { updatePlayer } from '../../redux/PlayersSlice'; +import { selectAllPlayerNames, updatePlayer } from '../../redux/PlayersSlice'; import { selectCurrentGame } from '../../redux/selectors'; import { logEvent } from '../Analytics'; import HeaderButton from '../components/Buttons/HeaderButton'; @@ -40,11 +43,19 @@ const EditPlayerScreen: React.FC = ({ }) => { const theme = useTheme(); + const inputRef = React.useRef(null); + const selectedNameRef = useRef(null); + + const dismissInput = useCallback(() => { + inputRef.current?.blur(); + Keyboard.dismiss(); + }, []); useLayoutEffect(() => { navigation.setOptions({ headerLeft: ({ tintColor }) => ( { + dismissInput(); navigation.goBack(); await logEvent('edit_player_back'); }}> @@ -52,7 +63,7 @@ const EditPlayerScreen: React.FC = ({ ), }); - }, [navigation, theme.tint]); + }, [dismissInput, navigation, theme.tint]); const dispatch = useAppDispatch(); const currentGame = useAppSelector(selectCurrentGame); const { index, playerId } = route.params; @@ -68,13 +79,12 @@ const EditPlayerScreen: React.FC = ({ const [localPlayerName, setLocalPlayerName] = useState(player?.playerName || ''); const [isFocused, setIsFocused] = useState(false); - const allPlayerNames = useAppSelector(state => { - const names: string[] = []; - for (const p of Object.values(state.players.entities)) { - if (p) names.push(p.playerName); - } - return [...new Set(names)]; - }); + const allPlayerNames = useAppSelector(selectAllPlayerNames); + + useEffect(() => { + const unsubscribe = navigation.addListener('beforeRemove', dismissInput); + return unsubscribe; + }, [dismissInput, navigation]); const suggestions = useMemo(() => { if (!isFocused || localPlayerName.length === 0) return []; @@ -93,21 +103,28 @@ const EditPlayerScreen: React.FC = ({ const text = selectedNameRef.current ?? e.nativeEvent.text; selectedNameRef.current = null; - if (text == '') { - setLocalPlayerName(originalPlayerName); - savePlayerName(originalPlayerName); - } else { + commitPlayerName(text); + }; + + const onSubmitEditingHandler = (e: NativeSyntheticEvent) => { + commitPlayerName(e.nativeEvent.text); + }; + + const onChangeHandler = (text: string) => { + if (text != '') { savePlayerName(text); } + setLocalPlayerName(text); }; - const onChangeHandler = (text: string) => { + const commitPlayerName = (text: string) => { if (text == '') { + setLocalPlayerName(originalPlayerName); savePlayerName(originalPlayerName); } else { + setLocalPlayerName(text); savePlayerName(text); } - setLocalPlayerName(text); }; const savePlayerName = (text: string) => { @@ -120,7 +137,12 @@ const EditPlayerScreen: React.FC = ({ }; const onFocus = () => setIsFocused(true); - const onBlur = () => setIsFocused(false); + const onBlur = () => { + setIsFocused(false); + if (localPlayerName == '') { + commitPlayerName(originalPlayerName); + } + }; const onSuggestionSelect = (name: string) => { selectedNameRef.current = name; setLocalPlayerName(name); @@ -128,9 +150,10 @@ const EditPlayerScreen: React.FC = ({ setIsFocused(false); inputRef.current?.blur(); }; - - const inputRef = React.useRef(null); - const selectedNameRef = useRef(null); + const clearPlayerName = () => { + setLocalPlayerName(''); + inputRef.current?.focus(); + }; return ( @@ -138,17 +161,15 @@ const EditPlayerScreen: React.FC = ({ { - setLocalPlayerName(''); - inputRef.current?.focus(); - } + onPress: clearPlayerName, }} containerStyle={{ flex: 1 }} inputContainerStyle={{ @@ -160,10 +181,12 @@ const EditPlayerScreen: React.FC = ({ maxLength={15} onChangeText={onChangeHandler} onEndEditing={onEndEditingHandler} + onSubmitEditing={onSubmitEditingHandler} onFocus={onFocus} onBlur={onBlur} placeholder='Player Name' renderErrorMessage={false} + returnKeyType="done" selectTextOnFocus={true} style={[styles.input, { color: theme.textSecondary }]} value={localPlayerName}