Detach mobile reachability state from non existent webview C-938 by nicoback2 · Pull Request #1913 · AudiusProject/audius-client

nicoback2 · 2022-09-13T17:15:29Z

Description

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.

dylanjeffers

This all makes sense. a few qs
So with this connectivity util, do we want to make that a "service" kinda like the other stuff, and have a polling daemon type thing in mobile sagas like we do with web sagas? might be overkill just wondering if we can use a similar pattern for both?

Ray was saying something like some phones have the reachability thing built in, but others we need to make a request to one of our services to check. Does that still apply?

dylanjeffers · 2022-09-13T17:16:48Z

 export const Connectivity: { netInfo: NetInfoState | null } = { netInfo: null }
 NetInfo.addEventListener((state: NetInfoState) => {
  Connectivity.netInfo = state
+  const newValue = checkConnectivity(state)


ah very cool

dylanjeffers · 2022-09-13T17:17:58Z

  getSearchResults,
  getTagSearchResults
 } from 'common/store/pages/search-page/sagas'
+import { isMobileWeb } from 'common/utils/isMobile'


i like the rename a lot. should we also change the isMobile file name?

nicoback2 · 2022-09-13T17:59:32Z

@dylanjeffers

So with this connectivity util, do we want to make that a "service" kinda like the other stuff, and have a polling daemon type thing in mobile sagas like we do with web sagas? might be overkill just wondering if we can use a similar pattern for both?

Yeah we can definitely do that. So like have a reachability mobile saga and just register the listener there instead of doing it in a side effect?

Ray was saying something like some phones have the reachability thing built in, but others we need to make a request to one of our services to check. Does that still apply?

I think the netinfo package esp. the latest version is pretty comprehensive in terms of covering all devices + it just pings a URL for devices that don't supply internet connectivity natively https://www.npmjs.com/package/@react-native-community/netinfo

dylanjeffers · 2022-09-13T19:28:45Z

@dylanjeffers

So with this connectivity util, do we want to make that a "service" kinda like the other stuff, and have a polling daemon type thing in mobile sagas like we do with web sagas? might be overkill just wondering if we can use a similar pattern for both?

Yeah we can definitely do that. So like have a reachability mobile saga and just register the listener there instead of doing it in a side effect?

Ray was saying something like some phones have the reachability thing built in, but others we need to make a request to one of our services to check. Does that still apply?

I think the netinfo package esp. the latest version is pretty comprehensive in terms of covering all devices + it just pings a URL for devices that don't supply internet connectivity natively https://www.npmjs.com/package/@react-native-community/netinfo

Okay awesome with the NetInfo package. Regarding the saga idea, def up to you on whether it's worth the effort rn vs later or at all :)

audius-infra · 2022-09-13T19:37:36Z

Preview this change https://demo.audius.co/nkang--reloaded-reachability

nicoback2 · 2022-09-14T15:45:05Z

@dylanjeffers

So with this connectivity util, do we want to make that a "service" kinda like the other stuff, and have a polling daemon type thing in mobile sagas like we do with web sagas? might be overkill just wondering if we can use a similar pattern for both?

Yeah we can definitely do that. So like have a reachability mobile saga and just register the listener there instead of doing it in a side effect?

Ray was saying something like some phones have the reachability thing built in, but others we need to make a request to one of our services to check. Does that still apply?

I think the netinfo package esp. the latest version is pretty comprehensive in terms of covering all devices + it just pings a URL for devices that don't supply internet connectivity natively https://www.npmjs.com/package/@react-native-community/netinfo

Okay awesome with the NetInfo package. Regarding the saga idea, def up to you on whether it's worth the effort rn vs later or at all :)

ok sgg. i started to play around with moving the listener into a saga but then realized that that might be complicating things unnecessarily. so i think we can roll with the current pattern and see if it ever becomes an issue (/Andrew might be reorganizing things anyway throughout the Offline Mode project)

audius-infra · 2022-09-14T16:54:41Z

Preview this change https://demo.audius.co/nkang--reloaded-reachability

audius-infra · 2022-09-14T18:26:56Z

Preview this change https://demo.audius.co/nkang--reloaded-reachability

[3394a06] Fix infinite lineup fetch (#1940) Sebastian Klingler [19381a3] [C-296] Persist stack history across bottom tabs (#1939) Dylan Jeffers [6705cb5] [C-1118] Send tip -> send audio (#1937) nicoback2 [9c63ab7] [C-1023] [C-1039] Sync account entropy from webview and improve splashscreen (#1938) Sebastian Klingler [e3d9cc4] [C-1064] Show "You're Offline" component on mobile (#1936) Andrew Mendelsohn [16a8b96] [C-1113] Improve native render performance (#1935) Dylan Jeffers [7f4ede3] Fix broken signup on web (#1934) Michael Piazza [2373ae0] fix rewards claim (#1933) nicoback2 [54dcd64] [C-1107] Fix push to sign in when identity is down (#1931) Raymond Jacobson [43222ee] [C-1091] Improve selector performance (#1932) Dylan Jeffers [eee12c7] Add request pagination for favorites for the new table (#1850) Kyle Shanks [5d43c12] upgrade sdk (#1929) nicoback2 [2fd28e4] [Reloaded] No more useDispatchWeb, messages C-1089 (#1927) nicoback2 [6fc8709] Merge pull request #1928 from AudiusProject/jn-exporter-default Johannes Naylor [24d9d3b] [C-838] Fix desktop update banner flow (#1719) Raymond Jacobson [aace104] remove console exporter and change default to undefined Johannes Naylor [81e9a4d] Detach mobile reachability state from non existent webview C-938 (#1913) nicoback2 [9c6c067] Merge pull request #1878 from AudiusProject/jn-con-385 Johannes Naylor [5f335bb] Update mobile readme install instructions (#1926) Dylan Jeffers [8469883] [C-907] Fix react native push notifications (#1922) Raymond Jacobson [543b013] Fix pull to refresh (#1925) Sebastian Klingler [370088e] Fix issue with using `accountUser` handle (#1924) Sebastian Klingler [5a496fa] fix more by artist lineup (#1921) nicoback2 [cab5181] downgrade node Johannes Naylor [5602eab] pin deps Johannes Naylor [7a95b3f] Fix infinite 404 (#1920) Sebastian Klingler [8cdc9b9] [C-1060] Fix /check page (#1919) Raymond Jacobson [375c5dd] [PAY-578] Fade in socials on mobile user screen (#1905) Reed [2b1cc7f] Wait for libs before signing data (#1918) Sebastian Klingler [d06e2c1] update env var Johannes Naylor [5efd985] [C-1031] Improve profile expand section touch target (#1911) Dylan Jeffers [f327636] [C-1053] Fix ios status color in dark/light modes (#1917) Dylan Jeffers [7a167b6] [C-1050] Fix profile search crash (#1916) Dylan Jeffers [32b8c82] [C-1057] Fix blank edit-profile screen (#1915) Dylan Jeffers [406e6c0] no more useselector web (#1914) nicoback2 [98ed3f6] [C-1029] Fix pull-to-refresh (#1912) Dylan Jeffers [7582351] Migrate useGetFirstOrTopSupporter (#1910) Sebastian Klingler [b9e4e41] add env vars Johannes Naylor [44060c1] remove telemetry Johannes Naylor [53483ef] remove telemetry Johannes Naylor [ae57626] remove telemetry Johannes Naylor [0515567] more telemetry Johannes Naylor [2da8c47] install Johannes Naylor [379af75] Merge branch 'main' into jn-con-385 merge main Johannes Naylor [d0deea6] changing things around Johannes Naylor [22288b1] [C-1012] Remove web build from mobile build (#1907) Raymond Jacobson [8572ee3] [C-1027] Improve bottom-bar perf (#1909) Dylan Jeffers [a41d536] the goods Johannes Naylor [128aa9b] move the instrumentation to the audiusBackend Johannes Naylor [a27c976] Fix profile clearing (#1908) Dylan Jeffers [75e6eec] Remove react-nil and NATIVE_NAVIGATION (#1906) Dylan Jeffers [ebbb561] Update social features to use entity manager (#1853) Isaac Solo [99952d1] [C-990] Nativefy oauth (#1885) Raymond Jacobson [c149204] Fix failing CI init (#1904) Sebastian Klingler [d436adb] [C-1009] Fix misaligned social icons (#1901) Dylan Jeffers [d21d5d4] [C-1020] Remove web pushes (#1902) Dylan Jeffers [600f673] [C-711] Change useSize to useMeasure in ProfileBio (#1903) Reed [497da98] stuff Johannes Naylor [7f4a144] try a bunch of things Johannes Naylor [66995fa] Clean up some more instances of useSelectorWeb/useDispatchWeb (#1900) Sebastian Klingler [3383135] Nativefy remixes screen (#1887) nicoback2 [8b0d1e5] Fix edit profile screen (#1899) Sebastian Klingler [fb3c189] [C-996] Remove NATIVE_MOBILE references (#1895) Dylan Jeffers [bb0a03e] Fix lineup state persisting across profile screens sliptype [5150a58] [PAY-626] Change playlist+tip challenges copy and emoji (#1897) Reed [e816c64] [PAY-625] Playlist challenge optimistic claiming (#1891) Reed [f3c02b6] [C-892] Fix multiple menus open at once (#1889) Andrew Mendelsohn [192c669] run setupTracing Johannes Naylor [00f3951] add tracer Johannes Naylor

nicoback2 requested review from dylanjeffers and sliptype September 13, 2022 17:15

nicoback2 assigned amendelsohn and raymondjacobson Sep 13, 2022

pull-request-size Bot added the size/L label Sep 13, 2022

nicoback2 changed the title ~~Detach mobile reachability state from non existent webview~~ Detach mobile reachability state from non existent webview C-938 Sep 13, 2022

dylanjeffers approved these changes Sep 13, 2022

View reviewed changes

nicoback2 force-pushed the nkang--reloaded-reachability branch from f6b2553 to cf86dab Compare September 13, 2022 19:10

raymondjacobson approved these changes Sep 13, 2022

View reviewed changes

nicoback added 6 commits September 14, 2022 13:49

reachability react native reloaded

f4fec94

upgrade netinfo

f3642f5

npm i

11998d5

pr review 1

1d642ac

cleanup

12a1f9c

npm i

caf4acf

nicoback2 force-pushed the nkang--reloaded-reachability branch from 127ba31 to caf4acf Compare September 14, 2022 17:59

nicoback2 merged commit 81e9a4d into main Sep 14, 2022

nicoback2 deleted the nkang--reloaded-reachability branch September 14, 2022 19:07

AudiusProject deleted a comment from linear Bot Sep 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Detach mobile reachability state from non existent webview C-938#1913

Detach mobile reachability state from non existent webview C-938#1913
nicoback2 merged 6 commits into
mainfrom
nkang--reloaded-reachability

nicoback2 commented Sep 13, 2022 •

edited

Loading

Uh oh!

dylanjeffers left a comment

Uh oh!

dylanjeffers Sep 13, 2022

Uh oh!

dylanjeffers Sep 13, 2022

Uh oh!

nicoback2 Sep 13, 2022

Uh oh!

nicoback2 commented Sep 13, 2022

Uh oh!

dylanjeffers commented Sep 13, 2022

Uh oh!

audius-infra commented Sep 13, 2022

Uh oh!

nicoback2 commented Sep 14, 2022

Uh oh!

audius-infra commented Sep 14, 2022

Uh oh!

audius-infra commented Sep 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

nicoback2 commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Dragons

How Has This Been Tested?

How will this change be monitored?

Feature Flags

Uh oh!

dylanjeffers left a comment

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

nicoback2 Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

nicoback2 commented Sep 13, 2022

Uh oh!

dylanjeffers commented Sep 13, 2022

Uh oh!

audius-infra commented Sep 13, 2022

Uh oh!

nicoback2 commented Sep 14, 2022

Uh oh!

audius-infra commented Sep 14, 2022

Uh oh!

audius-infra commented Sep 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nicoback2 commented Sep 13, 2022 •

edited

Loading