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

Add delete button to Lightbox #41

Merged
merged 15 commits into from
May 6, 2024
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@observation.org/react-native-components",
"version": "1.31.0",
"version": "1.32.0",
"main": "src/index.ts",
"repository": "[email protected]:observation/react-native-components.git",
"author": "Observation.org",
Expand Down
103 changes: 68 additions & 35 deletions src/components/Lightbox.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react'
import React, { useState } from 'react'
import { SafeAreaView, StyleSheet, Text, TextStyle, TouchableOpacity, View } from 'react-native'

import ImageView from '@observation.org/react-native-image-viewing'
Expand All @@ -10,34 +10,40 @@ import font from '../styles/font'
import textStyle from '../styles/text'
import theme from '../styles/theme'

const hitSlop = { top: 16, left: 16, bottom: 16, right: 16 }

const getLightboxHeaderComponent =
(numberOfPages: number, onClose: () => void) =>
({ imageIndex }: { imageIndex: number }) => {
const hitSlop = { top: 16, left: 16, bottom: 16, right: 16 }
return (
<SafeAreaView style={styles.lightboxHeaderContainer}>
<View style={styles.lightboxHeader}>
<View style={{ flex: 1 }} />
<View style={styles.pageIndicator}>
<PageIndicator currentPage={imageIndex + 1} numberOfPages={numberOfPages} />
</View>
<View style={{ flex: 1 }}>
<TouchableOpacity style={styles.closeButton} onPress={() => onClose()} hitSlop={hitSlop}>
<Icon
name="times"
color={Color(theme.color.white).alpha(0.5).string()}
size={theme.icon.size.extraExtraLarge}
testID="close-lightbox"
/>
</TouchableOpacity>
</View>
({ imageIndex }: { imageIndex: number }) => (
<SafeAreaView style={styles.lightboxHeaderContainer}>
<View style={styles.lightboxHeader}>
<View style={{ flex: 1 }} />
<View style={styles.pageIndicator}>
<PageIndicator currentPage={imageIndex + 1} numberOfPages={numberOfPages} />
</View>
</SafeAreaView>
)
}
<View style={{ flex: 1 }}>
<TouchableOpacity style={styles.closeButton} onPress={() => onClose()} hitSlop={hitSlop}>
<Icon
name="times"
color={Color(theme.color.white).alpha(0.5).string()}
size={theme.icon.size.extraExtraLarge}
testID="close-lightbox"
/>
</TouchableOpacity>
</View>
</View>
</SafeAreaView>
)

const getLightboxFooterComponent =
(title?: string, description?: string, content?: React.ReactNode, style?: LightboxStyle) => () => (
(
title?: string,
description?: string,
content?: React.ReactNode,
style?: LightboxStyle,
onPressDelete?: () => void,
) =>
() => (
<SafeAreaView style={styles.lightboxFooterContainer}>
<View style={styles.lightboxFooter}>
{title && (
Expand All @@ -51,6 +57,15 @@ const getLightboxFooterComponent =
</View>
)}
{content && <View style={styles.footerItem}>{content}</View>}
{onPressDelete && (
<View style={styles.buttonsContainer}>
<View style={styles.buttonContainer}>
<TouchableOpacity onPress={onPressDelete} hitSlop={hitSlop}>
<Icon name="trash-alt" color={theme.color.white} size={20} testID="delete-photo" />
</TouchableOpacity>
</View>
</View>
)}
</View>
</SafeAreaView>
)
Expand All @@ -62,24 +77,33 @@ type LightboxStyle = {
type Props = {
index?: number
onClose: () => void
onDelete?: (imageIndex: number) => void
photos: string[]
title?: string
description?: string
content?: JSX.Element
style?: LightboxStyle
}

const Lightbox = ({ index, onClose, photos, title, description, content, style }: Props) => (
<ImageView
images={photos.map((photo) => ({ uri: photo }))}
imageIndex={index ?? 0}
visible={index !== undefined}
swipeToCloseEnabled={false}
onRequestClose={onClose}
HeaderComponent={getLightboxHeaderComponent(photos.length, onClose)}
FooterComponent={getLightboxFooterComponent(title, description, content, style)}
/>
)
const Lightbox = ({ index, onClose, onDelete, photos, title, description, content, style }: Props) => {
const initialIndex = index ?? 0
const [currentImageIndex, setCurrentImageIndex] = useState<number>()
Copy link
Member

Choose a reason for hiding this comment

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

Of dit om consistent te blijven?

Suggested change
const initialIndex = index ?? 0
const initialImageIndex = index ?? 0


const onPressDelete = onDelete ? () => onDelete(currentImageIndex ?? initialIndex) : undefined

return (
<ImageView
images={photos.map((photo) => ({ uri: photo }))}
imageIndex={initialIndex}
visible={index !== undefined}
swipeToCloseEnabled={false}
onImageIndexChange={setCurrentImageIndex}
onRequestClose={onClose}
HeaderComponent={getLightboxHeaderComponent(photos.length, onClose)}
FooterComponent={getLightboxFooterComponent(title, description, content, style, onPressDelete)}
/>
)
}

export default Lightbox

Expand Down Expand Up @@ -123,4 +147,13 @@ const styles = StyleSheet.create({
...textStyle.body,
color: theme.color.white,
},
buttonsContainer: {
flexDirection: 'row',
marginVertical: theme.margin.large,
marginHorizontal: theme.margin.common,
},
buttonContainer: {
width: '50%',
alignItems: 'center',
Copy link
Member

Choose a reason for hiding this comment

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

Dit gaat mis als we drie buttons hebben. Ik denk dat het beter is om dit te vervangen door flex: 0.5, dat gaat wel goed bij >2 buttons.

},
})
47 changes: 46 additions & 1 deletion src/components/__tests__/Lightbox.test.tsx
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Zoals ook gezegd in de andere PR voor dit issue: ik weet niet hoe ik deze test moet implementeren:
"Swiping to the left and pressing the delete button, will delete the second photo".

Dit omdat ik niet goed inzie hoe het swipen kan gesimuleerd worden.

Copy link
Member

Choose a reason for hiding this comment

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

Een swipe doen kan zo te zien niet via testing library, maar second best is om een onImageIndexChange call te triggeren vanuit ImageView. Zou dat kunnen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Je kunt zo'n onImageIndexChange triggeren als je met een testID erbij kan, maar dat was nu niet 't geval.
Wat ik nu heb gedaan is een mock met requireActual, die de mock inject. Dus 't is uiteindelijk gelukt.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ const content = <Text>Jan de Vogelaar</Text>

let onClose: () => void

let mockOnImageIndexChange = jest.fn()
jest.mock('@observation.org/react-native-image-viewing', () => {
const actualModule = jest.requireActual('@observation.org/react-native-image-viewing')
return (props: any) => {
mockOnImageIndexChange = props.onImageIndexChange as any
return actualModule.default(props)
}
})

describe('Lightbox', () => {
beforeEach(() => {
onClose = jest.fn()
Expand Down Expand Up @@ -59,6 +68,15 @@ describe('Lightbox', () => {

expect(toJSON()).toMatchSnapshot()
})

test('With a delete button', () => {
const { toJSON, queryByTestId } = render(
<Lightbox photos={photos} index={0} onClose={onClose} onDelete={() => {}} />,
)

expect(queryByTestId('delete-photo')).not.toBeNull()
expect(toJSON()).toMatchSnapshot()
})
})

describe('Interaction', () => {
Expand All @@ -70,7 +88,34 @@ describe('Lightbox', () => {
await fireEvent.press(getByTestId('close-lightbox'))

// THEN
expect(onClose).toBeCalled()
expect(onClose).toHaveBeenCalled()
})

test('Press delete button calls onDelete', async () => {
// GIVEN
const onDelete = jest.fn()
const { getByTestId } = render(<Lightbox photos={photos} index={0} onClose={onClose} onDelete={onDelete} />)

// WHEN
await fireEvent.press(getByTestId('delete-photo'))

// THEN
expect(onDelete).toHaveBeenCalledWith(0)
})

test('When swiping to the second photo and pressing the delete button, onDelete is called with the second photo', async () => {
// GIVEN
jest.mock('@observation.org/react-native-image-viewing', () => 'ImageCarousel')

const onDelete = jest.fn()
const { getByTestId } = render(<Lightbox photos={photos} index={0} onClose={onClose} onDelete={onDelete} />)

// WHEN
mockOnImageIndexChange(1)
await fireEvent.press(getByTestId('delete-photo'))

// THEN
expect(onDelete).toHaveBeenCalledWith(1)
})
})
})
Loading
Loading