@bx80 opened this Pull Request on April 6th 2022 Contributor

Description:

Fixes #10716

This PR adds evolution percentages and up/down icons to the end of the sparklines shown on the overview screens for visits, goals and ecommerce.

Review

@bx80 commented on April 6th 2022 Contributor

Work in progress - visits and goals overviews are done, ecommerce renders sparklines a little differently and is not implemented yet.

@bx80 commented on April 11th 2022 Contributor

The Ecommerce overview spark lines have been implemented using a custom template rather than a visualization, probably to allow special handling for the abandoned cart goal metrics. There isn't enough time allocated for this issue to rework the overview to use a visualization, so instead I've chosen a pragmatic approach and rendered the evolution values in the custom spark line template. This isn't ideal but didn't take much time and will provide evolution metrics in one of the more useful places until things can be reworked at some future time.

In order for the override method Ecommerce.getMetricsForGoal() to gain access to the data row used in the parent Goals.getMetricsForGoal() I've added an optional parameter to the parent method to allow the preloaded data row to be passed by the child method. I thought this preferable to replicating the entire parent method, but open to better ideas! :slightly_smiling_face:

@github-actions[bot] commented on April 19th 2022 Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@sgiehl commented on May 3rd 2022 Member

@bx80 I've pushed an update to the bandwidth plugin and a small adjustment here, which should fix the unit problem of the byte metrics in the evolution chart. (Not yet sure, but maybe some tests need to be adjusted).
As discussed in the meeting ff you want feel free to have another look at the ecommerce sparklines. If you think it will take too long, feel free to postpone. Otherwise guess this PR should be almost finished.

@bx80 commented on May 18th 2022 Contributor

@sgiehl Thanks for fixing the bandwidth metrics, that's looking really good :smiley:
I don't think I'm going to have the time to rework the ecommerce sparklines for this release.
I've tidied up the remaining tests.

Not sure if you want to do a quick final review or shall I go ahead and merge?

@sgiehl commented on May 18th 2022 Member

@bx80 I have merge in the latest changed from 4.x-dev to see if any tests are still failing. But looking through the changed UI tests it seems like the numbers for the sparklines aren't formatted anymore when comparing data. Haven't looked in detail yet. I will wait for the tests to finish, maybe they only need to be updated 🤔

@sgiehl commented on May 18th 2022 Member

@bx80 Seems you need to have another look at that one. The number formatting seems to fail when comparing data. I was able to reproduce that locally.

@bx80 commented on May 19th 2022 Contributor

@sgiehl I'm not able to see where the number formatting is failing, the failed UI tests all seem to be caused by the action data tables not grouping and when I run this test locally they group correctly. I've also checked all the obvious places in the UI both in English and Deutsch (in case it's locale specific) but the sparkline tooltips number comparisons seem to be formatted correctly. I'm clearly missing something, where should I be looking? :eyes:

@sgiehl commented on May 19th 2022 Member

Are the average spent time and bounce rate metric formatted for you?

@bx80 commented on May 19th 2022 Contributor

Are the average spent time and bounce rate metric formatted for you?

Yes, they seem ok.
image
image
image

@sgiehl commented on May 20th 2022 Member

As mentioned before it seems only broken in comparison mode. So when comparing dates or segments.

@bx80 commented on May 21st 2022 Contributor

@sgiehl Sorry, I was thinking comparing via the tooltips not via comparison mode. :man_facepalming: I see the issue now and I've commit a fix to also the format metrics when the sparklines are in comparison mode.
image

@sgiehl commented on May 23rd 2022 Member

@bx80 with the fixes I've pushed, I think this should be good to merge now. Feel free to have another quick look at the changes and merge it if they look fine for you.
Regarding the ecommerce sparkline refactoring. Found this issue https://github.com/matomo-org/matomo/issues/15349, which we should handle that in.

Btw. before merging this one, you need to merge all related submodule PRs and update the submodules in this PR back to their 4.x-dev branch again. Otherwise we might get submodule reference issues....

This Pull Request was closed on May 24th 2022
Powered by GitHub Issue Mirror