Show static skeleton UI if no reportAction exists for chat report in ONYX when the client is offline #13233

techievivek · 2022-12-01T13:24:16Z

Details

Opening a chat that was not opened recently while being offline seems to take forever to load and just displays the animating skeleton UI. I have updated the code to now show a non-animating skeleton UI along with the compose box so optimistic action can be taken while the user is offline. I have also added some general fixes to not load the report if we are offline which was needed as part of this code change.

Fixed Issues

$ #13088
PROPOSAL: GH_LINK_ISSUE(COMMENT)

Tests

Login to newDot with an account that contains several chats with other users.
Now go to settings -> preferences and toggle the offline mode using Force offline switch.
Once you are offline, navigate to a chat report that was not open recently.
Make sure you see a non-animating skeleton UI and compose box to add new messages to the chat report.
Send a few messages and go back to online mode.
Once you are online it will start showing the skeleton UI in an animating state and chat history will get loaded.

Verify that no errors appear in the JS console

Offline tests

If you are offline and you opened a chat that was not opened recently you are expected to see a non-animating skeleton UI along with compose box input that will allow you to add new messages even though the chat history doesn't exist in the ONYX.
Once you come online you should an animating skeleton UI and the chat history along with the newly added messages will get loaded.

QA Steps

Same as the testing step above.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2022-12-20.at.5.31.20.PM.mov

Mobile Web - Chrome

Screen.Recording.2022-12-20.at.6.22.47.PM.mov

Mobile Web - Safari

Screen.Recording.2022-12-20.at.5.50.55.PM.mov

Desktop

Screen.Recording.2022-12-20.at.5.33.39.PM.mov

iOS

Screen.Recording.2022-12-20.at.5.48.45.PM.mov

Android

Screen.Recording.2022-12-20.at.6.19.37.PM.mov

…t report

techievivek · 2022-12-01T13:33:09Z

This is a DRAFT PR for now because I wanted to discuss something here.
So there are two ways to tackle this PR.

Since report actions are empty and we are offline I can falsely include a report action with the action CREATED which will get passed here which calls ReportActionsList here and it will automatically render the ReportActionItemCreated component. You can look through the flow to understand it better.
Second way is what I have done in this PR, which is to return early in ReportActionsView with ReportActionItemCreated component. This does seem to work but it gets messed up (styling) in the Native Android and iOS apps which I can look into but the button doesn't seem to response so I am thinking I am missing something here.

Looking for some suggestions as I am touching the App code after many months.

melvin-bot · 2022-12-01T13:34:00Z

@eVoloshchak @marcaaron One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

…if isChatReportEmptyWhileOffline is true

marcaaron

I can falsely include a report action with the action CREATED

I think by "falsely include" you mean that we will add a fake "created" action into the reportActions? I am not sure if it's a good idea to add it to the data in Onyx - I think not as it will soon be migrated to an object keyed by reportActionID and would be very confusing to have to remove it somehow later when we are "online".

Though we could possibly develop a solution around modifying the final array that gets passed here:

App/src/pages/home/report/ReportActionsList.js

Line 151 in 3342c39

data={this.props.sortedReportActions}

I think this would be a better way to do it as then we would not need to explicitly return early with the created action. Though - this could have other unintended side effects...

return early in ReportActionsView with ReportActionItemCreated component. This does seem to work but it gets messed up (styling) in the Native Android and iOS apps which I can look into but the button doesn't seem to response so I am thinking I am missing something here.

We have to look closer at the ReportActionsList styles to understand why this doesn't look good and isn't working.

This is a good start - but I think we should bring this one to Slack to get more ideas.

marcaaron · 2022-12-01T19:12:33Z

src/pages/home/ReportScreen.js

@@ -215,7 +215,10 @@ class ReportScreen extends React.Component {
        const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: this.state.viewportOffsetTop}];

        // There are no reportActions at all to display and we are still in the process of loading the next set of actions.
-        const isLoadingInitialReportActions = _.isEmpty(this.props.reportActions) && this.props.report.isLoadingReportActions;
+        const isLoadingInitialReportActions = (_.isEmpty(this.props.reportActions) && !this.props.network.isOffline) && this.props.report.isLoadingReportActions;


Why do we need this change?

I guess the thing that confuses me here is why isLoadingReportActions would be true if we are offline. I would maybe expect it to be false or ask why we are triggering a loader state when we know we can't fetch these actions because we are offline.

That was my first question as well. I will look into this part and probably clean it.

marcaaron · 2022-12-01T19:14:55Z

src/pages/home/ReportScreen.js

+        const isLoadingInitialReportActions = (_.isEmpty(this.props.reportActions) && !this.props.network.isOffline) && this.props.report.isLoadingReportActions;
+
+        // Check if for the current chat report there are no report actions available while we are offline
+        const isChatReportEmptyWhileOffline = _.isEmpty(this.props.reportActions) && this.props.network.isOffline;


I think since only the ReportActionsView cares about this we could move the check into that component instead of here.

marcaaron · 2022-12-01T19:16:37Z

src/pages/home/ReportScreen.js

@@ -293,6 +296,7 @@ class ReportScreen extends React.Component {
                                        isComposerFullSize={this.props.isComposerFullSize}
                                        isDrawerOpen={this.props.isDrawerOpen}
                                        parentViewHeight={this.state.skeletonViewContainerHeight}
+                                        isChatReportEmptyWhileOffline={isChatReportEmptyWhileOffline}


Continuing the previous thought.. we are creating this new variable and prop - but nothing else uses isChatReportEmptyWhileOffline so it seems like the responsibility of the ReportActionsView to:

Check the reportActions

See if we're offline

If there are no reportActions and we are offline then incorporate the fallback action

marcaaron · 2022-12-01T19:17:26Z

src/pages/home/report/ReportActionsView.js

+        // If the `isChatReportEmptyWhileOffline` prop is true then let's render ReportActionItemCreated component
+        if (this.props.isChatReportEmptyWhileOffline) {
+            return <ReportActionItemCreated reportID={this.props.report.reportID} />;
+        }


This works, but I think is going to be a bit weird UX. As soon as someone adds an optimistic action while offline this will disappear since reportActions.length > 0.

But do we have any other way out? Because it is bound to happen, right? Can we show some additional message to let user know we don't have data cached?

Yeah, very tricky. So to summarize:

We can tell a report has not loaded yet if it has 0 reportActions.length

If we are offline then we can show this new special "created" action

As soon as they add a comment it will look like the report has loaded

So the question becomes...

How can we tell that we need to keep showing the "created" action if we are depending on reportActions.length and adding optimistic actions will increase this length?

First solution that comes to mind is something like...

const hasAtLeastOneNonPendingAction = reportActions.some(action => !action.pendingAction);

Which would be the same as "empty or empty with only pending actions". What do you think?

I am not sure if I am thinking in the right direction or not but the problem with some will be that even if a chat report is loaded and we added an optimistic action to the report the above will evaluate to true right? So it all boils down to the fact the all report actions must be in a pending state or the list should be empty in order for us to show the created action, no?

I also went through the other issue which is very similar to this one and I really liked the idea of showing [broken cloud] Let's sync up when you get back online rather than a created action because both seem to be aligned with our optimistic approach but the latter one defines the situation better here.

the problem with some will be that even if a chat report is loaded and we added an optimistic action to the report the above will evaluate to true right

My assumption is that if the report has truly loaded it will have a "created" action that is not pending.

all report actions must be in a pending state or the list should be empty in order for us to show the created action, no

Yep, I think that makes sense.

I'm wondering should be showing the "created" action at all in this case. Because what if you have tons of chat history in the chat but you're simply offline so you haven't loaded it yet? In this issue we discussed showing the Skeleton UI without the animation, along with the offline indicator under the chat input. I wonder if we think this is a viable alternative?

I will bring this discussion into slack though so we can get on the same page a bit quicker.

Yeah, I agree we can show the skeleton UI without animation rather than a created action.

…f_no_reportAction_exists_while_Offline

techievivek · 2022-12-12T06:45:27Z

Keeping this on HOLD until the online behaviour gets fixed here.

…f_no_reportAction_exists_while_Offline

melvin-bot · 2022-12-20T13:00:01Z

@Santhosh-Sellavel @robertjchen One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

techievivek · 2022-12-20T13:01:47Z

This is ready for review. @marcaaron @yuwenmemon
I am seeing a bug where messages sent offline change positions once the user comes online, but this is already being discussed here and is not a priority for now.

yuwenmemon · 2022-12-20T21:35:50Z

src/pages/home/report/ReportActionsList.js

@@ -145,6 +145,10 @@ class ReportActionsList extends React.Component {
    }

    render() {
+        // Non-animating skeleton UI represents that chat history exists but is not loaded locally in Onyx.
+        // While offline if there is at least one non-pending action or if this is a new chat then we don't need to show the non-animating skeleton UI.
+        const shouldShowNonAnimatingSkeleton = !(_.some(this.props.sortedReportActions, action => !action.pendingAction || action.actionName === CONST.REPORT.ACTIONS.TYPE.CREATED));


Parens are unneccessary

How is this different from this.props.report.isLoadingReportActions?

shouldShowNonAnimatingSkeleton is not a great variable name either. It should ideally reflect what it represents - so in this case ... whether or not we still have report actions to load? (Hence why I have my question above)

this.props.isLoadingReportActions should be false when we are offline(more here), so I have updated some parts of code, so we don't call openReport when we are offline.

So we need another way to know whether we want to show the non-animating skeleton UI. (i.e., when we don't have any chat history in onyx and all we are left with are optimistic actions that were added by the user while being offline). So we will show the non-animating skeleton UI only if every action is optimistic and it is not a new chat report.

I have updated the condition to be more clear.

// Non-animating skeleton UI represents that chat history exists but is not loaded locally in Onyx. // While offline if there is at least one non-pending action or if this is a new chat then we don't need to show the non-animating skeleton UI. const shouldShowNonAnimatingSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);

yuwenmemon · 2022-12-20T21:36:13Z

src/pages/home/report/ReportActionsList.js

@@ -163,7 +167,7 @@ class ReportActionsList extends React.Component {
                    keyExtractor={this.keyExtractor}
                    initialRowHeight={32}
                    initialNumToRender={this.calculateInitialNumToRender()}
-                    onEndReached={this.props.loadMoreChats}
+                    onEndReached={() => !this.props.network.isOffline && this.props.loadMoreChats()}


Could we not move the isOffline check into the loadMoreChats function itself?

Agree, that would be cleaner.

techievivek · 2022-12-29T09:00:36Z

Gentle bump for reminding everyone to review this PR. Thanks

marcaaron

I'm having some trouble wrapping my head around these changes, but don't want to be a blocker on this PR. I have a couple of ideas on how to proceed:

Discuss more widely what it is we are trying to solve and the best way to do it. It feels like we are experimenting in the PR a bit (which is OK, but might be less effective than leading a conversation to get everyone on the same page before making changes).
Bring someone else in on the review who is also familiar with the ReportScreen flows to weigh in on some of the comments I've left.

marcaaron · 2022-12-29T19:24:47Z

src/pages/home/ReportScreen.js

@@ -165,7 +165,8 @@ class ReportScreen extends React.Component {
        // It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
        // is not stored locally yet. If props.report.reportID exists, then the report has been stored locally and nothing more needs to be done.
        // If it doesn't exist, then we fetch the report from the API.
-        if (this.props.report.reportID) {
+        // If the client is offline, we cannot fetch the report, so let's just return.


This comment explains what we are doing but not "why".

Looking at this - and I don't understand why being offline should prevent us from making this request. Here's my understanding of how our networking layer works:

OpenReport is a "write" request because we update the "last read" while fetching the report

If we are offline we will queue any "write" requests

App/src/libs/API.js

Lines 47 to 48 in 0ed2636

// Write commands can be saved and retried, so push it to the SequentialQueue

SequentialQueue.push(request);

App/src/libs/Network/SequentialQueue.js

Lines 90 to 96 in 0ed2636

// Add request to Persisted Requests so that it can be retried if it fails

PersistedRequests.save([request]);

// If we are offline we don't need to trigger the queue to empty as it will happen when we come back online

if (NetworkStore.isOffline()) {

return;

}

When we come back from offline -> online those queued "write" requests will run

App/src/libs/Network/SequentialQueue.js

Lines 83 to 84 in 0ed2636

// Flush the queue when the connection resumes

NetworkStore.onReconnection(flush);

In this case, we are offline - so shouldn't it be fine to make the request? And when we come back online the report will be fetched?

Is there some problem with the way this is working now that needs to be addressed? Because I want to say it's an anti-pattern to prevent a "write" from happening if we are offline (as that's what the queue itself was designed to do) but maybe I am missing something.

The reason I added this change was otherwise in the openReport method call

App/src/libs/actions/Report.js

Lines 309 to 314 in 0bde3f8

optimisticData: [{

onyxMethod: CONST.ONYX.METHOD.MERGE,

key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,

value: {

isLoadingReportActions: true,

isLoadingMoreReportActions: false,

it would set isLoadingReportActions: true and because of that in the ReportScreen.js

App/src/pages/home/ReportScreen.js

Lines 293 to 297 in 0bde3f8

{(this.isReportReadyForDisplay() && !isLoadingInitialReportActions) && (

<>

<ReportActionsView

reportActions={this.props.reportActions}

report={this.props.report}

we will keep showing the animating full-screen skeletonUI and the reportActionsView will never get loaded. I don't think that is expected if we want users to take optimistic actions here. What do you think about it?

This change and one below in ReportActionsView page are mainly to prevent setting value for a report prop. Do you have any recommendations on how to handle this better?

Let's stop here and consult the team on the best way to handle this.

The main question in my mind is why we should ever set any loading state before the API request actually begins? In the current design, it seems like we have a hybrid where loading states are set to true even though we are offline and not actually "loading" anything.

Taking a look at our usages elsewhere - and this isLoading is mostly only being used in a few "blocking read requests" (stuff related to payments and bank accounts) that I don't think will ever happen unless we are online. So it feels like there's an inconsistency or exception being made for the report page - and I'm not sure it's valid.

marcaaron · 2022-12-29T19:33:45Z

src/pages/home/report/ReportActionsList.js

@@ -145,6 +141,10 @@ class ReportActionsList extends React.Component {
    }

    render() {
+        // Non-animating skeleton UI represents that chat history exists but is not loaded locally in Onyx.


Non-animating skeleton UI represents that

I'd rephrase this in this format

When [x] happens a non-animating skeleton UI is shown because...

But honestly, I'm a bit confused about how to finish that sentence.

I'm having trouble understanding the idea that "chat history exists but is not loaded locally in Onyx" so I think we need to improve this comment more generally.

Does it mean that the data exists in storage, but not yet read into memory?

Or does it mean that we know some data exists on the server, but we haven't tried to load it yet?

Let's please try to be very specific here so that people don't have to work too hard to understand why we are doing something.

Or does it mean that we know some data exists on the server, but we haven't tried to load it yet?

This is the case here, I will update it.

Awesome thanks! And to make sure I understand - we know this is the case because we have some report data already stored, the report is "not optimistic", and no report actions?

Yep, that's my understanding so far 😄 (This is my first App PR in like 4-5 months).

marcaaron · 2022-12-29T19:35:05Z

src/pages/home/report/ReportActionsList.js

@@ -145,6 +141,10 @@ class ReportActionsList extends React.Component {
    }

    render() {
+        // Non-animating skeleton UI represents that chat history exists but is not loaded locally in Onyx.
+        // While offline if there is at least one non-pending action or if this is a new chat then we don't need to show the non-animating skeleton UI.
+        const shouldShowNonAnimatingSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);


Suggested change

const shouldShowNonAnimatingSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);

const shouldShowStaticSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);

We generally prefer variables that describe what something is vs. what it is not.

Alternatively, change this to shouldAnimateSkeleton and then we don't have to invert later !shouldShowNonAnimatingSkeleton. I think I like that suggestion better.

Seems unrelated to the changes here. But I'm kind of confused about why and how isOptimisticReport got introduced. I'd think that an optimistic report would have a pendingAction and not a separate flag to denote it as "optimistic".

We have decided to show the static skeleton UI for the chats that we know exist on the server but aren't loaded locally in ONYX.

Now if the chat is not loaded locally and we are offline then we can still take optimistic action but we will need to keep showing the static skeleton UI. But in the case of a new chat report, we don't have to show the static skeleton UI because we know this chat report doesn't exist on our server. But if I just check by pendingAction then static skeleton UI would show up for new chats as well so to filter them out I have added the condition to see if it is an optimistic report(which I think denotes a new chat created while offline).

Ah hmm, that's good context, but your response here isn't directly addressing either of my two concerns above (though perhaps you meant to add this context to a different thread).

Would you mind asking a follow up question or restating what you think I'm asking about here? I am happy to clarify things or say them in a different way to help us get on the same page.

Aaah, my bad. I see your question now. But to be honest, I don't have any explanation for this one(will have to check the blame). I used it because it was right in the report object, but this could have been easily computed as well.

marcaaron · 2022-12-29T19:38:18Z

src/pages/home/report/ReportActionsList.js

@@ -177,12 +177,13 @@ class ReportActionsList extends React.Component {
                        // Make sure the oldest report action loaded is not the first. This is so we do not show the
                        // skeleton view above the created action in a newly generated optimistic chat or one with not
                        // that many comments.
+                        // We will show non-animating skeleton UI if we are offline and chat history is not loaded in ONYX.


Comment does not explain "why" only the "what". Also let's try to be more consistent about capitalization. In other comments we have "Onyx" and this one "ONYX".

Suggested change

// We will show non-animating skeleton UI if we are offline and chat history is not loaded in ONYX.

// We will show a static skeleton UI if we are offline and chat history is not loaded in Onyx because...

marcaaron · 2022-12-29T19:44:26Z

src/pages/home/report/ReportActionsList.js

@@ -145,6 +141,10 @@ class ReportActionsList extends React.Component {
    }

    render() {
+        // Non-animating skeleton UI represents that chat history exists but is not loaded locally in Onyx.
+        // While offline if there is at least one non-pending action or if this is a new chat then we don't need to show the non-animating skeleton UI.


How does being offline come into play here? This component does not seem aware of the network status anymore since we are removing withNetwork.

Aaah, I was just trying to explain in comments and used that word. I will get rid of it.

marcaaron · 2022-12-29T19:46:11Z

src/pages/home/report/ReportActionsView.js

@@ -270,7 +270,7 @@ class ReportActionsView extends React.Component {

    // If the report is optimistic (AKA not yet created) we don't need to call openReport again
    openReportIfNecessary() {
-        if (this.props.report.isOptimisticReport) {
+        if (this.props.report.isOptimisticReport || this.props.network.isOffline) {


The first comment applies here too.

marcaaron · 2022-12-29T19:47:10Z

src/pages/home/report/ReportActionsView.js

@@ -283,7 +283,7 @@ class ReportActionsView extends React.Component {
     */
    loadMoreChats() {
        // Only fetch more if we are not already fetching so that we don't initiate duplicate requests.
-        if (this.props.report.isLoadingMoreReportActions) {
+        if (this.props.report.isLoadingMoreReportActions || this.props.network.isOffline) {


Why are we preventing this request from happening?

I have prevented calling this otherwise it would render an animating skeleton UI in the ReportActionList.js page.

App/src/pages/home/report/ReportActionsList.js

Lines 169 to 173 in 0bde3f8

if (this.props.report.isLoadingMoreReportActions) {

return (

<ReportActionsSkeletonView

containerHeight={CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT * 3}

/>

Ok I think this is also related to the weird issue where we are setting "loading" states when no "loading" is actually happening and we should pause until there is agreement about how to proceed with that.

marcaaron · 2022-12-29T19:51:02Z

src/pages/home/report/ReportActionsView.js

@@ -352,7 +352,7 @@ class ReportActionsView extends React.Component {

    render() {
        // Comments have not loaded at all yet do nothing
-        if (!_.size(this.props.reportActions)) {
+        if (!_.size(this.props.reportActions) && !this.props.network.isOffline) {


I think this needs a comment. It's not obvious why we should choose to return null when there are no report actions and we are online.

It also feels like maybe we should not be rendering the ReportActionsView at all if we are offline and have no actions.

I guess this whole flow is a bit messed up. I am using ListFooterComponent to show the static skeleton UI so I have this additional condition that will prevent ReportActionsView from returning null.

Another thing I can explore is to show the static skeleton view right from the ReportScreen page.

I guess this whole flow is a bit messed up.

I agree and think it's a possible sign that we are gonna make it even more messed up than it already is.

I would propose we stop exploring code changes for a moment and instead try to pull things back into the realm of problem/solution.

Next steps:

Figure out what problem we need to solve (I think we have mostly done this on the issue)

Look at how things work today. Does the existing code easily support it or not? Does an obvious solution present itself or not? What solution should we ultimately go with?

Summarize our best idea and take it to a few people to see what they think (either #engineering-chat or #expensify-open-source seems good)

Agree on a plan forward + make sure everyone understands the general thought process behind the changes

Make the changes

Is this discussion happening? If yes, it's the channel I don't have access to?

@aimane-chnaif No, @techievivek has been OOO - so you're not missing out on anything I believe 🙂

Catching up, I will open a thread to discuss this issue in the open-source channel by tomorrow.

I have opened a discussion here feel free to jump to add your thoughts.

aimane-chnaif · 2023-01-04T11:51:49Z

@techievivek please pull from latest main when ready for review. Now report footer (composer) is always shown while loading skeleton.
Also FYI in #12698, we'll show skeleton on Report screen even before personalDetails are loaded.

marcaaron · 2023-01-17T20:16:40Z

I think we are still discussing these changes. @techievivek do you think it might be helpful to just close out this PR for now until there's more of a consensus on what an agreed Problem and Solution? I'm trying to help move this one forward, and if feels like there's been so much conversation that it would be best to just start over at this point with a fresh Problem/Solution statement.

yuwenmemon · 2023-01-17T22:19:35Z

Based off of #13088 (comment) I do think we should close this out. Feel free to reopen if you disagree!

checked if we are offline and no reportactions exists in onyx for cha…

3ab922c

…t report

techievivek self-assigned this Dec 1, 2022

techievivek requested a review from a team December 1, 2022 13:33

melvin-bot bot requested review from eVoloshchak and marcaaron and removed request for a team December 1, 2022 13:33

added prop and returned early with ReportActionItemCreated component …

1297bee

…if isChatReportEmptyWhileOffline is true

techievivek force-pushed the techievivek_show_newly_created_chat_action_if_no_reportAction_exists_while_Offline branch from c8278a7 to 1297bee Compare December 1, 2022 13:38

marcaaron requested changes Dec 1, 2022

View reviewed changes

marcaaron mentioned this pull request Dec 2, 2022

[HOLD for payment 2022-12-27] White screen appears when loading chat, after being redirected from IOU modal #13074

Closed

techievivek mentioned this pull request Dec 5, 2022

Opening a chat that was not opened recently while being offline seems to take forever to load #13088

Closed

techievivek added 5 commits December 8, 2022 18:20

Merge branch 'main' into techievivek_show_newly_created_chat_action_i…

bed47a5

…f_no_reportAction_exists_while_Offline

updated code to show non-animating skeleton UI

2f52d2e

non-animation UI and donot call openReport while offline

bd9d22b

rendered skeleton UI of window height

a0112c6

donot show non-animating skeleton UI if its a new chat

8547182

techievivek force-pushed the techievivek_show_newly_created_chat_action_if_no_reportAction_exists_while_Offline branch from 83c5d00 to 8547182 Compare December 8, 2022 18:35

This comment was marked as off-topic.

Sign in to view

techievivek requested review from yuwenmemon and marcaaron December 8, 2022 18:44

techievivek added 3 commits December 15, 2022 12:25

calculated height for skeleton view based on layout

b58a3b2

Merge branch 'main' into techievivek_show_newly_created_chat_action_i…

2a9c759

…f_no_reportAction_exists_while_Offline

resolve merge conflicts

7b03e39

techievivek force-pushed the techievivek_show_newly_created_chat_action_if_no_reportAction_exists_while_Offline branch from 6867fce to 7b03e39 Compare December 20, 2022 11:44

techievivek marked this pull request as ready for review December 20, 2022 12:59

techievivek requested a review from a team as a code owner December 20, 2022 12:59

melvin-bot bot removed the request for review from a team December 20, 2022 12:59

melvin-bot bot requested review from robertjchen and Santhosh-Sellavel December 20, 2022 13:00

techievivek removed request for robertjchen and Santhosh-Sellavel December 20, 2022 13:02

yuwenmemon requested changes Dec 20, 2022

View reviewed changes

updated check for shouldShowNonAnimatingSkeleton

e4219bc

techievivek force-pushed the techievivek_show_newly_created_chat_action_if_no_reportAction_exists_while_Offline branch from d4d629d to e4219bc Compare December 21, 2022 07:33

techievivek requested a review from yuwenmemon December 21, 2022 16:42

marcaaron requested changes Dec 29, 2022

View reviewed changes

techievivek changed the title ~~Show chat as new with offline message if no report action exists for chat report while offline~~ Show static skeleton UI if no reportAction exists for chat report in ONYX when the client is offline Dec 30, 2022

techievivek requested review from aimane-chnaif and removed request for eVoloshchak December 30, 2022 11:57

techievivek mentioned this pull request Jan 12, 2023

[HOLD for payment 2023-04-12] [$4000] [Feature Request] Compose box should be accessible when opening a old chat #14223

Closed

6 tasks

yuwenmemon closed this Jan 17, 2023

	// Write commands can be saved and retried, so push it to the SequentialQueue
	SequentialQueue.push(request);

	// Add request to Persisted Requests so that it can be retried if it fails
	PersistedRequests.save([request]);

	// If we are offline we don't need to trigger the queue to empty as it will happen when we come back online
	if (NetworkStore.isOffline()) {
	return;
	}

	// Flush the queue when the connection resumes
	NetworkStore.onReconnection(flush);

	optimisticData: [{
	onyxMethod: CONST.ONYX.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
	value: {
	isLoadingReportActions: true,
	isLoadingMoreReportActions: false,

	{(this.isReportReadyForDisplay() && !isLoadingInitialReportActions) && (
	<>
	<ReportActionsView
	reportActions={this.props.reportActions}
	report={this.props.report}

	const shouldShowNonAnimatingSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);
	const shouldShowStaticSkeleton = !this.props.report.isOptimisticReport && _.every(this.props.sortedReportActions, action => action.pendingAction);

	// We will show non-animating skeleton UI if we are offline and chat history is not loaded in ONYX.
	// We will show a static skeleton UI if we are offline and chat history is not loaded in Onyx because...

	if (this.props.report.isLoadingMoreReportActions) {
	return (
	<ReportActionsSkeletonView
	containerHeight={CONST.CHAT_SKELETON_VIEW.AVERAGE_ROW_HEIGHT * 3}
	/>

Show static skeleton UI if no reportAction exists for chat report in ONYX when the client is offline #13233

Show static skeleton UI if no reportAction exists for chat report in ONYX when the client is offline #13233

Conversation

techievivek commented Dec 1, 2022 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

techievivek commented Dec 1, 2022 • edited Loading

melvin-bot bot commented Dec 1, 2022

marcaaron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

techievivek commented Dec 12, 2022

melvin-bot bot commented Dec 20, 2022

techievivek commented Dec 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek commented Dec 29, 2022

marcaaron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

techievivek Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aimane-chnaif commented Jan 4, 2023 • edited Loading

marcaaron commented Jan 17, 2023

techievivek commented Dec 1, 2022 •

edited

Loading

techievivek commented Dec 1, 2022 •

edited

Loading

techievivek Dec 5, 2022 •

edited

Loading

techievivek Dec 21, 2022 •

edited

Loading

techievivek Dec 30, 2022 •

edited

Loading

techievivek Dec 30, 2022 •

edited

Loading

techievivek Jan 5, 2023 •

edited

Loading

techievivek Jan 5, 2023 •

edited

Loading

techievivek Dec 30, 2022 •

edited

Loading

techievivek Dec 30, 2022 •

edited

Loading

aimane-chnaif commented Jan 4, 2023 •

edited

Loading