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

Native-fy search results tracks lineup #1883

Merged
merged 1 commit into from
Sep 9, 2022
Merged

Native-fy search results tracks lineup #1883

merged 1 commit into from
Sep 9, 2022

Conversation

nicoback2
Copy link
Contributor

Description

See title!
Screen Shot 2022-09-08 at 2 30 38 PM

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/nkang--rn-search

dylanjeffers
dylanjeffers approved these changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@dylanjeffers dylanjeffers left a comment

Choose a reason for hiding this comment

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

looks great, tricky stuff nice job

packages/web/src/pages/search-page/components/desktop/SearchPageContent.js
@@ -415,7 +421,11 @@ class SearchPageContent extends Component {
}
loadMore={(offset, limit) =>
this.props.dispatch(
tracksActions.fetchLineupMetadatas(offset, limit)
tracksActions.fetchLineupMetadatas(offset, limit, false, {
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting! was this stuff all broken even on web? or maybe looks like it was computed in sagas but not native compat?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah the latter! previously it was computed using the window.location

raymondjacobson
raymondjacobson approved these changes Sep 9, 2022
View reviewed changes
packages/web/src/common/store/lineup/sagas.js
@@ -39,7 +39,7 @@ const IS_NATIVE_MOBILE = process.env.REACT_APP_NATIVE_MOBILE
// This is copied from web/src/utils since moving it to common would require
// changing 90+ files. This will be ignored in RN-reloaded, since we check
// isNativeMobile from storeContext first.
const isMobile = () => {
export const isMobile = () => {
Copy link
Member

Choose a reason for hiding this comment

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

does IS_NATIVE_MOBILE have any meaning in common since we pull process env? I think we should just remove that part of this check entirely since this util really just checks user agent stuff

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 we put that there because this function is only meant to be run in web - in the two cases that we use it (this file and the search results track lineup sagas), we check isNativeMobile from storeContext first -- so effectively this does not run if there is no process.env.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @dylanjeffers

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah its a good callout! imo we are gonna do an IS_NATIVE_MOBILE round soon so can probably get it there, cause like @nicoback2 is saying it's not gonna get hit anyway. fwiw there are lot of cases like this littered around the app still.

@dylanjeffers
Copy link
Contributor

@nicoback2 sorry about the merge conflicts, i think the move might be to rebase-interactive all your changes into one commit, then cherry-pick it onto latest react-native, then just replace your branch with that?

@audius-infra
Copy link
Collaborator

Preview this change https://demo.audius.co/nkang--rn-search

@nicoback2 nicoback2 merged commit 036a37a into native-reloaded Sep 9, 2022
@nicoback2 nicoback2 deleted the nkang--rn-search branch September 9, 2022 20:01
sliptype pushed a commit that referenced this pull request Sep 9, 2022
@nicoback2 @nicoback @sliptype
nativefy search results lineup (#1883)
26ba05f 
Co-authored-by: Nikki Kang <kangaroo233@gmail.com>
audius-infra pushed a commit that referenced this pull request Sep 10, 2022
v1.3.4
1cca57c 
[f91506e] [C-1006] Improve initial load, hotfix missing web worker (#1896) Dylan Jeffers
[edc8433] @audius/sdk@1.0.4 (#1892) Sebastian Klingler
[bd9e726] [C-1003] Native Account Sagas (#1890) Sebastian Klingler
[a0a202b] [C-958] Add native tip reaction (#1888) Dylan Jeffers
[26ba05f] nativefy search results lineup (#1883) nicoback2
[c764096] [C-995] Remove message passing interfaces (#1886) Dylan Jeffers
[98a563d] Fix trending-screen react warnings Dylan Jeffers
[8f269fe] Fix merge conflicts Dylan Jeffers
[257cf96] [C-991] Add native edit/delete playlists Dylan Jeffers
[94c1343] [C-991] Add native create/edit playlists (#1882) Dylan Jeffers
[35ff23c] [C-989] Add common track-download (#1880) Dylan Jeffers
[6530fcb] [C-988] Fix social actions by fixing metro imports (#1875) Dylan Jeffers
[3d8dbf9] [C-969] Add native audio-screen (#1873) Dylan Jeffers
[3583795] Revert "Fix social features and other sig-util features (#1860)" Dylan Jeffers
[8fe2291] Fix social features and other sig-util features (#1860) Dylan Jeffers
[f93ce60] [C-967] Add native deactivate-account sagas (#1859) Dylan Jeffers
[a793591] [C-965] Add native reset-password (#1858) Dylan Jeffers
[54229a1] [C-962, C-904] Native recovery email (#1856) Dylan Jeffers
[fc9410d] [C-953] Migrate profile supporters/supporting (#1855) Sebastian Klingler
[27d5e00] [C-960] Add native push/email settings (#1854) Dylan Jeffers
[6b7f1b0] [C-937] Refactor theme state, Implement native theme (#1851) Dylan Jeffers
[fe149be] [C-955] Fully native settings screen (#1848) Dylan Jeffers
[f60e001] Split up sign-out saga to web and native (#1847) Dylan Jeffers
[24fc9b7] Add vipDiscordModal sagas Dylan Jeffers
[09892f9] [C-927] Move sign up to native (#1820) Raymond Jacobson
[eab6785] Migrate collectible details (#1839) Sebastian Klingler
[0b79b90] [C-916] Native Profile Screen (#1834) Sebastian Klingler
[f3eba19] [C-936] Remove WebView from native (#1831) sliptype
[b80f930] [C-934] Add fully native user lists (#1830) sliptype
[ec23451] [C-933] Add fully native social actions (#1825) sliptype
[d49fa7c] Oops (#1828) Sebastian Klingler
[5ae32b2] [C-929] Add fully native drawers (#1824) Dylan Jeffers
[577ebf3] [C-931] Fully native favorites screen (#1823) Dylan Jeffers
[7e143dd] [C-924] Add fully native explore screen (#1819) sliptype
[69f7fe5] [C-917] Add native trending screen (#1816) Dylan Jeffers
[5699349] [C-778] Native notifications (#1810) sliptype
[5d8be85] [C-872] Play audio in native (#1807) sliptype
[3e8f9a5] [React Native Reloaded] Make mobile search fully native (#1808) sliptype
[44046ce] [C-903] Migrate feed to native store (#1799) Dylan Jeffers
[eb3a714] [C-902] Move collection page to native (#1798) sliptype
[ec25739] Search results screen (#1782) sliptype
[b3394ca] [C-858] Add track page sagas to native, Refactor TrackScreen (#1768) Dylan Jeffers
[17f65ef] [C-974] Migrate account sagas (#1884) Sebastian Klingler
[40bda4c] Fix optimizely for local dev (#1874) Michael Piazza
[a3e62e8] Use bulk tracks from API client (#1849) Raymond Jacobson
[643cbc5] Remove duplicate create user bank on sign up (#1868) Raymond Jacobson
[7ec8f17] [C-868] Remove IPFS from client build steps (#1876) Raymond Jacobson
[9ac5410] [C-878] Show invalid file errors on upload (#1877) Raymond Jacobson
[369660f] Fixes profile pictures in top supporters tile (#1879) Reed
[95c1392] Update feed tip tile per QA feedback (#1872) Reed
[3f32abc] [PAY-600][PAY-599][PAY-607][PAY-617] RemoteConfig for Buy Audio and better transactiondetails link (#1861) Marcus Pasell
[35393b5] Fix profile left nav tile spacing and order (#1871) Saliou Diallo
[0297699] Revert "Fix optimizely local dev (#1261)" (#1870) Michael Piazza
[67b97dd] Clean up remote-config init handlers (#1866) Sebastian Klingler
[0806eed] [C-914] Fix windows build (#1867) Raymond Jacobson
[965f9c0] [C-879] Fix rewards page navigation (#1865) Dylan Jeffers
[9361232] Fix send $AUDIO flow after wallet saga migration (#1864) Reed
[670e142] fix dark mode follows you (#1863) nicoback2
[3c2e178] Fix optimizely local dev (#1261) Michael Piazza
[f133c0b] Adjust badge sizes and alignment (#1862) Sebastian Klingler
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dylanjeffers dylanjeffers dylanjeffers approved these changes

@raymondjacobson raymondjacobson raymondjacobson approved these changes

@sliptype sliptype sliptype approved these changes

Assignees

@raymondjacobson raymondjacobson

@sliptype sliptype

Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nicoback2 @audius-infra @dylanjeffers @raymondjacobson @sliptype @nicoback