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.

Sign up for GitHub

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

[PAY-2456][PAY-2438][PAY-2435] Bring mobile purchase flow up to lossless spec #7490

Merged
merged 8 commits into from
Feb 7, 2024
Merged

[PAY-2456][PAY-2438][PAY-2435] Bring mobile purchase flow up to lossless spec #7490

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
536f82d
[PAY-2456] Update mobile purchase flow for lossless
raymondjacobson Feb 7, 2024
3d3b8ac
[PAY-2438] Hide download all button when unnecessary
raymondjacobson Feb 7, 2024
144c33c
[PAY-2435] fire handlePressOut on native harmony button long press
raymondjacobson Feb 7, 2024
0a935c2
Undo api client change
raymondjacobson Feb 7, 2024
0334b31
Remove comment
raymondjacobson Feb 7, 2024
c88fe30
Add download access for download all button
raymondjacobson Feb 7, 2024
0306d60
Address comments
raymondjacobson Feb 7, 2024
36df262
Remove hasStreamAccess check
raymondjacobson Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mobile/src/components/challenge-rewards-drawer/CooldownSummaryTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const CooldownSummaryTable = ({
<SummaryTable
title={messages.upcomingRewards}
secondaryTitle={messages.audio}
summaryValueColor='neutral'
summaryValueColor='default'
items={cooldownChallenges}
summaryItem={cooldownChallengesSummary}
/>
Expand Down
12 changes: 11 additions & 1 deletion packages/mobile/src/components/premium-track-purchase-drawer/PremiumTrackPurchaseDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,13 @@ const RenderForm = ({
}
}, [setPurchaseVendor, showCoinflow, purchaseVendor])

const stemsPurchaseCount = track.is_download_gated
? track._stems?.length ?? 0
: 0
const downloadPurchaseCount =
track.is_download_gated && track.download?.is_downloadable ? 1 : 0
const streamPurchaseCount = track.is_stream_gated ? 1 : 0

return (
<View style={styles.root}>
{page === PurchaseContentPage.PURCHASE ? (
Expand All @@ -324,7 +331,7 @@ const RenderForm = ({
<AudioMatchSection amount={Math.round(price / 100)} />
) : null}
<View style={styles.formContentSection}>
<TrackDetailsTile trackId={track.track_id} />
<TrackDetailsTile trackId={track.track_id} showLabel={false} />
{isPurchaseSuccessful ? null : (
<PayExtraFormSection
amountPresets={presetValues}
Expand All @@ -334,6 +341,9 @@ const RenderForm = ({
<View style={styles.bottomSection}>
<PurchaseSummaryTable
{...purchaseSummaryValues}
stemsPurchaseCount={stemsPurchaseCount}
downloadPurchaseCount={downloadPurchaseCount}
streamPurchaseCount={streamPurchaseCount}
totalPriceInCents={totalPriceInCents}
/>
{isIOSDisabled || isUnlocking || isPurchaseSuccessful ? null : (
Expand Down
39 changes: 33 additions & 6 deletions packages/mobile/src/components/premium-track-purchase-drawer/PurchaseSummaryTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,58 @@ const messages = {
summary: 'Total',
payExtra: 'Pay Extra',
premiumTrack: 'Premium Track',
downloadableFiles: 'Downloadable Files',
existingBalance: 'Existing Balance',
total: 'Total',
youPaid: 'You Paid',
price: (price: string) => `$${price}`,
subtractPrice: (price: string) => `-$${price}`
subtractPrice: (price: string) => `-$${price}`,
included: 'included'
}

type PurchaseSummaryTableProps = {
totalPriceInCents: number
basePrice: number
extraAmount?: number
// How many "streams" are available for purchase
// Prior to albums/bundles this should be 0 or 1.
streamPurchaseCount?: number
// How many "downloads" are available for purchase
// Prior to albums/bundles this should be 0 or 1.
downloadPurchaseCount?: number
// How many stems are available for purchase
stemsPurchaseCount?: number
}

export const PurchaseSummaryTable = ({
totalPriceInCents,
basePrice,
extraAmount
extraAmount,
streamPurchaseCount,
downloadPurchaseCount,
stemsPurchaseCount
}: PurchaseSummaryTableProps) => {
const items: SummaryTableItem[] = [
{
const items: SummaryTableItem[] = []
if (streamPurchaseCount) {
items.push({
id: 'premiumTrack',
label: messages.premiumTrack,
value: messages.price(formatPrice(basePrice))
}
]
})
}

const downloadCount = (stemsPurchaseCount ?? 0) + (downloadPurchaseCount ?? 0)
if (downloadCount > 0) {
items.push({
id: 'premiumTrackDownload',
label: `${messages.downloadableFiles} (${downloadCount})`,
value: streamPurchaseCount
? messages.included
: messages.price(formatPrice(basePrice)),
color: streamPurchaseCount ? 'subdued' : 'default'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this seems brittle - we're trying to alternate colors in the table rows right? there's gotta be a better way to do this, but 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

not exactly. just show the color as lighter gray when the track is "stream for sale"

})
}

if (extraAmount != null) {
items.push({
id: 'payExtra',
Expand Down
39 changes: 23 additions & 16 deletions packages/mobile/src/components/summary-table/SummaryTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import React, { useCallback, useState } from 'react'
import { removeNullable } from '@audius/common/utils'
import { LayoutAnimation, View } from 'react-native'

import { Text } from 'app/components/core'
import type { TextColors } from '@audius/harmony-native'
import { Text } from '@audius/harmony-native'
import { Expandable, ExpandableArrowIcon } from 'app/components/expandable'
import { flexRowCentered, makeStyles } from 'app/styles'
import { type ThemeColors } from 'app/utils/theme'

const useStyles = makeStyles(({ spacing, palette }) => ({
container: {
Expand Down Expand Up @@ -42,15 +42,16 @@ export type SummaryTableItem = {
icon?: React.FC
content?: ReactNode // expandable content
disabled?: boolean
color?: TextColors
}

export type SummaryTableProps = {
items: SummaryTableItem[]
summaryItem?: SummaryTableItem
title: ReactNode
secondaryTitle?: ReactNode
summaryLabelColor?: keyof ThemeColors
summaryValueColor?: keyof ThemeColors
summaryLabelColor?: TextColors
summaryValueColor?: TextColors
renderBody?: (items: SummaryTableItem[]) => ReactNode
/** Enables an expand/collapse interaction. Only the title shows when collapsed. */
collapsible?: boolean
Expand All @@ -62,7 +63,7 @@ export const SummaryTable = ({
title,
secondaryTitle,
summaryLabelColor,
summaryValueColor = 'secondary',
summaryValueColor = 'accent',
renderBody: renderBodyProp,
collapsible = false
}: SummaryTableProps) => {
Expand All @@ -82,16 +83,20 @@ export const SummaryTable = ({
<View style={[styles.row, styles.grayRow]}>
<View style={styles.collapsibleTitle}>
<ExpandableArrowIcon expanded={isExpanded} iconSize='s' />
<Text weight='bold'>{title}</Text>
<Text variant='title' strength='default'>
{title}
</Text>
</View>
<Text variant='body' fontSize='large' weight='bold'>
<Text variant='title' strength='default'>
{secondaryTitle}
</Text>
</View>
) : (
<View style={[styles.row, styles.grayRow]}>
<Text weight='bold'>{title}</Text>
<Text variant='body' fontSize='large' weight='bold'>
<Text variant='title' strength='default'>
{title}
</Text>
<Text variant='title' strength='default'>
{secondaryTitle}
</Text>
</View>
Expand All @@ -104,16 +109,16 @@ export const SummaryTable = ({
<View style={[styles.row, styles.lastRow, styles.grayRow]}>
<Text
variant='body'
fontSize='medium'
weight='bold'
size='m'
strength='strong'
color={summaryLabelColor}
>
{summaryItem.label}
</Text>
<Text
variant='body'
fontSize='medium'
weight='bold'
size='m'
strength='strong'
color={summaryValueColor}
>
{summaryItem.value}
Expand All @@ -127,7 +132,7 @@ export const SummaryTable = ({
<>
{renderBodyProp
? renderBodyProp(items)
: nonNullItems.map(({ id, label, value }, index) => (
: nonNullItems.map(({ id, label, value, color }, index) => (
<View
key={id}
style={[
Expand All @@ -137,8 +142,10 @@ export const SummaryTable = ({
: null
]}
>
<Text>{label}</Text>
<Text>{value}</Text>
<Text variant='body'>{label}</Text>
<Text variant='body' color={color}>
{value}
</Text>
</View>
))}
{renderSummaryItem()}
Expand Down
40 changes: 23 additions & 17 deletions packages/mobile/src/components/track-details-tile/TrackDetailsTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,13 @@ const useStyles = makeStyles(({ spacing, palette }) => ({

type TrackDetailsTileProps = {
trackId: ID
showLabel?: boolean
}

export const TrackDetailsTile = ({ trackId }: TrackDetailsTileProps) => {
export const TrackDetailsTile = ({
trackId,
showLabel = true
}: TrackDetailsTileProps) => {
const styles = useStyles()
const { accentBlue, specialLightGreen } = useThemeColors()
const track = useSelector((state) => getTrack(state, { id: trackId }))
Expand Down Expand Up @@ -145,22 +149,24 @@ export const TrackDetailsTile = ({ trackId }: TrackDetailsTileProps) => {
size={SquareSizes.SIZE_150_BY_150}
/>
<View style={styles.metadataContainer}>
<View style={styles.streamContentLabelContainer}>
<IconComponent
fill={color}
width={spacing(5)}
height={spacing(5)}
/>
<Text
fontSize='small'
colorValue={color}
weight='demiBold'
textTransform='uppercase'
style={styles.streamContentLabel}
>
{title}
</Text>
</View>
{showLabel ? (
<View style={styles.streamContentLabelContainer}>
<IconComponent
fill={color}
width={spacing(5)}
height={spacing(5)}
/>
<Text
fontSize='small'
colorValue={color}
weight='demiBold'
textTransform='uppercase'
style={styles.streamContentLabel}
>
{title}
</Text>
</View>
) : null}
<Text
fontSize='xl'
weight='bold'
Expand Down
1 change: 1 addition & 0 deletions packages/mobile/src/harmony-native/components/Button/Button/Button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ export const Button = (props: ButtonProps) => {
<BaseButton
onPressIn={handlePressIn}
onPressOut={handlePressOut}
onLongPress={handlePressOut}
Copy link
Member Author

Choose a reason for hiding this comment

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

Ok @dylanjeffers also tagging you for this part too :)

Lmk if you have concerns -- PR description should cover it. It only improves the current UX, though we could definitely reconsider how long press works overall in our app

disabled={isDisabled}
style={[animatedButtonStyles, buttonStyles, style]}
sharedValue={pressed}
Expand Down
15 changes: 10 additions & 5 deletions packages/mobile/src/screens/track-screen/DownloadSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { useCallback, useState } from 'react'

import {
useCurrentStems,
useDownloadableContentAccess
useDownloadableContentAccess,
useGatedContentAccess
} from '@audius/common/hooks'
import { ModalSource, DownloadQuality } from '@audius/common/models'
import type { ID } from '@audius/common/models'
Expand Down Expand Up @@ -60,8 +61,15 @@ export const DownloadSection = ({ trackId }: { trackId: ID }) => {
const { onOpen: openWaitForDownloadModal } = useWaitForDownloadModal()
const [quality, setQuality] = useState(DownloadQuality.MP3)
const [isExpanded, setIsExpanded] = useState(false)
const track = useSelector((state: CommonState) =>
getTrack(state, { id: trackId })
)
const { stemTracks } = useCurrentStems({ trackId })
const shouldDisplayDownloadAll = stemTracks.length > 1
const { hasStreamAccess, hasDownloadAccess } = useGatedContentAccess(track)
// Hide the download all button if there aren't multiple downloads and if the user
// happens to not have stream access to the track
const shouldDisplayDownloadAll =
stemTracks.length > 1 && hasStreamAccess && hasDownloadAccess
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not sure if hasStreamAccess is necessary here since hasDownloadAccess will never be true while hasStreamAccess is false, but doesn't hurt.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah good callout -- I think it's helpful to be explicit, but I agree it's not possible as the backend currently allows it

Copy link
Contributor

Choose a reason for hiding this comment

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

nit but i think for simplicity purposes, we can rely on just one boolean. that is one of the goals of this design so that UI doesnt have to do a whole lot of thinking

Copy link
Member Author

Choose a reason for hiding this comment

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

ok sounds good, will make that change

const {
price,
shouldDisplayPremiumDownloadLocked,
Expand All @@ -76,9 +84,6 @@ export const DownloadSection = ({ trackId }: { trackId: ID }) => {
maximumFractionDigits: 2
})
: undefined
const track = useSelector((state: CommonState) =>
getTrack(state, { id: trackId })
)
const shouldHideDownload =
!track?.access.download && !shouldDisplayDownloadFollowGated

Expand Down
23 changes: 17 additions & 6 deletions packages/web/src/common/store/cache/tracks/utils/retrieveTracks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,14 @@ const getUserId = accountSelectors.getUserId
const { getIsInitialFetchAfterSsr } = trackPageSelectors
const { setIsInitialFetchAfterSsr } = trackPageActions

type UnlistedTrackRequest = { id: ID; url_title: string; handle: string }
type UnlistedTrackRequest = {
id: ID
// TODO: These are no longer required for unlisted track fetching
// They are only optional for a request for an unlisted track and can be
// traced through and deleted.
url_title?: string
handle?: string
}
type RetrieveTracksArgs = {
trackIds: ID[] | UnlistedTrackRequest[]
canBeUnlisted?: boolean
Expand Down Expand Up @@ -196,13 +203,17 @@ export function* retrieveTracks({
if (ids.length > 1) {
throw new Error('Can only query for single unlisted track')
} else {
const { id, url_title, handle } = ids[0]
fetched = yield* call([apiClient, 'getTrack'], {
id: ids[0].id,
id,
currentUserId,
unlistedArgs: {
urlTitle: ids[0].url_title,
handle: ids[0].handle
}
unlistedArgs:
url_title && handle
? {
urlTitle: url_title,
handle
}
: undefined
})
}
} else {
Expand Down
14 changes: 3 additions & 11 deletions packages/web/src/common/store/pages/track/sagas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,10 @@ function* watchFetchTrack() {
forceRetrieveFromSource
})
} else {
const ids =
canBeUnlisted && slug && handle
? [{ id: trackId, url_title: slug, handle }]
: [trackId]
const ids = canBeUnlisted
Copy link
Member Author

Choose a reason for hiding this comment

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

@dylanjeffers tagging you in this PR only for this part.

requiring slug & handle is not necessary for this to work, and probably hasn't been working as written for a while, but in the ts change we added the && slug && handle constraint, which aren't passed in via navigation, only when something is deep linked.

Separately, there was an additional retrieveTracks call for no reason

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for fixing, will post in client channel as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand @raymondjacobson - if they're not necessary, why the ternary at all? Isn't the point of this line that we retrieve by only the track id if we don't have slug and title?

Copy link
Member Author

Choose a reason for hiding this comment

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

The backend no longer needs them in order to serve back a hidden track. Isaac made that change a long time ago. They used to be required, but we made changes somewhere else that don't always provide them when canBeUnlisted = true

Copy link
Member Author

Choose a reason for hiding this comment

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

So the typescripting of this file totally broke loading stems on mobile across the board

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll make a ticket to rip out this code though, it's very misleading

Copy link
Contributor

Choose a reason for hiding this comment

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

That part makes sense, but I'm not sure what this code is accomplishing as it reads after your change - do we not want to retrieve by [{id, url_title, handle}] or [id] in all cases then?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah sorry github comments didn't load in

? [{ id: trackId, url_title: slug, handle }]
: [trackId]

retrieveTracks({
trackIds: ids,
canBeUnlisted,
withStems: true,
withRemixes,
withRemixParents: true
})
const tracks: Track[] = yield* call(retrieveTracks, {
trackIds: ids,
canBeUnlisted,
Expand Down
4 changes: 2 additions & 2 deletions ...ges/web/src/components/premium-content-purchase-modal/components/PurchaseSummaryTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const messages = {
youPaid: 'You Paid',
zero: '$0.00',
price: (val: string) => `$${val}`,
included: 'Included'
included: 'included'
}

type PurchaseSummaryTableProps = {
Expand Down Expand Up @@ -45,7 +45,7 @@ export const PurchaseSummaryTable = ({
value: messages.price(formatPrice(basePrice))
})
}
const downloadCount = (stemsPurchaseCount || 0) + (downloadPurchaseCount || 0)
const downloadCount = (stemsPurchaseCount ?? 0) + (downloadPurchaseCount ?? 0)
if (downloadCount > 0) {
items.push({
id: 'premiumTrackDownload',
Expand Down