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

Show flattened columns as extra columns in UI #12524

Merged
merged 27 commits into from Jun 11, 2019
Merged

Show flattened columns as extra columns in UI #12524

merged 27 commits into from Jun 11, 2019

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jan 31, 2018

No description provided.

@sgiehl sgiehl added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Jan 31, 2018
@mattab mattab added this to the 3.4.0 milestone Feb 1, 2018
@sgiehl sgiehl force-pushed the flattenviz branch 2 times, most recently from 4ebedc4 to 5fce3c9 Compare February 3, 2018 17:01
@sgiehl sgiehl force-pushed the flattenviz branch 2 times, most recently from c9c0145 to 5fa0410 Compare March 11, 2018 21:59
@sgiehl sgiehl force-pushed the flattenviz branch 2 times, most recently from 1af6629 to 7c4e352 Compare April 1, 2018 16:40
@diosmosis diosmosis modified the milestones: 3.6.0, 3.7.0 Jul 24, 2018
@mattab mattab modified the milestones: 3.7.0, 3.6.1 Sep 1, 2018
@mattab mattab modified the milestones: 3.6.1, 3.7.0 Oct 8, 2018
@sgiehl sgiehl force-pushed the flattenviz branch 2 times, most recently from 0dc8f1e to cd90367 Compare December 17, 2018 19:01
@sgiehl sgiehl added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Dec 19, 2018
@mattab mattab modified the milestones: 3.8.0, 3.9.0 Dec 31, 2018
@@ -106,6 +106,18 @@ private function flattenRow
(Row $row, $rowId, DataTable $dataTable, $level, $dimensionName,
$labelPrefix = '', $parentLogo = false)
{
$dimensions = $dataTable->getMetadata('dimensions');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance to do this maybe in the HtmlTable visualisation or so? Be nice to have this feature contained in one class or so and not spread across core, visualisations, etc.

Copy link
Member

@diosmosis diosmosis Jan 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgiehl can you answer this comment when you have time? (same w/ above one)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if there is a better/easier way to get all dimensions that were flattened. The only thing I can think of would be to get the report's metadata and then iterate through all sub reports to get all the dimensions. But not sure if that would work correctly with custom reports.
Nevertheless I think it might make sense to always have the dimensions metadata for flattened reports, as each dimension also has it's own metadata property. So the content of dimensions metadata makes it easy to identify the available dimension metadata properties.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tsteur can you check @sgiehl's reply?

@tsteur
Copy link
Member

tsteur commented Jan 2, 2019

Can you add some tests for this? And ideally if possible also contain the code in one module or class. It looks otherwise bit fragile/hacked.

I've also tried to use it and saw eg
image

Not sure if it is normal to see a dash before others in Acquisition => Websites report? (maybe isn't related to this PR)

Also my understanding was we would not only always show it in two columns but maybe have an option or two different "flattening" features whether you want it grouped or not? Personally in most cases I wouldn't like this view and prefer it in one column.

@diosmosis
Copy link
Member

@sgiehl will you be able to update this PR?

@sgiehl sgiehl force-pushed the flattenviz branch 2 times, most recently from 4a679b0 to 22ae40a Compare January 28, 2019 12:58
@sgiehl
Copy link
Member Author

sgiehl commented May 28, 2019

in this report, dimensions are set as Country > City > Provider, but the report once flattened shows a different order: Country > Provider > City

should be fixed with the last commit. The display order of additional dimensions was reverse.

Could the feature be enabled for Behavior > Downloads report? And for Behavior > Outlinks? Not required though.

It is enabled for all reports where the dimension for the subtable differs. Otherwise it doesn't actually make much sense to split it up. Same applies to all pages reports.

@mattab
Copy link
Member

mattab commented Jun 10, 2019

@diosmosis can you please review this quickly and merge if it looks good to you?

@diosmosis diosmosis merged commit bba8bac into 3.x-dev Jun 11, 2019
@diosmosis diosmosis deleted the flattenviz branch June 11, 2019 03:47
@mattab mattab added Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. c: Design / UI For issues that impact Matomo's user interface or the design overall. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. labels Jun 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Design / UI For issues that impact Matomo's user interface or the design overall. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Major Indicates the severity or impact or benefit of an issue is much higher than normal but not critical. Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants