Skip to content

Commit

Permalink
parent 5681d2f
Browse files Browse the repository at this point in the history
author Alex Ware <alex.ware@guardian.co.uk> 1571648155 +0100
committer Alex Ware <alex.ware@guardian.co.uk> 1572441783 +0000

Smaller thumbnail trail images

don't break thumbs in existing installs or existing content downloaded

archiver fixes

oops

fix lint and test

pr changes

pr comment

fix build

fix test?
  • Loading branch information
AWare committed Oct 30, 2019
1 parent c86fe6f commit 79a2ba0
Show file tree
Hide file tree
Showing 23 changed files with 328 additions and 682 deletions.
2 changes: 1 addition & 1 deletion projects/Mallard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"shake-android": "echo 'πŸ‘‹ πŸ“±' && adb shell input keyevent 82 && echo 'πŸ™ŒπŸ“²πŸ™Œ'",
"wifi-android": "ip=$(adb shell ifconfig wlan0 | awk '/inet addr/{print substr($2,6)}') && adb tcpip 5555 && adb connect $ip:5555",
"run-android": "yarn pre-run && adb reverse tcp:8081 tcp:8081 && adb reverse tcp:8097 tcp:8097 && adb reverse tcp:3131 tcp:3131 && react-native run-android",
"run-ios": "yarn pre-run && yarn install-pods && react-native run-ios",
"run-ios": "yarn pre-run && yarn install-pods && react-native run-ios",
"run-ipad": "yarn pre-run && react-native run-ios --simulator=\"iPad Air 2\"",
"validate": "cd ../.. && make validate-mallard",
"fix": "cd ../.. && make fix-mallard",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,7 @@ const Image = ({
const path = `${backend}${mediaPath(
publishedId,
imageSize,
imageElement.src.source,
imageElement.src.path,
imageElement.src,
)}`

return ImageBase({ path, ...imageElement })
Expand Down
7 changes: 5 additions & 2 deletions projects/Mallard/src/components/front/image-resource.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from 'react-native'
import { useAspectRatio } from 'src/hooks/use-aspect-ratio'
import { useImagePath, useScaledImage } from 'src/hooks/use-image-paths'
import { Image as IImage } from '../../../../common/src'
import { Image as IImage, ImageUse } from '../../../../common/src'

/**
* This component abstracts away the endpoint for images
Expand All @@ -22,6 +22,7 @@ import { Image as IImage } from '../../../../common/src'
*/
type ImageResourceProps = {
image: IImage
use: ImageUse
style?: StyleProp<ImageStyle>
setAspectRatio?: boolean
} & Omit<ImageProps, 'source'>
Expand Down Expand Up @@ -51,12 +52,14 @@ const ImageResource = ({
image,
style,
setAspectRatio = false,
use, //eslint-disable-line
...props
}: ImageResourceProps) => {
const [width, setWidth] = useState<number | null>(null)
const imagePath = useImagePath(image)
const imagePath = useImagePath(image, 'full-size') //TODO: This should be changed to use once we have a good amount of content published with trail thumbs
const aspectRatio = useAspectRatio(imagePath)
const styles = [style, setAspectRatio && aspectRatio ? { aspectRatio } : {}]

return width && imagePath ? (
<ScaledImageResource
key={imagePath} // an attempt to fix https://github.com/facebook/react-native/issues/9195
Expand Down
14 changes: 9 additions & 5 deletions projects/Mallard/src/components/front/items/image-items.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ import {
PropTypes,
tappablePadding,
} from './helpers/item-tappable'
import { isFullWidthItem, isSmallItem, isFullHeightItem } from './helpers/sizes'
import {
getImageHeight,
isFullHeightItem,
isFullWidthItem,
isSmallItem,
} from './helpers/sizes'
import { SportItemBackground } from './helpers/sports'
import { TextBlock } from './helpers/text-block'
import { SmallItem } from './small-items'
import { Standfirst } from './helpers/standfirst'
import { TextBlock } from './helpers/text-block'
import { useIsOpinionCard, useIsSportCard } from './helpers/types'
import { SmallItem } from './small-items'
import { TrailImageView } from './trail-image-view'
import { getImageHeight } from './helpers/sizes'

/*
Normal img on top + text
Expand All @@ -41,7 +45,6 @@ const imageStyles = StyleSheet.create({

const ImageItem = ({ article, size, ...tappableProps }: PropTypes) => {
const [isOpinionCard, isSportCard] = [useIsOpinionCard(), useIsSportCard()]

if (isOpinionCard && isSmallItem(size)) {
return <RoundImageItem {...{ article, size, ...tappableProps }} />
}
Expand Down Expand Up @@ -98,6 +101,7 @@ const RoundImageItem = ({ article, size, ...tappableProps }: PropTypes) => {
<ImageResource
style={[imageStyles.roundImage]}
image={article.bylineImages.cutout}
use="thumb"
/>
) : null}
</ItemTappable>
Expand Down
2 changes: 2 additions & 0 deletions projects/Mallard/src/components/front/items/items.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const CoverItem = ({ article, size, ...tappableProps }: PropTypes) => {
<ImageResource
style={coverStyles.cover}
image={article.trailImage}
use="thumb"
/>
) : null}
<TextBlock
Expand Down Expand Up @@ -101,6 +102,7 @@ const SplashImageItem = ({ article, size, ...tappableProps }: PropTypes) => {
style={[splashImageStyles.image]}
image={cardImage}
setAspectRatio
use="thumb"
/>
</View>
<HeadlineCardText style={[splashImageStyles.hidden]}>
Expand Down
18 changes: 15 additions & 3 deletions projects/Mallard/src/components/front/items/trail-image-view.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react'
import { StyleSheet, View, StyleProp } from 'react-native'
import { StyleProp, StyleSheet, View } from 'react-native'
import { CAPIArticle, ImageUse } from 'src/common'
import { Stars } from 'src/components/stars/stars'
import { CAPIArticle } from 'src/common'
import { useMediaQuery } from 'src/hooks/use-screen'
import { Breakpoints } from 'src/theme/breakpoints'
import { ImageResource } from '../image-resource'
import { SportScore } from 'src/components/sportscore/sportscore'
import { ArticleType } from '../../../../../common/src'
Expand Down Expand Up @@ -35,10 +37,18 @@ export const TrailImageView = ({
article: CAPIArticle
style: StyleProp<{ width?: string; height?: string; marginLeft?: number }>
}) => {
const isTablet = useMediaQuery(width => width >= Breakpoints.tabletVertical)

const { trailImage: image } = article
if (image == null) {
return null
}
const use: ImageUse =
'use' in image
? isTablet
? image.use.tablet
: image.use.mobile
: 'full-size'
const frameStyle = [trailImageViewStyles.frame, style]
const starRating = article.type === 'article' && article.starRating
const sportScore =
Expand All @@ -50,6 +60,7 @@ export const TrailImageView = ({
<ImageResource
style={trailImageViewStyles.image}
image={image}
use={use}
/>
<Stars
style={trailImageViewStyles.rating}
Expand All @@ -63,6 +74,7 @@ export const TrailImageView = ({
<ImageResource
style={trailImageViewStyles.image}
image={image}
use={use}
/>
<SportScore
style={trailImageViewStyles.rating}
Expand All @@ -71,6 +83,6 @@ export const TrailImageView = ({
</View>
)
} else {
return <ImageResource style={frameStyle} image={image} />
return <ImageResource style={frameStyle} image={image} use={use} />
}
}
31 changes: 19 additions & 12 deletions projects/Mallard/src/hooks/use-image-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,34 @@ import ImageResizer from 'react-native-image-resizer'
import RNFetchBlob from 'rn-fetch-blob'
import { imageForScreenSize } from 'src/helpers/screen'
import { APIPaths, FSPaths } from 'src/paths'
import { Image, ImageSize, Issue } from '../../../common/src'
import { Image, ImageSize, Issue, ImageUse } from '../../../common/src'
import { useSettingsValue } from './use-settings'
import { useIssueSummary } from './use-issue-summary'
import { Platform } from 'react-native'

const getFsPath = (
localIssueId: Issue['localId'],
{ source, path }: Image,
image: Image,
size: ImageSize,
) => FSPaths.media(localIssueId, source, path, size)
use: ImageUse,
) => FSPaths.image(localIssueId, size, image, use)

export const selectImagePath = async (
apiUrl: string,
localIssueId: Issue['localId'],
publishedIssueId: Issue['publishedId'],
{ source, path }: Image,
image: Image,
use: ImageUse,
) => {
const imageSize = await imageForScreenSize()
const api = `${apiUrl}${APIPaths.media(
const api = `${apiUrl}${APIPaths.image(
publishedIssueId,
imageSize,
source,
path,
image,
use,
)}`

const fs = getFsPath(localIssueId, { source, path }, imageSize)
const fs = getFsPath(localIssueId, image, imageSize, use)
const fsExists = await RNFetchBlob.fs.exists(fs)

const fsUpdatedPath = Platform.OS === 'android' ? 'file:///' + fs : fs
Expand Down Expand Up @@ -57,21 +59,26 @@ const compressImagePath = async (path: string, width: number) => {
*
* */

export const useImagePath = (image?: Image) => {
export const useImagePath = (image?: Image, use: ImageUse = 'full-size') => {
const { issueId } = useIssueSummary()

const [paths, setPaths] = useState<string | undefined>()
const apiUrl = useSettingsValue.apiUrl()
useEffect(() => {
if (issueId && image) {
const { localIssueId, publishedIssueId } = issueId
selectImagePath(apiUrl, localIssueId, publishedIssueId, image).then(
setPaths,
)
selectImagePath(
apiUrl,
localIssueId,
publishedIssueId,
image,
use,
).then(setPaths)
}
}, [
apiUrl,
image,
use,
issueId ? issueId.publishedIssueId : undefined, // Why isn't this just issueId?
issueId ? issueId.localIssueId : undefined,
])
Expand Down
32 changes: 22 additions & 10 deletions projects/Mallard/src/paths/__tests__/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,6 @@ describe('paths', () => {
)
})

it('should give correct media root directory', () => {
expect(FSPaths.mediaRoot('daily-edition/2019-10-10')).toEqual(
'path/to/base/directory/issues/daily-edition/2019-10-10/media',
)
})

it('should give correct zip file location based on a local issue id and filename', () => {
expect(
FSPaths.zip('daily-edition/2019-10-10', '2019-10-10'),
Expand All @@ -57,17 +51,35 @@ describe('paths', () => {
)
})

it('should give a media path on the local device', () => {
it('should give a media path on the local device for a full sized image', () => {
expect(
FSPaths.media(
FSPaths.image(
'daily-edition/2019-10-10',
'source',
'path',
'phone',
{
source: 'source',
path: 'path',
},
'full-size',
),
).toEqual(
'path/to/base/directory/issues/daily-edition/2019-10-10/media/phone/source/path',
)
})
it('should give a media path on the local device for a thumbnail image', () => {
expect(
FSPaths.image(
'daily-edition/2019-10-10',
'phone',
{
source: 'source',
path: 'path',
},
'thumb',
),
).toEqual(
'path/to/base/directory/issues/daily-edition/2019-10-10/thumbs/phone/thumb/source/path',
)
})
})
})
18 changes: 9 additions & 9 deletions projects/Mallard/src/paths/index.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import {
issuePath,
mediaPath,
frontPath,
Issue,
Collection,
Front,
CAPIArticle,
ImageSize,
Image,
ImageUse,
} from 'src/common'
import RNFetchBlob from 'rn-fetch-blob'
import { defaultSettings } from 'src/helpers/settings/defaults'
import { imagePath } from '../../../common/src'

export interface PathToIssue {
localIssueId: Issue['localId']
Expand All @@ -27,26 +29,24 @@ export interface PathToArticle {
export const APIPaths = {
issue: issuePath,
front: frontPath,
media: mediaPath,
image: imagePath,
}

const issuesDir = `${RNFetchBlob.fs.dirs.DocumentDir}/issues`

const issueRoot = (localIssueId: string) => `${issuesDir}/${localIssueId}`
const mediaRoot = (localIssueId: string) => `${issueRoot(localIssueId)}/media`
export const MEDIA_CACHE_DIRECTORY_NAME = 'cached'

export const FSPaths = {
issuesDir,
issueRoot,
mediaRoot,
contentPrefixDir: `${issuesDir}/${defaultSettings.contentPrefix}`,
media: (
issueRoot,
image: (
localIssueId: string,
source: string,
path: string,
size: ImageSize,
) => `${mediaRoot(localIssueId)}/${size}/${source}/${path}`,
image: Image,
use: ImageUse,
) => imagePath(issueRoot(localIssueId), size, image, use),
zip: (localIssueId: string, filename: string) =>
`${issueRoot(localIssueId)}/${filename}.zip`,
issueZip: (localIssueId: string) => `${issueRoot(localIssueId)}/data.zip`,
Expand Down
27 changes: 26 additions & 1 deletion projects/archiver/src/tasks/front/helpers/media.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { CAPIArticle } from '../../../../common'
import { CAPIArticle, TrailImage } from '../../../../common'
import { getImagesFromArticle } from './media'

test('getImage', () => {
Expand All @@ -24,3 +24,28 @@ test('getImage', () => {
}
expect(getImagesFromArticle(article)).toContain(image)
})

test('getImageUse', () => {
const image: TrailImage = {
source: 'test',
path: 'image',
use: { mobile: 'full-size', tablet: 'thumb' },
}
const article: CAPIArticle = {
key: 'πŸ”‘',
type: 'article',
headline: 'πŸ—£',
showByline: false,
byline: '🧬',
standfirst: 'πŸ₯‡',
kicker: 'πŸ₯Ύ',
trail: 'πŸ›£',
trailImage: image,
showQuotedHeadline: false,
mediaType: 'Image',
elements: [],
isFromPrint: false,
bylineHtml: '<a>🧬</<a> Senior person',
}
expect(getImagesFromArticle(article)).toContain(image)
})
Loading

0 comments on commit 79a2ba0

Please sign in to comment.