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

LUN-357: Replace vocabulary detail modal with own route #306

Merged
merged 11 commits into from
Jun 1, 2022
46 changes: 30 additions & 16 deletions src/components/ExerciseHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ import React, { useEffect, useState } from 'react'
import { BackHandler } from 'react-native'
import { ProgressBar as RNProgressBar } from 'react-native-paper'
import { widthPercentageToDP as wp } from 'react-native-responsive-screen'
// import { HiddenItem } from 'react-navigation-header-buttons'
import styled, { useTheme } from 'styled-components/native'

// import { MenuIcon } from '../../assets/images'
import labels from '../constants/labels.json'
import { RoutesParams } from '../navigation/NavigationTypes'
import { Route, RoutesParams } from '../navigation/NavigationTypes'
import CustomModal from './CustomModal'
import FeedbackModal from './FeedbackModal'
import NavigationHeaderLeft from './NavigationHeaderLeft'
// import OverflowMenu from './OverflowMenu'
import { ContentSecondary } from './text/Content'

const ProgressBar = styled(RNProgressBar)`
Expand All @@ -25,22 +28,29 @@ const HeaderRightContainer = styled.View`
const ProgressText = styled(ContentSecondary)`
margin-right: ${props => props.theme.spacings.sm};
`
// /* TODO Remove comment when LUN-269 is ready */
// TODO Remove comment when LUN-269 is ready
// const MenuIconPrimary = styled(MenuIcon)`
// color: ${props => props.theme.colors.primary};
// `

interface ExerciseHeaderProps {
navigation: StackNavigationProp<RoutesParams, 'WordChoiceExercise' | 'ArticleChoiceExercise' | 'WriteExercise'>
currentWord: number
numberOfWords: number
navigation: StackNavigationProp<RoutesParams, Route>
currentWord?: number
numberOfWords?: number
confirmClose?: boolean
}

const ExerciseHeader = ({ navigation, currentWord, numberOfWords }: ExerciseHeaderProps): JSX.Element => {
const ExerciseHeader = ({
navigation,
currentWord,
numberOfWords,
confirmClose = true
}: ExerciseHeaderProps): JSX.Element => {
const [isModalVisible, setIsModalVisible] = useState(false)
const [isFeedbackModalVisible, setIsFeedbackModalVisible] = useState(false)
const theme = useTheme()
const progressText = numberOfWords !== 0 ? `${currentWord + 1} / ${numberOfWords}` : ''
const showProgress = numberOfWords !== undefined && numberOfWords > 0 && currentWord !== undefined
const progressText = showProgress ? `${currentWord + 1} / ${numberOfWords}` : ''

useEffect(
() =>
Expand Down Expand Up @@ -71,12 +81,14 @@ const ExerciseHeader = ({ navigation, currentWord, numberOfWords }: ExerciseHead

useEffect(() => {
const showModal = (): boolean => {
setIsModalVisible(true)
return true
if (confirmClose) {
setIsModalVisible(true)
return true
}
return false
}
const bEvent = BackHandler.addEventListener('hardwareBackPress', showModal)
return bEvent.remove
}, [])
return BackHandler.addEventListener('hardwareBackPress', showModal).remove
}, [confirmClose])

const goBack = (): void => {
setIsModalVisible(false)
Expand All @@ -85,10 +97,12 @@ const ExerciseHeader = ({ navigation, currentWord, numberOfWords }: ExerciseHead

return (
<>
<ProgressBar
progress={numberOfWords > 0 ? currentWord / numberOfWords : 0}
color={theme.colors.progressIndicator}
/>
{showProgress && (
<ProgressBar
progress={numberOfWords > 0 ? currentWord / numberOfWords : 0}
color={theme.colors.progressIndicator}
/>
)}

<CustomModal
testID='customModal'
Expand Down
45 changes: 45 additions & 0 deletions src/components/VocabularyList.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import React from 'react'
Copy link
Member Author

Choose a reason for hiding this comment

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

Just moved from VocabularyListScreen

import { FlatList } from 'react-native'
import styled from 'styled-components/native'

import { Document } from '../constants/endpoints'
import labels from '../constants/labels.json'
import Title from './Title'
import VocabularyListItem from './VocabularyListItem'

const Root = styled.View`
background-color: ${props => props.theme.colors.background};
height: 100%;
width: 100%;
padding: 0 ${props => props.theme.spacings.sm};
`

interface VocabularyListScreenProps {
documents: Document[]
onItemPress: (index: number) => void
}

const VocabularyList = ({ documents, onItemPress }: VocabularyListScreenProps): JSX.Element => {
const renderItem = ({ item, index }: { item: Document; index: number }): JSX.Element => (
<VocabularyListItem document={item} onPress={() => onItemPress(index)} />
)

return (
<Root>
<FlatList
ListHeaderComponent={
<Title
title={labels.exercises.vocabularyList.title}
description={`${documents.length} ${documents.length === 1 ? labels.general.word : labels.general.words}`}
/>
}
data={documents}
renderItem={renderItem}
keyExtractor={item => `${item.id}`}
showsVerticalScrollIndicator={false}
/>
</Root>
)
}

export default VocabularyList
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import React, { ReactElement } from 'react'
import { widthPercentageToDP as wp } from 'react-native-responsive-screen'
Copy link
Member Author

Choose a reason for hiding this comment

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

This is why I don't really like route specific component folders, once it is used in multiple routes it has to be moved/renamed.

Copy link
Member

Choose a reason for hiding this comment

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

Yes i see, but the need to move and rename a file, just happend for the first time and will maybe occur once in a few weeks/months, whereas I am looking for files every day and finding them in a folder with ~100 files is a really a pain.

Copy link
Member Author

@steffenkleinle steffenkleinle May 31, 2022

Choose a reason for hiding this comment

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

yeah there are different opinions about this which is definitely fine. I personally find it worse that I have to look in multiple folders to find the right file than to find the file in a large folder. No need to change this though, especially if you favor the other way.

Copy link
Contributor

Choose a reason for hiding this comment

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

it is now merged. you can base on it

Copy link
Member Author

@steffenkleinle steffenkleinle May 31, 2022

Choose a reason for hiding this comment

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

Already on it 🚀 Assuming your comment is regarding #286 and not this discussion :D

import styled from 'styled-components/native'

import AudioPlayer from '../../../components/AudioPlayer'
import ListItem from '../../../components/ListItem'
import { ContentTextLight } from '../../../components/text/Content'
import { Document } from '../../../constants/endpoints'
import { getArticleColor } from '../../../services/helpers'
import { Document } from '../constants/endpoints'
import { getArticleColor } from '../services/helpers'
import AudioPlayer from './AudioPlayer'
import ListItem from './ListItem'
import { ContentTextLight } from './text/Content'

const StyledImage = styled.Image`
margin-right: ${props => props.theme.spacings.sm};
Expand All @@ -29,12 +29,12 @@ const Speaker = styled.View`
padding-top: ${props => props.theme.spacings.sm};
`

export interface VocabularyListItemProp {
interface VocabularyListItemProps {
document: Document
setIsModalVisible?: () => void
onPress: () => void
}

const VocabularyListItem = ({ document, setIsModalVisible }: VocabularyListItemProp): ReactElement => {
const VocabularyListItem = ({ document, onPress }: VocabularyListItemProps): ReactElement => {
const { article, word, document_image: documentImage } = document

const title = <StyledTitle articleColor={getArticleColor(article)}>{article.value}</StyledTitle>
Expand All @@ -43,13 +43,11 @@ const VocabularyListItem = ({ document, setIsModalVisible }: VocabularyListItemP
<StyledImage testID='image' source={{ uri: documentImage[0].image }} width={24} height={24} />
) : undefined

const noop = () => undefined

return (
<ListItem
title={title}
description={word}
onPress={setIsModalVisible ?? noop}
onPress={onPress}
icon={icon}
rightChildren={
<Speaker>
Expand Down
55 changes: 55 additions & 0 deletions src/components/__tests__/VocabularyList.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { fireEvent } from '@testing-library/react-native'
import React from 'react'

import labels from '../../constants/labels.json'
import DocumentBuilder from '../../testing/DocumentBuilder'
import { mockUseLoadAsyncWithData } from '../../testing/mockUseLoadFromEndpoint'
import render from '../../testing/render'
import VocabularyList from '../VocabularyList'

jest.mock('../AudioPlayer', () => {
const Text = require('react-native').Text
return () => <Text>AudioPlayer</Text>
})

describe('VocabularyList', () => {
beforeEach(() => {
jest.clearAllMocks()
})

const onItemPress = jest.fn()
const documents = new DocumentBuilder(2).build()

it('should display vocabulary list', () => {
mockUseLoadAsyncWithData(documents)

const { getByText, getAllByText, getAllByTestId } = render(
<VocabularyList documents={documents} onItemPress={onItemPress} />
)

expect(getByText(labels.exercises.vocabularyList.title)).toBeTruthy()
expect(getByText(`2 ${labels.general.words}`)).toBeTruthy()
expect(getByText('Der')).toBeTruthy()
expect(getByText('Spachtel')).toBeTruthy()
expect(getByText('Das')).toBeTruthy()
expect(getByText('Auto')).toBeTruthy()
expect(getAllByText('AudioPlayer')).toHaveLength(2)
expect(getAllByTestId('image')).toHaveLength(2)
})

it('should call onItemPress with correct index', () => {
const { getByText } = render(<VocabularyList documents={documents} onItemPress={onItemPress} />)

expect(onItemPress).toHaveBeenCalledTimes(0)

fireEvent.press(getByText('Auto'))

expect(onItemPress).toHaveBeenCalledTimes(1)
expect(onItemPress).toHaveBeenCalledWith(1)

fireEvent.press(getByText('Spachtel'))

expect(onItemPress).toHaveBeenCalledTimes(2)
expect(onItemPress).toHaveBeenCalledWith(0)
})
})
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { fireEvent } from '@testing-library/react-native'
import React from 'react'
import 'react-native'

import { ARTICLES } from '../../../../constants/data'
import { Document } from '../../../../constants/endpoints'
import render from '../../../../testing/render'
import { ARTICLES } from '../../constants/data'
import { Document } from '../../constants/endpoints'
import render from '../../testing/render'
import VocabularyListItem from '../VocabularyListItem'

jest.mock('../../../../components/AudioPlayer', () => () => null)
jest.mock('../AudioPlayer', () => () => null)

describe('VocabularyListItem', () => {
const onPress = jest.fn()

const document: Document = {
article: ARTICLES[1],
audio: '',
Expand All @@ -19,17 +22,21 @@ describe('VocabularyListItem', () => {
}

it('should display image passed to it', () => {
const { getByTestId } = render(<VocabularyListItem document={document} />)
const { getByTestId } = render(<VocabularyListItem document={document} onPress={onPress} />)
expect(getByTestId('image')).toHaveProp('source', { uri: document.document_image[0].image })
})

it('should display article passed to it', () => {
const { queryByText } = render(<VocabularyListItem document={document} />)
const { queryByText } = render(<VocabularyListItem document={document} onPress={onPress} />)
expect(queryByText(document.article.value)).toBeTruthy()
})

it('should display word passed to it', () => {
const { queryByText } = render(<VocabularyListItem document={document} />)
expect(queryByText(document.word)).toBeTruthy()
const { getByText } = render(<VocabularyListItem document={document} onPress={onPress} />)
expect(getByText(document.word)).toBeTruthy()

expect(onPress).toHaveBeenCalledTimes(0)
fireEvent.press(getByText(document.word))
expect(onPress).toHaveBeenCalledTimes(1)
})
})
5 changes: 5 additions & 0 deletions src/navigation/NavigationTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ interface ExerciseParams {
closeExerciseAction: CommonNavigationAction
}

type DetailExerciseParams = ExerciseParams & {
documentIndex: number
}

export interface ExercisesParams extends Omit<ExerciseParams, 'documents' | 'closeExerciseAction'> {
discipline: Discipline
documents: Document[] | null
Expand Down Expand Up @@ -47,6 +51,7 @@ export type RoutesParams = {
discipline: Discipline
initialSelection: boolean
}
VocabularyDetail: DetailExerciseParams
Exercises: ExercisesParams
VocabularyList: ExerciseParams
WordChoiceExercise: ExerciseParams
Expand Down
8 changes: 7 additions & 1 deletion src/navigation/Navigator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ import labels from '../constants/labels.json'
import { useTabletHeaderHeight } from '../hooks/useTabletHeaderHeight'
import ResultDetailScreen from '../routes/ResultDetailScreen'
import ResultScreen from '../routes/ResultScreen'
import VocabularyDetailScreen from '../routes/VocabularyDetailScreen'
import VocabularyListScreen from '../routes/VocabularyListScreen'
import ArticleChoiceExerciseScreen from '../routes/choice-exercises/ArticleChoiceExerciseScreen'
import WordChoiceExerciseScreen from '../routes/choice-exercises/WordChoiceExerciseScreen'
import ExerciseFinishedScreen from '../routes/exercise-finished/ExerciseFinishedScreen'
import VocabularyListScreen from '../routes/vocabulary-list/VocabularyListScreen'
import WriteExerciseScreen from '../routes/write-exercise/WriteExerciseScreen'
import BottomTabNavigator from './BottomTabNavigator'
import { RoutesParams } from './NavigationTypes'
Expand All @@ -33,6 +34,11 @@ const HomeStackNavigator = (): JSX.Element | null => {
component={VocabularyListScreen}
options={({ navigation }) => options(overviewExercises, navigation, false)}
/>
<Stack.Screen
name='VocabularyDetail'
component={VocabularyDetailScreen}
options={({ navigation }) => options('', navigation, true)}
/>
<Stack.Screen
name='WordChoiceExercise'
component={WordChoiceExerciseScreen}
Expand Down
9 changes: 7 additions & 2 deletions src/routes/ResultDetailScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { DoubleCheckCircleIconWhite, ArrowRightIcon, RepeatIcon } from '../../as
import Button from '../components/Button'
import Loading from '../components/Loading'
import Title from '../components/Title'
import VocabularyListItem from '../components/VocabularyListItem'
import { BUTTONS_THEME, ExerciseKeys, RESULTS } from '../constants/data'
import labels from '../constants/labels.json'
import { DocumentResult, RoutesParams } from '../navigation/NavigationTypes'
import VocabularyListItem from './vocabulary-list/components/VocabularyListItem'

const Root = styled.View`
background-color: ${prop => prop.theme.colors.background};
Expand Down Expand Up @@ -73,7 +73,12 @@ const ResultDetailScreen = ({ route, navigation }: ResultScreenProps): JSX.Eleme
/>
)

const Item = ({ item }: { item: DocumentResult }): JSX.Element => <VocabularyListItem document={item.document} />
const Item = ({ item, index }: { item: DocumentResult; index: number }): JSX.Element => (
<VocabularyListItem
document={item.document}
onPress={() => navigation.navigate('VocabularyDetail', { ...route.params, documentIndex: index })}
/>
)

const repeatIncorrectEntries = (): void =>
navigation.navigate('WriteExercise', {
Expand Down
Loading