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

[C-4488] Hide actions + play count for hidden content #8796

Merged
merged 2 commits into from
Jun 12, 2024
Merged

[C-4488] Hide actions + play count for hidden content #8796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50b6c09
[C-4488] Hide actions + play count for hidden content
dharit-tan Jun 11, 2024
152ee30
lint
dharit-tan Jun 12, 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/screens/collection-screen/CollectionScreenDetailsTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ export const CollectionScreenDetailsTile = ({
description={description}
descriptionLinkPressSource='collection page'
duration={collectionDuration}
hideOverflow={hideOverflow || !isReachable}
hideOverflow={hideOverflow || !isReachable || (isPrivate && !isOwner)}
hidePlayCount={true}
hasStreamAccess={hasStreamAccess}
streamConditions={streamConditions}
Expand Down
29 changes: 13 additions & 16 deletions packages/mobile/src/screens/track-screen/TrackScreenDetailsTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,10 @@ export const TrackScreenDetailsTile = ({
const {
_co_sign,
description,
field_visibility,
genre,
has_current_user_reposted,
has_current_user_saved,
is_unlisted,
is_unlisted: isUnlisted,
is_stream_gated: isStreamGated,
owner_id,
play_count,
Expand All @@ -135,12 +134,13 @@ export const TrackScreenDetailsTile = ({
ddex_app: ddexApp,
is_delete,
duration,
release_date
release_date: releaseDate,
is_scheduled_release: isScheduledRelease
} = track

const isOwner = owner_id === currentUserId
const hideFavorite = is_unlisted || !hasStreamAccess
const hideRepost = is_unlisted || !isReachable || !hasStreamAccess
const hideFavorite = isUnlisted || !hasStreamAccess
const hideRepost = isUnlisted || !isReachable || !hasStreamAccess

const remixParentTrackId = remix_of?.tracks?.[0]?.parent_track_id
const isRemix = !!remixParentTrackId
Expand Down Expand Up @@ -273,7 +273,7 @@ export const TrackScreenDetailsTile = ({
albumInfo ? OverflowAction.VIEW_ALBUM_PAGE : null,
OverflowAction.VIEW_ARTIST_PAGE,
isOwner && !ddexApp ? OverflowAction.EDIT_TRACK : null,
isOwner && track?.is_scheduled_release && track?.is_unlisted
isOwner && isScheduledRelease && isUnlisted
? OverflowAction.RELEASE_NOW
: null,
isOwner && !ddexApp ? OverflowAction.DELETE_TRACK : null
Expand Down Expand Up @@ -312,17 +312,14 @@ export const TrackScreenDetailsTile = ({
}
hideFavorite={hideFavorite}
hideRepost={hideRepost}
hideShare={(is_unlisted && !isOwner) || !field_visibility?.share}
Copy link
Contributor

Choose a reason for hiding this comment

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

Really a product question, but do we want to stop respecting existing field visibility settings, or just take away the ability to set new ones?

I def understand doing it for collections and new track uploads, but I could see someone getting annoyed if they specifically set it not to share before.

Probably small potatoes and nobody cares, but just checking to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think on the call yesterday we decided to leave it in the backend, but remove it from the front-end

hideOverflow={!isReachable}
hideFavoriteCount={is_unlisted}
hidePlayCount={
(!isOwner && is_unlisted && !field_visibility?.play_count) ||
isStreamGated
}
hideRepostCount={is_unlisted}
hideShare={isUnlisted && !isOwner}
hideOverflow={!isReachable || (isUnlisted && !isOwner)}
hideFavoriteCount={isUnlisted}
hidePlayCount={(!isOwner && isUnlisted) || isStreamGated}
hideRepostCount={isUnlisted}
isPlaying={isPlaying && isPlayingId}
isPreviewing={isPreviewing}
isUnlisted={is_unlisted}
isUnlisted={isUnlisted}
isDeleted={is_delete}
onPressEdit={handlePressEdit}
onPressFavorites={handlePressFavorites}
Expand All @@ -343,7 +340,7 @@ export const TrackScreenDetailsTile = ({
contentType={PurchaseableContentType.TRACK}
ddexApp={ddexApp}
duration={duration}
releaseDate={release_date}
releaseDate={releaseDate}
/>
)
}
55 changes: 28 additions & 27 deletions packages/web/src/components/track/GiantTrackTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,7 @@ export const GiantTrackTile = ({
}

const renderListenCount = () => {
const shouldShow =
isOwner || (!isStreamGated && (isUnlisted || fieldVisibility.play_count))
const shouldShow = isOwner || (!isStreamGated && !isUnlisted)

if (!shouldShow) {
return null
Expand Down Expand Up @@ -600,32 +599,34 @@ export const GiantTrackTile = ({
{renderStatsRow()}
</div>

<div
className={cn(styles.actionButtons, fadeIn)}
role='group'
aria-label={messages.actionGroupLabel}
>
{renderShareButton()}
{renderMakePublicButton()}
{hasStreamAccess && renderRepostButton()}
{hasStreamAccess && renderFavoriteButton()}
<span>
{/* prop types for overflow menu don't work correctly
{isUnlisted && !isOwner ? null : (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i could put this logic inside the render functions but we also want to hide the overflow menu. open to ideas.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is fine

nit: we generally do the null at the end like condition ? <Content /> : null
I see both in the codebase, but this one is about 6 times as frequent 🤷

<div
className={cn(styles.actionButtons, fadeIn)}
role='group'
aria-label={messages.actionGroupLabel}
>
{renderShareButton()}
{renderMakePublicButton()}
{hasStreamAccess && renderRepostButton()}
{hasStreamAccess && renderFavoriteButton()}
<span>
{/* prop types for overflow menu don't work correctly
so we need to cast here */}
<Menu {...(overflowMenu as any)}>
{(ref, triggerPopup) => (
<div className={cn(styles.menuKebabContainer)} ref={ref}>
<Button
variant='secondary'
aria-label='More options'
iconLeft={IconKebabHorizontal}
onClick={() => triggerPopup()}
/>
</div>
)}
</Menu>
</span>
</div>
<Menu {...(overflowMenu as any)}>
{(ref, triggerPopup) => (
<div className={cn(styles.menuKebabContainer)} ref={ref}>
<Button
variant='secondary'
aria-label='More options'
iconLeft={IconKebabHorizontal}
onClick={() => triggerPopup()}
/>
</div>
)}
</Menu>
</span>
</div>
)}
</div>
<Flex
gap='s'
Expand Down
11 changes: 5 additions & 6 deletions packages/web/src/pages/track-page/components/mobile/TrackHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ import IconCalendarMonth from '@audius/harmony/src/assets/icons/CalendarMonth.sv
import IconRobot from '@audius/harmony/src/assets/icons/Robot.svg'
import IconVisibilityHidden from '@audius/harmony/src/assets/icons/VisibilityHidden.svg'
import cn from 'classnames'
import moment from 'moment'
import dayjs from 'dayjs'
import { shallowEqual, useSelector } from 'react-redux'

import CoSign from 'components/co-sign/CoSign'
Expand Down Expand Up @@ -211,15 +211,14 @@ const TrackHeader = ({
const showPreview = isUSDCPurchaseGated && (isOwner || !hasStreamAccess)
// Play button is conditionally hidden for USDC-gated tracks when the user does not have access
const showPlay = isUSDCPurchaseGated ? hasStreamAccess : true
const showListenCount =
isOwner || (!isStreamGated && (isUnlisted || fieldVisibility.play_count))
const showListenCount = isOwner || (!isStreamGated && !isUnlisted)
const { data: albumInfo } = trpc.tracks.getAlbumBacklink.useQuery(
{ trackId },
{ enabled: !!trackId }
)
const isPlaylistAddable = useIsGatedContentPlaylistAddable(track)
const shouldShowScheduledRelease =
track?.release_date && moment(track.release_date).isAfter(moment())
track?.release_date && dayjs(track.release_date).isAfter(dayjs())

const image = useTrackCoverArt(
trackId,
Expand Down Expand Up @@ -436,8 +435,8 @@ const TrackHeader = ({
<ActionButtonRow
showRepost={showSocials}
showFavorite={showSocials}
showShare={!isUnlisted || fieldVisibility.share || isOwner}
showOverflow
showShare={!isUnlisted || isOwner}
showOverflow={!isUnlisted || isOwner}
shareToastDisabled
isOwner={isOwner}
isReposted={isReposted}
Expand Down