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

When offline, show which messages have been queued but not yet sent; when back online, show when they've been successfully delivered #2432

Closed
laurenreidexpensify opened this issue Apr 16, 2021 · 17 comments · Fixed by #2478 or #2526
Assignees
Labels
Daily KSv2

Comments

@laurenreidexpensify
Copy link
Contributor

laurenreidexpensify commented Apr 16, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Expected Result:

When offline, know that a message hasn't been sent and, upon going back online, know that it has been sent.

Actual Result:

You don't know if the message has been or when it does.

Action Performed:

  • Go offline on e.cash by switching off your internet connection to your device
  • Send a message to someone
  • Observe that you can't tell if the message hasn't been sent
  • Once going back online, observe you can't tell when a message has successfully sent

Deliverable:

Using the design mock up below, to implement a "You appear to be offline" prompt, with unsent messages in grey font -

image

E/E issue - https://github.com/Expensify/Expensify/issues/155259
Upwork Posting - https://www.upwork.com/jobs/~01d9a1ee89310f7704

@MelvinBot
Copy link

Triggered auto assignment to @Luke9389 (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@parasharrajat
Copy link
Member

parasharrajat commented Apr 16, 2021

Proposal

  1. We will listen to ONYXKEYS.NETWORK onyx key in ReportActionCompose.js.
 withOnyx({
        network: {
            key: ONYXKEYS.NETWORK,
        },
    }),

It will give us the offline status of the user.

  1. we update the UI to reflect the You appear to be offline status after ReportTypingIndicator based on the above key.
    https://github.com/Expensify/Expensify.cash/blob/24d3803268b179711cc48838727271f971f95163/src/pages/home/report/ReportActionCompose.js#L418

  2. To show unsent messages as per the Mock up, we add styles in ReportActionItemMessage.

    • When action is in loading state (action.loading)
    • if action has no ClientID, we need this check to differentiate the unsent messages and attachments as attachment messages are also loading(action.loading ==true) until they are uploaded to the server.

    I prefer adding opacity to show unsent state as it will cover all type of comments.

Questions

  1. Do we need to update the styles for all type of comments on a report including attachments, when they are in unsent state ?

@Luke9389
Copy link
Contributor

Hi @parasharrajat, thanks for the proposal! I'm going to look over some areas of code and come back with any clarifying questions I have about this implementation.

To answer your question, my gut response would be yes, let's apply this style to any and all unsent comments. However, that would require another style mockup for what to do with unsent attachments (how would we visually represent that it hasn't been sent?) It's possible that this first pass at the problem should just be for text-based comments.

I'll allow some time for @laurenreidexpensify and @shawnborton to weigh in with their thoughts.

@Luke9389
Copy link
Contributor

Hey, after a preliminary pass through the files you've mentioned, I'm thinking this proposal is looking great!

One thing that sticks out to me is that it'd be nice for us to avoid adding the withOnyxKeys HOC to ReportActionItemMessage. I took a look around and it seems that the nearest component in the tree that already has withOnyxKeys is ReportActionsView. What are your thoughts @parasharrajat, do you think we should thread this prop down to avoid adding the HOC?

I also have a clarifying question. From my understanding, once the online status changes, any component that subscribes to ONYXKEYS.NETWORK would re-render. For example, if a user if offline, and then suddenly gets internet connection back, then we would expect a re-render, and the unsent styles to clear, correct?

@parasharrajat
Copy link
Member

parasharrajat commented Apr 16, 2021

@Luke9389
So when the messages are sent they are updated in the Onyx with loading state and as a optimistic Action, when they are updated to the server we update the Onyx and remove the optimistic action save the backend data of the message. Thus the list of messages will update and action will re-render. So unsent state of messages will go away.

So I will apply the withOnyx HOC to ReportActionCompose which the input box for typing messages. There is no need to apply the HOC to ReportActionItemMessage (which is used to render the messages). ReportActionCompose is rendered once per report. When Internet comes back, You appear to be offline status will be removed.

I hope it answers your concerns.

@parasharrajat
Copy link
Member

Question:

  1. When do you want to show the unsent style?
  • for any message until it is sent.
  • Only for new messages when we are offline.

@Luke9389
Copy link
Contributor

Ok so just to clarify, you're saying that we don't need to add withOnyxKeys to ReportActionItemMessage because it'll get updated anyway. In other words the unsent style will clear itself without needing the HOC. Correct?

When do you want to show the unsent style?

I think it'd be best for us to show the unsent style for any message until it is sent. It just makes more sense from the user's perspective.

@parasharrajat
Copy link
Member

Yeah. unsent style will clear itself without needing the HOC.

I think it'd be best for us to show the unsent style for any message until it is sent. It just makes more sense from the user's perspective.

Ok

@Luke9389
Copy link
Contributor

Cool cool! Well let's give some time for @shawnborton to chime in on whether we should use opacity for unsent images. Thanks for all these prompt responses and clear answers @parasharrajat 🙇

@parasharrajat
Copy link
Member

parasharrajat commented Apr 19, 2021

@laurenreidexpensify am I good to go here?

@shawnborton
Copy link
Contributor

Using the reduced opacity for unsent images makes sense to me 👍

@Luke9389
Copy link
Contributor

@parasharrajat you've got my go-ahead to start the PR. 👍

@parasharrajat
Copy link
Member

@shawnborton Can I have the svg icon for this?
image

@shawnborton
Copy link
Contributor

Here you go: offline.svg.zip

@laurenreidexpensify
Copy link
Contributor Author

@parasharrajat offer sent in UpWork!

@laurenreidexpensify
Copy link
Contributor Author

Wow @parasharrajat speedy!!!

@marcaaron
Copy link
Contributor

Just linking back this conversation here:

https://expensify.slack.com/archives/C01GTK53T8Q/p1619048027494500

Seems like we've got the online behavior incorrect here as there was some feedback that if we are online we should always assume the messages will succeed and not grey them out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2
Projects
None yet
6 participants