Hook up "No Transactions" view

[allendav]

When there are no transactions available to the display, present something meaningful instead of an empty table.

cc @LevinMedia for a mock

Child of #82

[LevinMedia]

This will be best served by the empty table component:

https://woocommerce.github.io/woocommerce-admin/#/components/packages/table?id=emptytable-component

for a message lets use "No transactions to display"

[reykjalin]

Using the EmptyTable component results in this:

Is this what's expected? I feel like there should be more of a "table skeleton", like table headers and a title or something along those lines so it's not extremely different from what a filled table looks like.

[dechov]

@reykjalin As you said and as @LevinMedia has pointed out, we may want the columns and other heading / controls – so we can render the table and then the <EmptyTable> below it, and we probably will want to use lower-level components than <TableCard>.

[reykjalin]

@dechov @allendav Should we be including that in resolving this issue, or would that be a different issue?

Specifically, changing from a <TableCard> to something lower level like some combination of <TableSummary> and <Table> I feel like should probably be a separate issue?

Also, since <TableCard> only contains <TableSummary>, <Table>, and <Pagination>, wouldn't implementing this using lower-level components be sort of redundant, or would we want to use different components?

[dechov]

@reykjalin Hm, I'm not sure. The table designs don't look to me like they are necessarily <Card>s, going by the border and title at least – I just used it initially for convenience. @LevinMedia do you know one way or another, based on the use of Cards in the design system?

[LevinMedia]

This is getting out of my wheelhouse 😄 but I think <TableCard> is what we're after as a wrapper for all of this. cc @psealock or @timmyc for best practices on setting up tables.

[reykjalin]

I'm starting to think the best way is to just add a summary saying "No transactions to display" if the table is empty, like this:

@LevinMedia @dechov Let me know whether this ^ would work.

I think the most consistent/obvious way to approach this would be to add something along the lines of an emptyMessage property to the <TableCard>, and display that message in an <EmptyTable> if the rows prop is empty on <TableCard>.

I can have this look sort of the same using an <EmptyTable> by wrapping <Table> with no data and <EmptyTable> in a <Card>, but the additional functionality provided by <TableCard> makes it both look different, and it's missing features such as enabling/disabling columns.

<TableCard>	<Card> wrapping <Table> and <EmptyTable>

EDIT 23:00 UTC June 28:
Just to add to this a bit: the emptyMessage property suggestion should probably be on <Table>, and in <TableCard> the emptyMessage property can just be passed through to <Table>. That way you could have consistent design across both <Table>s and <TableCard>s with just one implementation.

That said, I don't know if this fits with @LevinMedia's (and others?) design, so I think this might be a question best answered by them 🙂 .

[timmyc]

The only EmptyTable usage right now in wc-admin is on the dashboard leaderboard areas. @LevinMedia if you come up with a desired behavior for this across all tables, let me know and we can update report tables to match in wc-admin.

[dechov]

I defer to @LevinMedia, especially on how far upstream to take it, but the appearance looks right to me.

I'm not sure if the missing features in the empty state are a big deal – but I actually didn't realize those features (incl. search, download, etc.) were on TableCard, not Table, so I guess we shouldn't give up TableCard as easily as I was suggesting :-)

[LevinMedia]

@LevinMedia @dechov Let me know whether this ^ would work.

Yeah I think that looks great.

I think the most consistent/obvious way to approach this would be to add something along the lines of an emptyMessage property to the ,

That seems like a reasonable approach to me! Sounds like it's inline with what @timmyc mentioned above too:

@LevinMedia if you come up with a desired behavior for this across all tables, let me know and we can update report tables to match in wc-admin.

I'll sketch up a design for that first thing tomorrow (Monday July 1)

[LevinMedia]

@reykjalin I added an empty state to the transactions page in figma

The row height with the message is the same as a normal table row: 60px

The text styles for the empty table message should match the empty message style in the chart component:
https://github.com/woocommerce/woocommerce-admin/blob/master/packages/components/src/chart/d3chart/style.scss#L19

cc @timmyc If we're going to make these changes in the <TableCard> there are a couple other small details I wanted to address as well 😄

When a table is empty we're not being very consistent with what remains visible in the table header - I'd like to update it so that everything that's visible when table data is present is still visible in the empty state, it's just disabled. The exception to that rule being the kebab menu that allows users to toggle columns on and off. That should still be functional even when there's no data.

Header elements that are disabled when the table is empty:

Compare button (when applicable)
Search input (when applicable)
Download Button

Header elements that are always functional:

Kebab that controls which columns are visible

[timmyc]

cc @timmyc If we're going to make these changes in the there are a couple other small details I wanted to address as well

If you could make an issue on the wc-admin repo that would be :fab:

[reykjalin]

Just going to ping @LevinMedia here, since I don't see an issue or PR in wc-admin related to this, and I'm unsure if that's because I can't find it, or if @LevinMedia missed this message.

[psealock]

@reykjalin, I made an issue to track on wc-admin side woocommerce/woocommerce-admin#2613

[reykjalin]

As per the discussions in #120 this has been made (sort of) redundant for now. An empty table will now leave the default message No data to display.

Eventually the TableCard component from woocommerce-admin might provide a way to customize that message, but until then the default message will do fine, as per #120 (comment).

If we get to the point where the TableCard component provides a way to customize this, hopefully that will be something as simple as adding a property to the TableCard along the lines of:

emptyMessage={ 'No transactions to display' }

@allendav it might make sense to move this issue to another milestone? I'll leave that up to you.

[naman03malhotra]

Hi folks, I was thinking of picking up this issue as my 2nd "good first issue".

Looks like adding a prop to the TableCard component would give us the desired behaviour of passing a custom message as @reykjalin suggested. Can we discuss if this issue is still relevant before we start working on it? As it has been a while since the last activity.

[naman03malhotra]

Presently the empty table renderers with the default message No data to display. The TableCard component currently does not support the passing of a custom message.

Screenshot with default message on route /payments/transactions

Screenshot with custom message on route /payments/transactions

We can discuss if there is a requirement to pass a custom message as of now, then we have to make changes to the TableCard component in WCAdmin.

IMO it would be a "good to have", not sure about the requirement of this feature at this moment.

Or if the present default message works for us, we can close this issue and re-open if the need arises.

cc @LevinMedia

[LevinMedia]

I think the current present default message works - thanks @naman03malhotra !

[jrodger]

I'm not sure what's going on with the "undefined transactions" displayed in the footer. I've seen that from time to time, but I thought it was limited to the loading state.

Would you mind creating a new issue so we can look into that @naman03malhotra?

What would you expect to be displayed there @LevinMedia? "0 transactions $0 total $0 fees $0 net", something like that? Or omitted entirely?

[naman03malhotra]

@jrodger I observed this issue as well, though it occurs intermittently. Initially, I thought there was a problem with my local setup, but doesn't look like it. I'll create an issue for the same and look into it. Thank you for confirming this!

For now, I am closing this issue as per the comment. #97 (comment)