Insights table - New UI

[paolodamico]

Changes

Closes #4492.

• Improves insights table to display smart/useful column and row names.
• Adds median and average options for row calculation.

Checklist

• All querysets/queries filter by Organization, by Team, and by User
• Django backend tests
• Jest frontend tests
• Cypress end-to-end tests
• Migrations are safe to run at scale (e.g. PostHog Cloud) – present proof if not obvious
• Frontend/CSS is usable at 320px (iPhone SE) and decent at 360px (most phones)

[alexkim205]

I love these new table changes, so excited for when we roll this out!

Quick heads up, there is a bug where when we toggle the compare previous option, only one date is shown in the columns. See We Dec 31 below

---

Ninja edit

Found out why this is happening. It looks like the backend is returning differently formed data depending on whether the compare option is toggled on. We select on indexedResults[0].days which sometimes looks like [2021-06-10, 2021-06-11, ...] and other times like [0, 1, 2]. There's also a separate parameter indexedResults[0].dates that exists sometimes.

I've opened a ticket (#4718) with the analytics team. I think we should aim to fix this in the backend first, but the hacky stopgap solution here would be to check for dates and fall back on days if that parameter doesn't exist.

Hope you don't mind my commit to fix this!

[samwinslow]

Love this! Thank you for taking the reins here — hope some of the CSS I wrote a couple weeks ago was helpful :)

[paolodamico]

Thanks for the reviews! The baseline code you both set up on tooltips and the insight table was indeed super helpful to get this moving quickly.

[paolodamico]

@mariusandra FYI rebased from master post-merge of #4651. Will wait for your review before merging.

[mariusandra]

Love the new look and feel, yet some things still to do before it sparks joy :)

1. We could add this table also below the "bar chart > time" view (and another for "bar chart > value").

2. This is definitely unexpected (possible reason marked out with an inline comment):
   

3. "Sessions" is broken
   

4. It's a bit weird if I have just one line with a breakdown, I don't see the "event", only the breakdown value:
   
   vs multiple:
   

I guess it's fine if this is intentional... if not, might be worth a look. Since the table is infinitely scrollable, it's probably also fine to add more values for clarity/screenshottability.

5. The total/average column is slightly inconsistent and doesn't save its state in the URL. I guess the latter feature's not needed right now, but seeing one thing, sharing to a friend, and then having a different view (see CMD+R in the screencast) is probably not good:
   

[paolodamico]

Thanks for the review @mariusandra. FYI comments below. If you find anything else let me know and I’ll address in the follow-up PR.

1. 👍 addressed
2. 👍 addressed
3. 👍 addressed
4. This is indeed intended, idea is making it easier to understand what’s going on. One less thing to think about? But might actually be the opposite, happy to change, anyone has any thoughts on this? @kpthatsme
5. Agreed but I won’t implement this right now because we’re already refactoring the logic to store the state in the URL and don’t want to add extra complexity for something so minor (and feature flagged) at this point.

[kpthatsme]

On point #4 raised by Marius, I definitely see both sides of it. I don't think it makes a huge difference either way, and given that I think I'd opt to include the event names for consistency and erring on the side of clarity.

One piece of feedback I did have – I feel like it was better to have the aggregate close to the event name. I feel like the first thing you want to see on the table is usually the total sum as the chart shows the values over time.

edit: I realize maybe I've just been trained this way now from seeing it that way in PostHog, so if there was a compelling reason to move it, I'm all for forming a new habit.

[paolodamico]

Thanks @kpthatsme, makes sense. Re location of aggregate column I actually had it as column 3 on my original mockups, but felt this wasn't very intuitive (following typical patterns), but happy to get push back on this. I'll keep it for now and we can iterate too.

[paolodamico]

Ah cypress acting up again, can't seem to getting running at all. Tests were passing the final minor fixes, so merging based off of that.