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.

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

Show Connected via Pusher status #261

Merged
merged 15 commits into from
Aug 25, 2020
Merged

Show Connected via Pusher status #261

merged 15 commits into from
Aug 25, 2020

Conversation

marcaaron
Copy link
Contributor

Fixes #252

This PR colorizes the dot next to a user's name when they have an "active connection"

Previously we set the user to "offline" when Pusher disconnects or when they successfully make an API request. This adds another layer to set them online and offline when they lose connectivity via the NetInfo package.

Note: Disconnecting from Pusher via a bad internet connection or turning off Wi-Fi takes about 10-15 seconds. So we can't really "trust" that we are truly connected to Pusher or not since we could have lost connectivity via Wi-Fi well before Pusher recognizes that we have been disconnected.

To get around this I spent some time focusing on getting the NetInfo native module to work.

At first, I could not get it working in RN4W even though it's supposed to work so I dug around in the source code for the package and realized that when a react native library supports multiple platforms the react-native-web source files will have a web.js extension. Added that to the webpack config and it began to work on web.

cc @AndrewGable

Another Note: There's a bug on iOS Simulator that prevents NetInfo from working correctly so test with a real iOS device and throw it into Airplane mode to see what happens.

@marcaaron marcaaron self-assigned this Aug 20, 2020
@Jag96
Copy link
Contributor

Jag96 commented Aug 21, 2020

I'm getting the following error when trying to run this, but it isn't obvious to me what is causing it. Did you run into this while getting NetInfo working?

react-dom.development.js:88 Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of `WithIon(HeaderView)`.
    in HeaderView (created by WithIon(HeaderView))
    in WithIon(HeaderView) (created by Context.Consumer)
    in withRouter(WithIon(HeaderView)) (created by Context.Consumer)
    in div (created by View)
    in View (created by Context.Consumer)
    in Route (created by Context.Consumer)
    in div (created by View)
    in View (created by Context.Consumer)
    in div (created by View)
    in View (created by NativeSafeAreaView)
    in NativeSafeAreaView (created by SafeAreaProvider)
    in SafeAreaProvider (created by App)
    in App (created by Context.Consumer)
    in Route (created by Expensify)
    in Switch (created by Expensify)
    in Router (created by HashRouter)
    in HashRouter (created by Expensify)
    in Expensify (created by WithIon(Expensify))
    in WithIon(Expensify)
    in Unknown
    in div (created by View)
    in View (created by AppContainer)
    in div (created by View)
    in View (created by AppContainer)
    in AppContainer

Also adding @tgolen as a reviewer since I'll be OOO til the 31st.

@Jag96 Jag96 requested a review from tgolen August 21, 2020 23:51
@marcaaron
Copy link
Contributor Author

Pretty sure this just needs master merged I will take a quick look

tgolen
tgolen previously approved these changes Aug 24, 2020
Copy link
Contributor

@tgolen tgolen left a comment

Choose a reason for hiding this comment

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

Need to fix some conflicts

@marcaaron
Copy link
Contributor Author

Conflicts are fixed now on this one.

@quinthar
Copy link
Contributor

Real excited for this one...

@marcaaron marcaaron requested a review from tgolen August 25, 2020 00:11
@tgolen tgolen merged commit 84c2f7f into master Aug 25, 2020
@tgolen tgolen deleted the marcaaron-showOnlineStatus branch August 25, 2020 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show online status indicator and reliably reconnect after going offline
4 participants