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

Row Evolution: usability improvements, UX #3158

Closed
mattab opened this issue May 29, 2012 · 29 comments
Closed

Row Evolution: usability improvements, UX #3158

mattab opened this issue May 29, 2012 · 29 comments
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. worksforme The issue cannot be reproduced and things work as intended.
Milestone

Comments

@mattab
Copy link
Member

mattab commented May 29, 2012

This ticket is a follow up of #534 - the first version of Row evolution is part of Piwik 1.8.

There are items which I believe will make the feature easier to discover, easier to use, harder to misuse! here they are:

Usability

  • within the popover, text "Metrics for Page URL: photos.php" could show yellow logo + link to the page URL in a new tab. Similarly Referers Websites will link to the actual page URL.
  • On the Evolution Graph, clicking on a different date should reload the same report (Row evolution popover closed) for the clicked date
  • When the popover finishes loading, the browser should scroll smoothly to the top of popover, the same way we scroll when sub datatables are loaded.

Multi Row Evolution UX
Multi Row Evolution is an awesome feature but currently hard to discover, so here are UX suggestions:

  • When user presses the SHIFT key, other "Row Evolution" icons should be displayed, from the rows in the current mouse-focused data table. These icons would be clearly clickable. On releasing SHIFT, icons are hidden again.
    • When a row is "Multi-clicked" the icon stays visible/"enabled" for this row even if SHIFT is released.
  • Maybe update row evolution icon tooltip, append " (tip: press Shift to compare multiple rows)"
  • The Multi row evolution report is opened when user clicks on any "enabled" row evolution icon
    • if user "enables" 2 rows, then hovers on a new one and clicks on it, the Multi row evolution should open and compare these 3 rows together. So that comparing 3 rows takes only Shift+3 clicks. Nice!

In the multi row evolution report:

  • when selecting another metric, do not hide the current report. display "Loading data..." at the bottom and replace the current report when new one is loaded.

If you have any suggestion or find any bug with Row Evolution please report here!

See also:

@mattab
Copy link
Member Author

mattab commented May 31, 2012

(In [6410]) Refs #534, Refs #3158

  • Displaying logos when comparing multiple rows. It looks very nice for example comparing Firefox VS Chrome VS IE Vs Safari :)

@mattab
Copy link
Member Author

mattab commented May 31, 2012

See also

@julienmoumne
Copy link
Member

To fully implement #3013, API.getRowEvolution needs to support a new parameter : $idGoal.

This is required for Goal reports with dimension such as :

  • getVisitsUntilConversion
  • getDaysToConversion

Without this parameter it is not possible to plot, for example, the evolution of 0 days (to conversion) for eCommerce orders (&idGoal=ecommerceOrder).

@julienmoumne
Copy link
Member

It has been suggested in comment:5:ticket:3013 that static graphs should display the top 5 row evolutions.

The logic needed to get the top 5 row evolutions should maybe be located in the API.getRowEvolution method.

The $label parameter could become a non required parameter.

When no labels are not provided, the top 5 label evolutions would be returned.

It could even support keywords such as 'top5' and 'last5'.

@mattab
Copy link
Member Author

mattab commented Jun 24, 2012

The logic needed to get the top 5 row evolutions should maybe be located in the API.getRowEvolution method.

The $label parameter could become a non required parameter.
When no labels are not provided, the top 5 label evolutions would be returned.
It could even support keywords such as 'top5' and 'last5'.

Sounds good, in the code I propose to simply use the value of filter_limit - if it's specified use it, otherwise default to 5. So it is consistent with existing API.

top5 and last5 is an interesting idea, but I think a bit too early unless we make good use of such features in the UI :)

@mattab
Copy link
Member Author

mattab commented Jun 24, 2012

Replying to JulienM:

To fully implement #3013, API.getRowEvolution needs to support a new parameter : $idGoal.

+1

@julienmoumne
Copy link
Member

Replying to matt:

Sounds good, in the code I propose to simply use the value of filter_limit - if it's specified use it, otherwise default to 5. So it is consistent with existing API.

filter_limit gives you the top labels per period. If you look at Referers.getRetererType period=year date=last5 filter_limit=2 you can see that the top 2 labels are not the same every year.

Which top x labels should be taken? The ones from the last period? Or something more sophisticated involving averages over periods?

@mattab
Copy link
Member Author

mattab commented Jun 25, 2012

Keep the top labels for the most recent period (ie. current period) being compared. As part of #57 we will highlight which ones are decreasing and increasing, but for plotting values we simply use the current period.

If you look at Referers.getRetererType period=year date=last5 filter_limit=2 you can see that the top 2 labels are not the same every year.

That's correct, but for refererType example, the graph will always display all lines since there will be max 4 (direct,website,search,campaign)

@julienmoumne
Copy link
Member

refs #3158

  • $label is now optional, will return top N=filter_limit row evolutions sorted with filter_sort_column when omitted
  • support for $idGoal
  • integration tests updated

@mattab
Copy link
Member Author

mattab commented Sep 6, 2012

See #3361 Row Evolutions not returning data for Actions.get[Entry|Exit]PageTitles

@julienmoumne
Copy link
Member

(In [6925])
refs #3158

  • adding more report exceptions from being filtered by AddColumnProcessedMetrics filter
  • I forgot to manage an edge case when $label=false and there are no data, what should the API return ? see @review
  • only retrieve the last period when looking for top n labels

refs #3013

  • better error message when specifying invalid metrics in $columns
  • same image height for all graph types in all page orientations for pdf reports

@mattab
Copy link
Member Author

mattab commented Sep 7, 2012

Thanks for making the suggested changes! Nice commit.

  • I forgot to manage an edge case when $label=false and there are no data, what should the API return ? see @review

Indeed it's a good point.

When there is no labels in the "last date" report, or when there are less than 5 labels, I propose that we "top up" the labels list by requesting the report for the current year of data, and use this to fill in the 5 labels ?

(ie. the code calling $this->loadRowEvolutionDataFromAPI ) could be refactored to be called twice with Last date and Current year

this would ensure that, when there's a bug in the site for 2 days for example, the graph show that the top 5 values this year have all decreased a lot. If we don't use yearly data, the graphswill show "no data" when in fact there's an emergency situation ;-)

what do you think ?

@mattab
Copy link
Member Author

mattab commented Sep 26, 2012

(In [7064]) Refs #3158 #3227

  • Fixing encoding bugs. We never noticed but there was a bug in the code (see change in XML files which are now correct).
  • Display Between X and Y even when values are the same (otherwise it looks like there's a bug)
  • fixing other tests
    I'm SO glad we have nice tests coverage for this complicated code otherwise we would be totally screwed!

@julienmoumne
Copy link
Member

(In [7086])

refs #3158 refs #3013

  • remove metric names from metadata with new API parameter $legendAppendMetric == false (defaults to true)

refs #3158

@mattab
Copy link
Member Author

mattab commented Oct 7, 2012

(In [7116]) Refs #3158
Row evolution works when the label or sub label contains "script" - this used to be a smarty hack to make the JS widgets work, but we only support iframe now, so we can safely remove our overwrite

@timo-bes
Copy link
Member

(In [7179]) refs #534: bringing the new popover behavior to row evolution

  • the popover parameter is used to remember the current state of row evolution
  • Piwik_Popover is used for controlling the popover
  • ajax helper methods are refactored from Transitions to piwikHelper and reused in row evolution
  • simplified logic for remembering rows for multi row evolution
  • applied code formatter (hence the whitespace changes)

@timo-bes
Copy link
Member

(In [7182]) refs #3158: remember selected metric in multi row evolution as part of the url - using the back and forward buttons after picking different metrics is possible now

@julienmoumne
Copy link
Member

Replying to matt:

Thanks for making the suggested changes! Nice commit.

  • I forgot to manage an edge case when $label=false and there are no data, what should the API return ? see @review

Indeed it's a good point.

When there is no labels in the "last date" report, or when there are less than 5 labels, I propose that we "top up" the labels list by requesting the report for the current year of data, and use this to fill in the 5 labels ?

(ie. the code calling $this->loadRowEvolutionDataFromAPI ) could be refactored to be called twice with Last date and Current year

this would ensure that, when there's a bug in the site for 2 days for example, the graph show that the top 5 values this year have all decreased a lot. If we don't use yearly data, the graphswill show "no data" when in fact there's an emergency situation ;-)

what do you think ?

See 3790

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

Missing data: go to demo link. Reproduce: click on a domain name in the table, then click any Row evolution icon for a download -> the graph and sparklines display but there is no stats written on the right of sparklines.

This is a feature, not a bug. Matt, you didn't like that row evolution showed +100% if the period was longer than the time data was tracked for the current report. So we decided to ignore all values until the first non-zero value. In the report you link to, the rows have only zero values except for the current month. Since there is only one data point we consider, we don't show the change.

I hope I managed to express the point clearly... The question is whether we want to keep this "feature" or not. Your call, Matt.

Note to myself: To remove this, get rid of $firstNonZeroFound and if ($first == 0) in API_API::enhanceRowEvolutionMetaData().

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

In 221b74d: refs #3158 row evolution

  • bug fix: the export beneath row evolution graphs didn't work. the reason was that it passed an expanded=1 and a label parameter to the API. solution: ignore expanded parameter if label parameter is set.
  • small code improvements

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

metric selector graph: Would it be possible to add the "Metrics picker" in the graph in row evolution report?

Why would we want to do that? Row evolution has its own implementation of the metrics picker. In the regular view, you can click the metrics below the graph and in Multi-RowEvolution, there's the selectbox at the bottom (and in this case, the regular picker wouldn't work because we have to reload the data after changing the metric).

Can we remove this request?

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

Popover state in hash part is implemented.

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

Small detail: When report is Flattened, and label is long and truncated (eg. custom var name - value), hovering on the row to see the tooltip also displays a white spot where the Row evolution icon normally is, hidding some text

I can't reproduce this. Has it maybe been fixed by now? Matt, please double check.

@timo-bes
Copy link
Member

timo-bes commented Apr 8, 2013

Loading data... could be instead "Loading Evolution data for {here icon and label name}" this could be done by copying the HTML of Label row, including the icons/logos?

The problem is that we only have the label in the URL parameter. Since RE launched by changing the hash part of the URL, we don't have the data table row DOM el. What we have is the label - we could show that but it looks weird for subtables. E.g. when row evolution is openend on an outlink, it might look like "www.example.com > /".
We don't have the icon at all so there's no way to show it expept add it to the URL too which introduces XSS possibilities. We could only show it when RE has been launched from a table and pass the row dom el around but this would add complexity to the already complex code.

So I'm not sure what to do about this request. It would be nice, yes. But I don't see a decent way to implement it.

@mattab
Copy link
Member Author

mattab commented Apr 8, 2013

Removing 'add the "Metrics picker" in the graph in row evolution report' from description

@mattab
Copy link
Member Author

mattab commented Apr 8, 2013

Nice that you looked into these.

I removed them and updated and cleaned up the ticket description.

@timo-bes
Copy link
Member

timo-bes commented Apr 9, 2013

Thanks, Matt.

Why did you remove this from the description?

Rows across pages can't be compared. Reproduce: mark a row for Multi-RE, klick "next >" and open RE on another row. The selection will be lost because the marked rows are stored in the data table instance which is destroyed. Possible solution: store state in the data table manager. Clear the state when switching between reports.

@mattab
Copy link
Member Author

mattab commented Apr 10, 2013

I'm not sure if it's a good idea to allow multi pages selection. It could be easier to use the Bottom-right limit selector to show more rows, and easily select them at once.

@mattab mattab added this to the 2.x - The Great Piwik 2.x Backlog milestone Jul 8, 2014
@mattab mattab added the T: Task label Jul 8, 2014
@mattab mattab added the c: Usability For issues that let users achieve a defined goal more effectively or efficiently. label Oct 12, 2014
@mattab
Copy link
Member Author

mattab commented Mar 12, 2015

Closing this issue as Row evolution works well - for specific requests, we should create new separate issues 👍

@mattab mattab closed this as completed Mar 12, 2015
@mattab mattab added the worksforme The issue cannot be reproduced and things work as intended. label Mar 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. worksforme The issue cannot be reproduced and things work as intended.
Projects
None yet
Development

No branches or pull requests

3 participants