matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.84k stars 2.64k forks source link

Flattening Tables via API and UI #3073

Closed timo-bes closed 12 years ago

timo-bes commented 12 years ago

Provide a mechanism to make nested tables into a single flat table. This will allow users to compare (i.e. sort) rows from different data tables that make up a nested report.

timo-bes commented 12 years ago

(In [6124]) refs #3073 data table flattener and test cases

timo-bes commented 12 years ago

The previous commit introduces a base class for manipulating data tables. The idea is that the label filter uses it as a base as well.

Please review.

timo-bes commented 12 years ago

(In [6125]) refs #3073 svn properties for previous commit

timo-bes commented 12 years ago

(In [6126]) refs #3073 proper class name for tests

timo-bes commented 12 years ago

There's a number of things that the UI mechanism should provide.

How about a dialog to configure the way the table should be displayed?

We could have some explanation of the different options there to make it easier than more icons at the bottom of the table. Also, the selected behavior from the dialog could influence the way the export is rendered. So most of the stuff we discussed about an export dialog would be valid here as well.

Here' a very rough sketch of what I have in mind:

[  ]  Flatten the report

---

(  )  Concatene labels

(  )  Keep labels and indent

---

(  )  Include inner nodes

(  )  Only show the leafs

What do you think?

I know this is not the way we usually do things but adding more icons will become confusing at some point. With this dialog, we can easily add more options in the future. Or is there a more simple way for adding the functionality we need?

mattab commented 12 years ago

Code review:

mattab commented 12 years ago

Also there's the option to include inner nodes or not. We could provide good defaults for including inner node via meta data but people might want different things.

What would the difference be exactly when this is enabled or disabled?

Since there are multiple version of flattening planned, we need a way to select that as well.

For this, we could for example name each combination, and put a screenshot of what the data will look like when this is ticked. I saw a very good example of such UI in Songbird software. It shows you screenshot of the directory/file organization based on the automatic filename pattern you choose.

timo-bes commented 12 years ago

For what including inner nodes means, see #2714

In your review, what css code are you talking about?

So should we add a dialog? I would suggest to make it appear between the table and the icons when you click the (leftmost) table icon.

Filters and what I called Manipulators are different (btw: the label filter is a manipulator as well, I want to refactor it into the new directory):

Are you sure we should combine those two things? IMO, only loading subtables from different places makes them different enough to keep them separate.

The alternative would be to force expanded=1 and use them in API_DataTableGenericFilter. Then they would work like the other filters. But that's very wasteful for the label filter becuase at the moment, it only loads the subtables from the archive it really needs. That makes a huge difference on reports like the page urls report.

mattab commented 12 years ago

Replying to EZdesign:

For what including inner nodes means, see #2714

Thanks. we have to change the name... How about "keepParents" or "keepExpanded" ?

In your review, what css code are you talking about?

I meant, it would be nice to use this new code in the CSV exporter which does some of the same logic.

So should we add a dialog?

I have one idea among others:

It's just an idea, not sure how to integrate this easily yet discreetly as it will not be used a lot and cannot add a new icon visible all the time?

I would suggest to make it appear between the table and the icons when you click the (leftmost) table icon.

agreed, rather than popover..

Filters and what I called Manipulators are different (btw: the label filter is a manipulator as well, I want to refactor it into the new directory):

  • Filters mainly change rows and keep the overall structure intact - Manipulators change the entire thing.
  • Filters work on data in memory; the base class loads from Piwik_DataTable_Manager - Manipulators work on archived data; the base class Piwik_API_DataTableManipulator loads them from the API
  • Filters are used in the APIs of plugins - Manipulators are used in ResponseBuilder

Thanks. Can you please write this as a comment in the manipulator?

Are you sure we should combine those two things? IMO, only loading subtables from different places makes them different enough to keep them separate. OK as long as code is not duplicated a lot between these 2 similar concepts (but they're different so OK)

The alternative would be to force expanded=1 and use them in API_DataTableGenericFilter. Then they would work like the other filters. But that's very wasteful for the label filter becuase at the moment, it only loads the subtables from the archive it really needs. That makes a huge difference on reports like the page urls report.

Can you please elaborate on this problem?

Cheers

timo-bes commented 12 years ago

(In [6159]) refs #3073 flatten table UI, option to include aggregate rows (this option will replace includeInnerNodes in the CSV renderer)

timo-bes commented 12 years ago

I thought long and hard about how we could add these new options to the footer that already is pretty packed. So I added a dialog similar to the selection of how many rows you want to display. I put exclude low population and options for flattening in there. It can easily be extended in the future.

I renamed includeInnerNodes to includeAggregateRows (since you suggested to pick a different name). Is that more clear? When the option is selected, the labels of the aggregate rows are suffixed with [aggregate] to make it clear. The option is only there in the UI if the report is flat.

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

mattab commented 12 years ago

UX Review:

mattab commented 12 years ago

I renamed includeInnerNodes to includeAggregateRows (since you suggested to pick a different name). Is that more clear?

Yes thanks. However there are some "InnerNodes" left in code ini many places can you please change everywhere for consitency?

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

  • That is very cool. How ever the graph don't work with flattened, but it would be very good to be able to plot these tables. It would allow for first time to plot in particular custom variable values, which I know many users are interested in.
  • it should also be fowrarded in all clicks in footer (API, tableGoals, tableAllColumns etc)
mattab commented 12 years ago

(In [6162]) Refs #3073 New icon more discreet

mattab commented 12 years ago

One more UX suggestion:

Thoughts?

timo-bes commented 12 years ago

Replying to matt:

  • Please open the submenu on hover, without requiring a click, as this should be easy to access yet not taking place

I did it on click because to number of rows selection opens on click too. They look similar and are located next to eachother. Thus, they should work in a similar fashion. So do we do both on hover or both on click? The problem with hover is that the number of rows selection is pretty narrow so it would be easy to mouseout on accident.

  • What other options do we plan to include in this window later on?

I'm sure we will come up with new things ;-)

Replying to matt:

Matt, you mentioned two kinds of flattening in an email to me. If I understood what you meant correctly, both are implemented now. Please check.

  • That is very cool. How ever the graph don't work with flattened, but it would be very good to be able to plot these tables. It would allow for first time to plot in particular custom variable values, which I know many users are interested in.

I don't understand. Can you clarify? Do you mean row evolution? What is special about the custom variables in this context?

Replying to matt:

  • when we start recording the full status of each view (exclude low pop, flatten, etc.) to restore the table as they were, I think it will be very important to highlight the fact that the table is currently flattened / or that low populations are excluded (or both).
    • when any option was clicked or initialized in the new menu (btw we need a name for it...), Could you please put the icon in yellow? I think this would at least help users understand a bit why the view is the way it is.
    • it would be also very nice if we could somehow display somewhere "Report is flattened - un-flatten report" or "Rows with few visits are hidden - Show all rows" so it is clear, just by reading the menu, what is the status of the report.

Maybe we could show the modified state of the table on hover over the icon. So you'd have a highlighted icon on the bottom of the table. You'd hover over it and the tooltip would show "Report is flat. Aggregate rows are excluded". On click, the menu would open and give you the options. This way, you could quickly see what is going on with the table on hover.

timo-bes commented 12 years ago

Replying to matt:

(In [6162]) Refs #3073 New icon more discreet

Are you sure this is an improvement? I don't think the faded style fits well with the other icons.

timo-bes commented 12 years ago

(In [6181]) refs #2714, refs #3073

timo-bes commented 12 years ago

When you flatten the websites report and then display it as a bar chart, you get weird labels like www.example.org/http://example.org/thePage.html. This is because the flattener gets a data table with the full URLs as labels for the second level. It does not happen when the report is displayed as a regular data table. Do you know why?

mattab commented 12 years ago

Feedback on new commit:

I see some very useful use case where you could see the evolution over time of a particular custom variable name-value pair... very meaningful & powerful that would be!

I don't understand. Can you clarify? Do you mean row evolution? What is special about the custom variables in this context? Sorry, I didn't identify the problem well, you fixed the issue by forwarding the "flat" parameter.

I did it on click because to number of rows selection opens on click too. They look similar and are located next to eachother. Thus, they should work in a similar fashion. So do we do both on hover or both on click? The problem with hover is that the number of rows selection is pretty narrow so it would be easy to mouseout on accident.

My thoughts are that:

Im definitely more keen for a one fit all approach: show on hover the menu that shows both the current status and links to change status ie.

Maybe you can try it and if you don't like it we can all try it and discuss it. I think it will make the feature more accessible and generally better :)

Are you sure this is an improvement? I don't think the faded style fits well with the other icons.

I'm not entirely convinced indeed... but I prefer it than the previous one.

However we should keep open mind as there might be better solution ? thoughts?

aggregate rows are italic (imo, the plus logo would be misleading because it would suggest clickability)

I thought initially about using this icon which would be consistent with the Pages report: http://demo.piwik.org/themes/default/images/minus.png ? I think it would look nice and easier to understand. But then I realized that aggregated rows are not kept together but are sorted by metric. So, the minus icon would be very confusing in this case.

Displaying in italic is definitely an improvement!

When you flatten the websites report and then display it as a bar chart, you get weird labels like www.example.org/http://example.org/thePage.html. This is because the flattener gets a data table with the full URLs as labels for the second level. It does not happen when the report is displayed as a regular data table. Do you know why?

I took a quick 5minutes look and couldn't find the cause. I'll definitely check before the release if you haven't found the bug then.

I'm sure we will come up with new things ;-) Definitely... I already thought of one: "Expand all rows" #1040 which would be very cool and useful for example for the Downloads/Outlinks reports (usually a few hosts only) or for the page URLs report (on small websites).

timo-bes commented 12 years ago

Replying to matt:

Is "flat" working OK with row evolution? if it's working but the only problem is performance, I would be very keen to leave the feature in, and we can improve it later (i'm hoping to look into row evolution performance at some point). I see some very useful use case where you could see the evolution over time of a particular custom variable name-value pair... very meaningful & powerful that would be!

Row evoution does work on custom variable name-value pairs. Just open the subtable and click the icon. Using it on flat tables does not add any functionality. It's just orders of magnitue slower. Also, it doesn't work without further code modifications.

timo-bes commented 12 years ago

Regarding the icon, take a look at this: http://www.iconfinder.com/search/?q=iconset%3Asilk2+cog

We could show the first one (maybe faded out a little) by default. On hover, the menu opens and it's shown opaquely. When low population is excluded or the report is flat, we can show one of the other icons, maybe the one with the small yellow (!) sign?

timo-bes commented 12 years ago

(In [6182]) refs #3073

timo-bes commented 12 years ago

(In [6183]) refs #3073

timo-bes commented 12 years ago

(In [6184]) refs #3073 svn properties for new file

timo-bes commented 12 years ago

(In [6185]) refs #3073: extended test suite FlattenReports

mattab commented 12 years ago

KABOOM! it looks beautiful!!

Row evoution does work on custom variable name-value pairs. Just open the subtable and click the icon. Using it on flat tables does not add any functionality. It's just orders of magnitue slower. Also, it doesn't work without further code modifications.

That makes sense. Perfect as it is.

Regarding the icon, take a look at this: http://www.iconfinder.com/search/?q=iconset%3Asilk2+cog We could show the first one (maybe faded out a little) by default. On hover, the menu opens and it's shown opaquely. When low population is excluded or the report is flat, we can show one of the other icons, maybe the one with the small yellow (!) sign?

First one looks good (agreed on fading out as it's too dark).

for the "enabled state", the (!) wouldn't work as it triggers a "warning" feeling. I would be more inclined to have a "highlight" effect (for example yellow hallow around the icon, maybe needs a new icon image) that shows that something is enabled, but should not distract the user, but I'm not sure what would work nice... thoughts?

worst case scenario we could use: http://www.iconfinder.com/icondetails/9568/22/cog_face_female_gearhead_icon

Feedback:

timo-bes commented 12 years ago

(In [6191]) refs #3073 new icon, highlighted icon when the configuration differs from default, showing (default) in options

timo-bes commented 12 years ago

(In [6192]) refs #3073 removing left over lang string

timo-bes commented 12 years ago

(In [6194]) refs #3073: cross-browser ie789, safari, chrome, firefox

timo-bes commented 12 years ago

My to do list for this feature is empty now.

Are there any more suggestions? If not, we can close the ticket.

mattab commented 12 years ago

Well done, Looks good to me :)

sgiehl commented 12 years ago

(In [6242]) refs #3073 save new options in dashboard, so that they are restored

mattab commented 12 years ago

Small bug noticed: the cog icon should appear on the graph, since the graph plotting flattened custom variables is very useful in some cases, but better have a way to disable the flattening.

mattab commented 12 years ago

Fixed in [6424]