Skip to content
This repository was archived by the owner on Oct 4, 2023. It is now read-only.

[C-1937, C-1964] Add pagination to the mobile favorites screen and fix the title width for deleted track list items#2738

Merged
Kyle-Shanks merged 1 commit into
mainfrom
kj-Add-pagination-to-mobile-favorites-page
Jan 31, 2023
Merged

[C-1937, C-1964] Add pagination to the mobile favorites screen and fix the title width for deleted track list items#2738
Kyle-Shanks merged 1 commit into
mainfrom
kj-Add-pagination-to-mobile-favorites-page

Conversation

@Kyle-Shanks

@Kyle-Shanks Kyle-Shanks commented Jan 31, 2023

Copy link
Copy Markdown
Contributor

Description

  • Update mobile favorites page to use the sagas in common that were set up for pagination on web to allow for paginated requests
  • Add spinner at the bottom when we are fetching more so that the user know that there are more tracks in the list
  • Update the component that we have the layout change callback on to fix the deleted track text disappearing bug

Dragons

This is a sizable update to how favorites are loaded to there is some risk of introducing weird errors states

How Has This Been Tested?

Lots of manual testing

How will this change be monitored?

N/A

Feature Flags

N/A

@Kyle-Shanks Kyle-Shanks requested review from a team, amendelsohn and dylanjeffers January 31, 2023 16:00
sliptype
sliptype approved these changes Jan 31, 2023
View reviewed changes

@sliptype sliptype left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome! Glad we were able to use the existing common sagas for this 🔥

Comment thread packages/mobile/src/screens/favorites-screen/TracksTab.tsx Outdated
dylanjeffers
dylanjeffers approved these changes Jan 31, 2023
View reviewed changes

@dylanjeffers dylanjeffers left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome stuff, added some extra comments that are probably off the optical variety

Comment thread packages/mobile/src/components/track-list/TrackList.tsx Outdated
Comment thread packages/mobile/src/components/track-list/TrackList.tsx Outdated
Comment thread packages/mobile/src/components/track-list/TrackListItem.tsx
>
<View
style={styles.topLine}
onLayout={(e) => setTitleWidth(e.nativeEvent.layout.width)}

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wondering is we can avoid the layout calculation for performance?

Comment thread packages/mobile/src/screens/favorites-screen/TracksTab.tsx
])

useEffect(() => {
const allTracks = tracks.every((track) => track.kind === Kind.TRACKS)

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This does feel like it would be expensive as the list gets long? What do you think?

@Kyle-Shanks Kyle-Shanks Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

simple equality check so doesn't seem too bad. I think that it may run more often than it needs to, but would wait to see if we have perf issues before trying to fix this

Comment thread packages/mobile/src/screens/favorites-screen/TracksTab.tsx
placeholder={messages.inputPlaceholder}
onChangeText={setFilterValue}
/>
<WithLoader loading={initialFetch}>

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally would like to see skeletons render initially, would that be hard?

@Kyle-Shanks Kyle-Shanks Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

More of a product question I suppose. Probably not too difficult, but I was trying to not change things too much

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah for this go around no issues, just wanted to air me thoughts in case you felt similar

@Kyle-Shanks Kyle-Shanks Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea I like the idea. I'll add an issue in linear to update this later

Comment thread packages/mobile/src/screens/favorites-screen/TracksTab.tsx
)
) : (
<>
<OfflineContentBanner />

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can all this be part of the flat list header? Also I wonder if we can remove the virtualized scroll view then

Comment thread packages/mobile/src/screens/favorites-screen/TracksTab.tsx
/>
</Tile>
) : null}
{isFetchingMore ? (

@dylanjeffers dylanjeffers Jan 31, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be added to flat list footer?

@Kyle-Shanks Kyle-Shanks force-pushed the kj-Add-pagination-to-mobile-favorites-page branch from baf33a9 to dcc9dee Compare January 31, 2023 18:20
@Kyle-Shanks Kyle-Shanks merged commit 36687e5 into main Jan 31, 2023
@Kyle-Shanks Kyle-Shanks deleted the kj-Add-pagination-to-mobile-favorites-page branch January 31, 2023 18:20
audius-infra pushed a commit that referenced this pull request Feb 4, 2023
v1.5.9
fef9da7 
[45f122f] [C-2015] Debounce the callbacks for the download toggles for favorites and collections (#2766) Kyle Shanks
[e0cafef] [C-2034] Cancel all download jobs on log out (#2775) Andrew Mendelsohn
[ab581b1] C-2022 C-1970 Fix for: When offline and not fully downloaded, queued jobs keep running and fail (#2773) nicoback2
[1c674a9] C-1958 Show # of downloaded tracks in collection card instead of actual # of tracks in collection when offline (#2757) nicoback2
[014afaa] [C-2036] Fix modals not closing on click outside (#2776) Marcus Pasell
[07b7961] [DMs] Redo create chat modal (#2706) Marcus Pasell
[8b89acf] [C-1994] Add download indicator to the track screen (#2774) Kyle Shanks
[70619cd] Improve download toggle, download progress and fetch (#2772) Raymond Jacobson
[860291d] [C-1941] Refresh queue when download toggle is flipped (#2770) Raymond Jacobson
[3e8e33b] Fix CI (#2771) Michael Piazza
[f94bcc9] [C-2012] [C-1999] [C-1734] Fix collection image loading (#2769) Sebastian Klingler
[80da94c] Consume .env.git to properly tag Sentry release (#2767) Michael Piazza
[b2f96f6] [C-1974] Add batch remove downloads (#2764) Dylan Jeffers
[5ff30b4] [C-1921] Make sure to download local saves (#2763) Raymond Jacobson
[de5d5fd] [C-2006] Fix download toggle when offline (#2762) Raymond Jacobson
[66449a1] [PAY-868] Handle follow and tip send states across (#2756) Saliou Diallo
[171e46e] [PAY-866] Add new remix settings - Part 1 (#2708) Saliou Diallo
[90579c5] Refactor lineup hooks (#2760) Sebastian Klingler
[5bfa0dc] C-2002 Un-download deleted playlists on sync (#2759) nicoback2
[1dd8b68] [C-2001] Remove queued downloads if deleted/unlisted/etc (#2761) Raymond Jacobson
[265e666] [C-1968] Fix dequeue track (#2758) Dylan Jeffers
[fbaf5c0] [C-1992] Fix unfavorite downloaded track (#2755) Dylan Jeffers
[73563f8] [C-1957] [C-1929] Improve TrackList and offline mode performance (#2753) Sebastian Klingler
[b771776] [C-1987][C-1936][C-2009] Queue for collection downloads (#2751) Andrew Mendelsohn
[016342d] [C-1985] Remove collection tracks when removing favorites (#2749) Dylan Jeffers
[5b675c8] [PAY-881] Mobile messages (chats) list screen (#2742) Reed
[d2b1c4c] C-1989 Fix "Download is not re-starting even though there are still many tracks that need to be downloaded" (#2746) nicoback2
[fec0134] [PAY-872][PAY-873][PAY-874] More premium content QA fixes (#2741) Saliou Diallo
[9530cae] Hide buttons on profiles when offline and remove timeout (#2750) Raymond Jacobson
[d658e3e] [C-1952] Show download successful when offline in status indicator (#2733) Raymond Jacobson
[bbb58e8] [C-1950] Smooth offline<>online playback (#2748) Raymond Jacobson
[059f79e] Hide share/repost when offline C-1942 (#2747) nicoback2
[6458512] [C-2000] Fix collections not downloading on favorites download (#2745) Dylan Jeffers
[4ba882c] Do not write subscribe to discovery on auto-subscribe after follow (#2562) Michelle Brier
[e21edc7] Fix download counter in error state (#2744) Raymond Jacobson
[3ad7a55] [PAY-777] Add premium content track upload on mobile - Part 3 (#2721) Saliou Diallo
[ad3b6d2] Update undownload favorites to hide progress earlier (#2743) Kyle Shanks
[16a45f5] [C-1938] Remove tracks within collections when toggling off favorites (#2737) Raymond Jacobson
[5e008a3] [C-1977] Hide more by this artist when offline (#2734) Raymond Jacobson
[b1d261f] Improve useFetchAllFavoritedTracks fetch (#2740) Dylan Jeffers
[a39f874] [C-1993 C-1995] Refactor download switch, improve collection header (#2739) Dylan Jeffers
[36687e5] [C-1937, C-1964] Add pagination to the mobile favorites screen and fix the title width for deleted track list items (#2738) Kyle Shanks
[adc74fa] [C-1976] Hide/disable overflow menus when offline (#2735) Raymond Jacobson
[18e63ff] [C-1938] Disable downloading from toggle when offline (#2736) Raymond Jacobson
[a0d3097] [C-1991] Fix downloading rive animation location (#2732) Raymond Jacobson
[1e8d9d5] [C-1960] Do not show favorite/repost on own (#2723) Raymond Jacobson
[563842e] [C-1930] Prevent self favorite/repost/follow (#2724) Raymond Jacobson
[4975945] [C-1916 C-1982 QA-328] Fix smart-collection artwork (#2731) Dylan Jeffers
[bbc2477] [C-1963] Add download progress bar (#2730) Raymond Jacobson
[5ef3d06] [C-1782] Use proxy for sentry dsn (#2684) Raymond Jacobson
[2c2cd82] [C-1962] Update offline icons/animations (#2725) Raymond Jacobson
[8607f9c] Fix crash on favorites screen if you start the app offline C-1943 C-1951 (#2726) nicoback2
[74c9324] [C-1986] Use RNFetchBlob for offline mode (#2728) Sebastian Klingler
[3084ea9] Refactor ArtistChip (#2715) Marcus Pasell
[7a9cec6] [C-1973] Improve offline image downloading (#2720) Sebastian Klingler
[075aa5b] Fix some web premium content bugs (#2709) Saliou Diallo
@AudiusProject AudiusProject deleted a comment from linear Bot Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@amendelsohn amendelsohn Awaiting requested review from amendelsohn
2 more reviewers
@dylanjeffers dylanjeffers dylanjeffers approved these changes
@sliptype sliptype sliptype approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kyle-Shanks @dylanjeffers @sliptype