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

Transactions and deposits counts on the table summary are rendered as "undefined" while the data is loading. #1639

Closed
naman03malhotra opened this issue Apr 20, 2021 · 6 comments · Fixed by #1788
Assignees
Labels
component: payouts Issues related to Payouts component: transactions Issues related to Transactions size: small The issue is sized small. type: bug The issue is a confirmed bug.

Comments

@naman03malhotra
Copy link
Contributor

naman03malhotra commented Apr 20, 2021

The following behavior is applicable for both Transactions and Deposits tables.

Presently we render the number of transactions and deposits in the summary section of the respective tables. While the data is being fetched from the server the count is rendered as undefined.

Screenshot 2021-04-21 at 3 42 00 AM

Screenshot 2021-04-21 at 3 56 33 AM

The same issue can be replicated if the WCPay server is not running and still being proxied by WCPay dev plugin, i.e When API call is failing which results in depositsSummary being an empty object.

Steps to reproduce:

  • goto develop branch.
  • try loading deposits or transactions page, and watch the table summary (bottom of the table).
  • you should see undefined transaction while the table data is being loaded.

Potential solutions:

  1. We can hide the summary from being rendered all together, until we have the final data to display.
  2. We can keep the default count as Zero always.
@naman03malhotra naman03malhotra added type: bug The issue is a confirmed bug. component: transactions Issues related to Transactions component: payouts Issues related to Payouts labels Apr 20, 2021
@naman03malhotra naman03malhotra self-assigned this Apr 20, 2021
@naman03malhotra
Copy link
Contributor Author

As discussed in the slack thread p1618933953413800/1618863954.403900-slack-CGGCLBN58 and #97 (comment) we are leaning toward displaying zero while empty state, which is consistent with our other analytics screen. I am thinking the same, wanted to confirm before I implement the fix.

cc @jrodger @LevinMedia

@jrodger
Copy link
Contributor

jrodger commented Apr 21, 2021

which is consistent with our other analytics screen

This makes sense as a general approach to me.

@LevinMedia
Copy link
Contributor

@naman03malhotra while we're showing the loading state - can we use the preview shape with the pulse to represent that data until it loads? (just like the data in the table above)

@naman03malhotra
Copy link
Contributor Author

can we use the preview shape with the pulse to represent that data until it loads? (just like the data in the table above)

@LevinMedia I went through the codebase and came to know that component responsible to render the footer of the table is TableSummary which doesn't support skeleton loading at the moment. Right now it just takes whatever data is being passed to it and renders it, including falsy data (undefined, null etc).

We can take one of the following approaches to fix this issue.

  1. Render zero until the data is being loaded.
  2. We do not render the summary (footer content) at all unless the data to render is available.
  3. We can make changes to the TableSummary and add the support for the skeleton loader while the data is being fetched.

1st & 2nd are relatively faster to implement and push than 3rd.

IMO, we can go with either 1st or 2nd to fix this issue immediately and eventually go for 3rd. I can create a separate issue in WCAdmin repo(as components reside there) and work on it, meanwhile, I can work on the quick fix.

I like the suggestion for the 3rd, as it is consistent with the rest of the loading pattern. The fix for 3rd will look something like this:
Screenshot 2021-04-23 at 2 47 28 AM

@naman03malhotra
Copy link
Contributor Author

I have created an issue in WCAdmin to implement the skeleton loader for the TableSummary component. woocommerce/woocommerce-admin#6961 I'll follow up on the same.

@LevinMedia
Copy link
Contributor

We do not render the summary (footer content) at all unless the data to render is available.

Updating the table summary to support the skeleton loader is great - thanks! In the mean time, proceeding with option two above works for me - we simply won't display anything until / unless there's data loaded to show. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: payouts Issues related to Payouts component: transactions Issues related to Transactions size: small The issue is sized small. type: bug The issue is a confirmed bug.
Projects
None yet
3 participants