@sgiehl opened this Pull Request on April 3rd 2017 Member

Proposed changes to the way how visitor log and profile are created:

Introduces new classes (named VisitorDetails) that can be implemented in every plugin.
With this classes the visitor and action data used for the Live plugin can be provided, filtered and manipulated. Additionally the classes can define extra content to be rendered in visitor log & profile

Additional changes / improvements

  • Actions for visits are now queried as bulk requests, instead of for each visit seperatly
  • Actions of a visit in Visitor Log are hidden by default but can be expanded on click
  • Show Browser Language in Country Flag Tooltip (refs #11707)
  • Display Content Impressions & Interactions in Visitor Log & Profile (fixes #6416 refs #6268)
  • Display a overview of used devices in Visitor Profile (refs #6161)
  • Show visit detail icons in visit list of user profile (fixes #11520)
  • Show top visited pages in Visitor Profile (fixes #8912)
  • Do not group duplicate page views, but show them with a refresh icon instead, to prevent loss of additional information (fixes #9586)
  • Always show visitor profile link in visitor log (fixes #10272)
  • Display custom Dimensions in Visitor Log & Profile (refs piwik/plugin-CustomDimensions#28, piwik/plugin-CustomDimensions#61)
  • Display detailed campaign data in Visitor Log & Profile (refs piwik/plugin-MarketingCampaignsReporting#3, piwik/plugin-MarketingCampaignsReporting#11)
  • Show bandwidth data in Visitor Log & Profile (piwik/plugin-Bandwidth#13)
  • Reverse enumeration of visits in visitor profile (fixes #8491)
  • Show idvisit in the visitor log and user profile (IP tooltip)
  • Always return raw referrer url for visitor details (fixes #11976)

fixes #6111
refs #9507

layout changes also fixes #11589

@tsteur commented on May 4th 2017 Owner

FYI: An issue I ran a few times into when extending the visitor logs eg via event or so is that most of the events are methods / hooks are triggered per visitor. When showing like 250 or up to 500 visits on one page, we might do 500 times a SQL query for each plugin that hooks into. For example plugins would check if there is a custom dimension, or media views, form interactions, recorded sessions, etc. They are all in different tables etc and query per visitor.

It would be great if there was like an event or method / hook that gives you the row of visitors so it is more efficiently possible to fetch in one query the data (eg using idvisit in (....)) instead of having heaps of individual queries, then render it later individually. I haven't looked much into PR or the other PRs if something like this is already implemented but be good to have.

@mattab commented on May 9th 2017 Owner

This looks promising :+1:

  • What are your thoughts regarding Thomas comment on performance?
  • if possible, It would be great to include in this PR the refactoring of all template code into relevant plugins. This could validate the new APIs and finalise the project.

We might need to move this into 3.1.0 release as I assume it won't be finished in time for 3.0.4?

@mattab commented on July 3rd 2017 Owner

Note:

@mattab commented on July 21st 2017 Owner

(Haven't reviewed yet, but right now I would need to see the idvisit in the visitor log and seems it's not displayed yet?)

  • Show idvisit in the visitor log and user profile in a tooltip or so
@sgiehl commented on July 22nd 2017 Member

Show idvisit in the visitor log and user profile in a tooltip or so

What would that information be good for? We don't have any API that takes that id as argument...

@mattab commented on July 22nd 2017 Owner

We have a segment for idVisit and it is useful for debugging

@sgiehl commented on July 23rd 2017 Member

Will be shown in IP tooltip

@mattab commented on July 24th 2017 Owner

Feedback

  • Add some padding in the refresh icon so the border is further away from the icon.
  • add spacing to the left of the number (currently it's too close to the icon), maybe consistent spacing as the Goal conversion count?
  • In the action list, for goal conversions, the text on a same line should vertically aligned. Currently the text "Revenue" is displayed few pixels too high. See for example https://media.githubusercontent.com/media/piwik/piwik/84c1be61dc2165b97a7250c16ae6f04d171ca73d/tests/UI/expected-screenshots/UIIntegrationTest_visitors_with_site_search_visitorlog.png (Visit 1, action 2 and 3)
  • "Actions of a visit in Visitor Log are hidden by default but can be expanded on click". In the User Profile a link is "Show X actions". Instead, show all actions by default (this as it's one of the most useful and important parts).
  • Only when there are more than 500 actions, the link could let users show all actions. Btw: clicking on the link does not work for me. It closes the popover and show an empty report page.
  • is there an icon we could use in our icon set, to replace the old "Returning visit" icon?
  • is there any potential that this PR could regress performance of visitor log or visitor profile? For example running more SQL queries, or new queries?
  • In the fixture for the visitor log and user profile UI tests, add the following cases, so that all the visualisation are tested:
    • Content Tracking requests.
    • Custom dimensions requests.
    • A visitor who used several devices
    • Any other data point that is not yet shown in UI tests
  • Commented core in Contents/VisitorDetails.php could be removed?
  • Custom Dimensions are not part of this PR but should be?
  • New term "Revisited pages", instead could we use "Pages viewed more than once"
  • I'm worried about " Actions for visits are now queried as bulk requests, instead of for each visit seperatly" because it could make the page load actually slower. We need to test this carefully (on demo2) and be ready to revert if needed, as soon as PR is merged.
@sgiehl commented on July 24th 2017 Member
  • Add some padding in the refresh icon so the border is further away from the icon.

Added one more pixel padding

  • add spacing to the left of the number (currently it's too close to the icon), maybe consistent spacing as the Goal conversion count?

Added 2 pixel padding

Not able to reproduce that locally. For me the text is on one line in Chrome and Firefox. Maybe a font rendering issue for phantom. Can you reproduce that in your browser?

  • "Actions of a visit in Visitor Log are hidden by default but can be expanded on click". In the User Profile a link is "Show X actions". Instead, show all actions by default (this as it's one of the most useful and important parts).

No. I would disagree here. It is not useful nor important to see all actions when opening the profile. The Overview should give you the most important information. If not, something is missing there. Besides that it's useful to see the different visits. Having all actions expanded by default would make the profile very long and you need to scroll very very long in some cases to get to the next visits.

  • Only when there are more than 500 actions, the link could let users show all actions. Btw: clicking on the link does not work for me. It closes the popover and show an empty report page.

The link works for me on Chrome and Firefox. Where did you test? Maybe the assets haven't been rebuilt?

  • is there an icon we could use in our icon set, to replace the old "Returning visit" icon?

Don't thinks so. Let's replace later if we have a good one.

  • is there any potential that this PR could regress performance of visitor log or visitor profile? For example running more SQL queries, or new queries?

Yes, there are new and changed queries. Also a lot more information might be displayed.

  • In the fixture for the visitor log and user profile UI tests, add the following cases, so that all the visualisation are tested:
    • Content Tracking requests.
    • Custom dimensions requests.
    • A visitor who used several devices
    • Any other data point that is not yet shown in UI tests

Need to create some new fixtures for that. And Custom Dimensions need to be tested in the plugin itself. Will do that later.

  • Commented core in Contents/VisitorDetails.php could be removed?

Prepared some new stuff there. But decided to add that later. So will remove it for now.

  • Custom Dimensions are not part of this PR but should be?

No. Custom Dimensions are handled in the plugin itself. But the submodule should contain it in this branch. See https://github.com/piwik/plugin-CustomDimensions/pull/61

  • New term "Revisited pages", instead could we use "Pages viewed more than once"

Changed.

  • I'm worried about " Actions for visits are now queried as bulk requests, instead of for each visit seperatly" because it could make the page load actually slower. We need to test this carefully (on demo2) and be ready to revert if needed, as soon as PR is merged.

Sure, we need to check that. But having one query instead of many should be faster in most cases. Especially when joining multiple tables.

@mattab commented on July 24th 2017 Owner
  • It's very important to show all actions in both visitor log and user profile 100%. Not negotiable!
  • Thanks for making the change also to enhance our fixtures so all is tested and shown in UI tests

Looking forward to merge the PR! Nice refactor

@sgiehl commented on July 24th 2017 Member

It's very important to show all actions in both visitor log and user profile 100%. Not negotiable!

All actions can be shown, they are only hidden by default to save some space and be able to show an overview over all visits with their details (icons). Showing all actions by default will remove that benefit completely.

@mattab commented on July 24th 2017 Owner

If the idea is to save space, let's add a button or icon to collapse all
actions on click. by default need to show all actions as it is possible
purpose of the right panel to make it easy to see all without having to
click once or 10 times, and make a consistent view in both visitor log and
profile

@sgiehl commented on July 24th 2017 Member

@mattab It's not only about saving space. Imho the visitor profile should give the viewer a rough overview for the visitor (with the possibility to get more details). Getting overloaded with dozens of actions listed on the right wouldn't give a better user experience I think. It would make it really hard to see how many visits are listed and to find a specific one. Viewing a visitor with many visits with hundreds of action would be a pain and simply unusable.
Feel free to explain your point of view. I simply don't see a use case where it would make sense to have it expanded by default. (Adding a button to toggle all would make sense - even if I don't where to place it)

@mattab commented on July 24th 2017 Owner

Agreed that the visitor profile is meant to provide an overview of all visits and important user information, but this overview is actually restricted to the left panel. The left panel giving an overview has been improved by this PR which is good.

However the right panel is meant not as an overview, but is meant to show the detailed view of all visits and actions by this visitor at a glance. And it is also meant as a continuity of the visitor log (same visual design and UX): It is important to maintain consistency between visitor log and user profile because users want to see this same information, but for a particular user, which the visitor log does not. Users are able to browse through the list of visits and actions visually and analyse it.

It would make it really hard to see how many visits are listed and to find a specific one.

The number of total visits is already displayed in the left panel. So this information is already present.

But finding a specific visit is not easily done at the moment and it's definitely a use case to address.

For the design:

  • we could add a "Red minimise/maximise icon" next to the red close cross icon. This red icon would collapse actions across all visits.
  • for more flexibility, we also could show a minimise/maximise icon on hover on a particular visit, next to the date in the grey block. This way one could collapse all visits, and then expand one particular visit they're interested in. Would be awesome useful when there are like dozens of visits but we only interested in 1 or 2 or 3 of them.

Once a user has 'minimised' actions (whether globally or in a visit), the icon would change to the 'maximise' icon so they can again show the actions. The icon would be the same as the one we use already in the widgets in the dashboard: icons

Thoughts? Would be great to have in this release, but we can do later also as we need to cut the RC today or tomorrow ideally

@sgiehl commented on July 25th 2017 Member

Added a new button to toggle all actions and they are now shown by default. See screenshot tests to see how it looks now: https://github.com/piwik/piwik/tree/visitordetails/plugins/Live/tests/UI/expected-ui-screenshots

@mattab commented on July 26th 2017 Owner

Almost there. Deployed the branch on demo2 and made final tests.

Feedback and improvements

  • [x] Make "hide N actions" not bold.
  • [x] make the time of visit next to the Visit #x not bold.
  • [x] remove the bold on the visitor ID
  • [x] remove bold on visitor IP
  • [x] Remove any other bold added i've missed. Bold is only for Goals and Ecommerce orders
  • [x] Re-add the IP address of the last visit on the top left panel as it was before
  • [x] I would also re-add the country, device icon, etc. of the last visit in the top left panel, along with the label Firefox, Win 7, etc. as it was looking better before and was an important part of the "Visitor profile". Along with the refresh of data in the top left panel when the visit is clicked (as before)
  • [x] Maybe would move the row of country / device / into the "More visit details" expanded panel. 95% of the time all the same icons would be displayed in that row which does not add value and take space and mental energy to scan...
  • [x] in the more visit panel, display from Google instead of just Google (and remove the bold)
  • [x] is #9586 really fixed? haven't noticed a functional change related to this
  • [x] Most devices are tracked as "Unkown". instead of "3 visits from Desktop devices: Unknown (3x)" write "3 visits from Desktop devices" (ie. no info is provided by unknown so let's remove this noise)
  • [x] And display the device names (when not unknown) on the same line rather than a new line
  • [x] Top visited pages should link to the pages, re-using same macro code as the right panel (ie. we remove protocol and check for xss)
  • [x] And let's remove the border around the refresh icon. I noticed it looks simpler/better without border
  • [x] Move the "First/Last visit" section above the "Top visited pages"
  • [x] The tooltip is missing on the "Top visited pages" icon. it should be consistent and reuse code from the right panel refresh pages icons
  • [x] Remove the Custom variables and other such descriptors in visitor log and user profile, as it would be often repeated in each visit without added value. Let's display those descriptor names in the tooltip only of each metadata instead.
  • [x] on the country flag the tooltip says Browser language: Language code: XX but should say Browser language: YY \n Language code: YY
  • [x] is there a screenshot test showing Custom dimension of scope visit?
  • [x] in some cases (example) Top visited pages does not show any page. Why?
  • [x] below "top Visited pages", when unique page views == page views, only display pageviews
  • [x] Performance: using this branch, loading the visitor log on demo2 for 500 visits takes between 55seconds and 70 seconds. Using 3.x-dev, it takes 5 seconds. The new code is 10 times slower, so let's revert all SQL changes and keep same SQL logic as before (load all visits at once)

This branch is deployed on demo2 for now

@sgiehl commented on July 26th 2017 Member
  • Re-add the IP address of the last visit on the top left panel as it was before
  • I would also re-add the country, device icon, etc. of the last visit in the top left panel, along with the label Firefox, Win 7, etc. as it was looking better before and was an important part of the "Visitor profile". Along with the refresh of data in the top left panel when the visit is clicked (as before)

As discussed before, this part of the visitor profile should give an overview. I don't know how data from only the last visit should give an overview over anything. That's somehow inconsistent. Also those information was only visible at that point before, now it can be viewed within the visits list. Maybe there is some other data that would make more sense to add there.

Maybe would move the row of country / device / into the "More visit details" expanded panel. 95% of the time all the same icons would be displayed in that row which does not add value and take space and mental energy to scan...

I would disagree here. When collapsing all actions, showing those icons gives a nice overview over the visits, like which of them converted a goal. For customers using cross device tracking it might be of value to directly see which visits were made on which device...

in the more visit panel, display from Google instead of just Google

This is the same as displayed in visitor log and it wasn't changed in any way. It should show Google Keywords: keyword. Should it be from Google Keywords: keyword?

is #9586 really fixed? haven't noticed a functional change related to this

Yes. The comparison is now done in JavaScript. Only if the complete html is the same as of the element before they will be shown together.

on the country flag the tooltip says Browser language: Language code: XX but should say Browser language: YY \n Language code: YY

No. Language code XX is displayed if the language code can't be transformed to a language name. That is the way how it is handled in the browser language report as well.

in some cases (example) Top visited pages does not show any page. Why?

Only pages visited more than once are displayed in a list. If each page was viewed only once, there will be no list. (Also only Top 5 pages will be shown.)

Performance: using this branch, loading the visitor log on demo2 for 500 visits takes between 55seconds and 70 seconds. Using 3.x-dev, it takes 5 seconds. The new code is 10 times slower, so let's revert all SQL changes and keep same SQL logic as before (load all visits at once)

Might not necessarily help. The SQL queries to get actions are now more complex as also content actions are tried to fetch (which is a join more), also much more information is now fetched (custom variables and dimensions)

@mattab commented on July 26th 2017 Owner

Performance problem is fixed :rocket:

As discussed before, this part of the visitor profile should give an overview. I don't know how data from only the last visit should give an overview over anything.

an overview is defined as a short description of something that provides general information about it, but no details. Since in 99% of the time a user will use same device, same geo location, custom dimensions, showing those items is indeed a valuable general information and creates a valuable overview about the user. it's not in this case about being 100% accurate but providing this general overview at a glance

Also those information was only visible at that point before, now it can be viewed within the visits list. Maybe there is some other data that would make more sense to add there.

In the user profile, users expect the overview to include IP addresses and other key info (last software used, custom dimensions, last device...). From experience people love this report and information and we shouldn't break what is loved. Note that the most important data to display on the top left which wasn't there before (and is the first reason we started improving the profile again) is "Custom Dimensions of Visit Scope" as they are key visit data that people want to see on the user profile without having to click on last visit.

When collapsing all actions, showing those icons gives a nice overview over the visits, like which of them converted a goal.

Agreed with the goal conversion, this one is useful. Same would be to show the ecommerce icon, is it already done with ecommerce orders/abandoned carts?

On the other hand the other icons are not useful. The returning visit icon will always be there, except in the first visit. it does not provide value to show this returning visit icon in the user profile (while it does provide value in the visitor log). regarding the OS/software/device icons. Only a very small number of users would be tracked cross devices, as it's rare occurrence and most people don't use User ID. Even when they do, it's not common (in most cases) for users to login cross devices. So the icons are in 99% of the time not useful, and the same, repeated over and over, causing fatigue and mental energy for no benefit to users. It would be great to show them in the Visit box which expands as this is when user wants to see more information that they click on this box.

This is the same as displayed in visitor log and it wasn't changed in any way. It should show Google Keywords: keyword. Should it be from Google Keywords: keyword?

from Google, Keywords: keyword sounds good (only show keywords if it is actually defined). we could also add the from in the visitor log to make it consistent

Language code XX is displayed if the language code can't be transformed to a language name. That is the way how it is handled in the browser language report as well.

Ok in that case let's simplify the output and write only "Browser Language code XY" if the language name is not known

also much more information is now fetched (custom variables and dimensions)

note that selecting new columns typically doesn't cause any significant performance overhead

Pending feedback

(maybe forgot to repeat some items but hopefully OK)

  • [x] Most devices are tracked as "Unkown". instead of "3 visits from Desktop devices: Unknown (3x)" write "3 visits from Desktop devices" (ie. no info is provided by unknown so let's remove this noise)
  • [x] And display the device names (when not unknown) on the same line rather than a new line
  • [x] Show Visitor ID not in bold
  • [ ] In each visit on the right, remove icon browser, OS, country icon, returning visit icon, in each visit
  • [ ] and only show the Goal icon + Ecommerce icon at the top (both abandoned carts and orders) + possibly the device icon (device icon would give a way to see cross device visits with just one icon)
  • [ ] show these OS + browser + country icon+ their corresponding short labels in the top left (for the last visit)
  • [x] when there are no "Pages visited more than once" then hide the section "Top visited pages" since it does not provide value but takes valuable space
  • [ ] write "Browser Language code XY" if the language name is not known (instead of Browser language: Language code )
  • [ ] show custom dimension of the last visit in the top left panel
  • [ ] show custom dimension in the UI tests for both visitor log and profile, if not already done
@sgiehl commented on July 29th 2017 Member

from Google, Keywords: keyword sounds good (only show keywords if it is actually defined). we could also add the from in the visitor log to make it consistent

It seems there used to be the search engine icon in front of the search engine name, but it wasn't displayed anymore. Showing the icons would maybe make sense, but it doesn't look nice if there is the from as well. So shall we remove the from again, as the icon is now displayed again?:
image

In each visit on the right, remove icon browser, OS, country icon, returning visit icon, in each visit
and only show the Goal icon + Ecommerce icon at the top (both abandoned carts and orders) + possibly the device icon (device icon would give a way to see cross device visits with just one icon)

I had to change some parts to make it possible to extend the shown logos and to reuse the same templates for visitor log and profile, so I would prefer not to start using different views again. I've now hidden the "Returning Visitor" icon using css, as it really doesn't have much value there. Showing the other logos still makes sense to me even if they are the same in most cases.

show these OS + browser + country icon+ their corresponding short labels in the top left (for the last visit)

Added the icons for the first visit, but the are showing the same as currently on the right (but with hidden goal/ecommerce), showing different stuff here would make it impossible to show additional custom icons

write "Browser Language code XY" if the language name is not known (instead of Browser language: Language code)

Does the duplication of "Language" hurt so much? To change that I would need to add a new language string and a new method to return return and translate it in another way than the reports use it. Isn't imho worth the effort

show custom dimension of the last visit in the top left panel

There is too less space to show that. We already had much more information in that area before and had various issues with overlapping content.
But we could add a custom dimension summary, showing all dimensions and values used.

is #9586 really fixed? haven't noticed a functional change related to this

I just had another thought on it. Actually the tooltips of two entries are different almost always, as stuff like generation time and time on page isn't the same.
So guess we need to decide if we want to loose those additional information by grouping them, or always show every entrie (without grouping similar).
Another option would maybe be to group them, but implement an option to show all entries. Thoughts?

@mattab commented on August 3rd 2017 Owner

So shall we remove the from again, as the icon is now displayed again?

yes i guess the from could be removed as it would be quite clear to most people that this is the referrer?

Does the duplication of "Language" hurt so much? To change that I would need to add a new language string and a new method to return return and translate it in another way than the reports use it. Isn't imho worth the effort

No it's fine, was just a small detail compared to the other pending items

is <a href='/9586'>#9586</a> really fixed? haven't noticed a functional change related to this

I just had another thought on it. Actually the tooltips of two entries are different almost always, as stuff like generation time and time on page isn't the same. So guess we need to decide if we want to loose those additional information by grouping them, or always show every entrie (without grouping similar). Another option would maybe be to group them, but implement an option to show all entries. Thoughts?

Let's skip this and not mark #9586 as fixed. other pending items are much more important and are blocking release :+1:

Showing the other logos still makes sense to me even if they are the same in most cases.

I'm not convinced. Why repeat all these little icons when they are always the same? This provide no value at all and takes valuable mental energy, and this doesn't look as clean as it could.

Typical look would be:

repeated icons

Pending for release

  • [x] show custom dimension of the last visit in top left panel, or a summary of all custom dimensions values in the last 10 visits in the left in a new section as you suggested
  • [x] show custom dimension in the UI tests for both visitor log and profile
  • [x] In each visit on the right, remove icon browser, OS, country icon, and instead show these OS + browser + country icon+ their corresponding short labels in the top left (for the last visit). The summary on the left already gives this information nicely. (Then the extra line is focused on displaying Goals + Ecommerce + show count of actions as text, all key information which differ in each visit)
  • [x] show Ecommerce icon at the top of each visit (both abandoned carts and orders)
@sgiehl commented on August 7th 2017 Member

Summary of Custom Dimensions would currently look like this:
image
Or do you think it would make more sense only to show dimensions of scope visit?

In each visit on the right, remove icon browser, OS, country icon, and instead show these OS + browser + country icon+ their corresponding short labels in the top left (for the last visit). The summary on the left already gives this information nicely. (Then the extra line is focused on displaying Goals + Ecommerce + show count of actions as text, all key information which differ in each visit)

Ok. I tried to find a good compromise. All Icons besides the ecommerce/goal icons are hidden by default and will show on hover only.

@mattab commented on August 8th 2017 Owner

General feedback:

  • [x] Currently the order is Summary, Bandwidth, First/Last visit, Top Visited pages, Devices, Ecommerce, Custom Dimensions, Location. Instead let's move most important sections at top in this order: Summary, Custom Dimensions, Ecommerce, First/Last visit, Top Visited pages, Devices, Bandwidth, Location
  • [x] Grey line below the "Hide X actions" can be removed
  • [ ] Still feeling this whole row with "hide X actions" is not needed and would be better removed completely as it was before.
  • [ ] If we keep it for now, we need to find a solution for spacing. eg. Make this row "Hide X actions" less high and then add spacing at the end of the list of actions, so that spacing looks more consistent before/after the action list. A bit like this below. Maybe can find an even better spacing so it looks good when showing > 1 visit... Ideally, we would move this list of icons into the grey panel on click on a visit as it takes one row for each visit for no real benefit IMHO... The only real benefit is showing the number of actions in the visit, but we could find another solution for showing this metric (ie. in the Visit #x title bar or so)

spacing

  • [x] Make the "Export this dataset" icon bottom aligned with the visitor ID text. noticed it looks good in the screenshots tests, so maybe only an issue in Firefox? It looks like this in FF:

not aligned

  • [x] Below "Top visited pages" the tooltip on hover on the refresh icon is wrong, as in that case the refresh icon does not mean "Number of times where the page was refreshed in a row". So let's replace tooltip with the description of the actual meaning eg. Number of times this page was seen in all visits by this visitor or so?
  • [x] Regarding "Top visited pages" : I would completely remove the text " 27 Pageviews (11 Unique Pageviews, 5 Pages viewed more than once) " and then replace the title "Top visited pages" by "Pages viewed more than once" to keep it simpler and easier to read. This more detailed info could be shown instead on hover on the X pageviews in the Summary section.

Custom dimension related:

  • [x] display "Scope visit" first (before action)
  • [x] We want to highlight the custom dimension values. To put less emphasis on the 1x/2x/3x, let's move this information to the tooltip on hover on the custom dimension. value. so what's left is only the custom dimension name and value.
  • [x] Order them by frequency desc if not already done
  • [x] In the tooltip on hover on each Action in the right panel and visitor log, show the Custom Dimension section right after Time on page/ Before the Custom Variables (as custom dimensions are more important than these other metadata)
  • [x] Let's change "Custom Dimensions" title to "Custom Dimensions (Visit)" title
  • [x] and remove the sub-headers and then add a new section "Custom Dimensions (Action)" header. as a result we have one less header and header level, and more readable summary.
  • [x] When displaying custom dimension name and value, switch the colors: Value in black, Name in grey (more emphasis on the value)
  • [x] In the Visitor log, let's not display "No value" for a custom dimension when there was no value set (it does not bring added value).
  • [x] Also hide the "no value" dimensions in the tooltip on each action (imagine there are 10 or 20 dimensions it would be un-readable so we only show the actual dimensions with value)
@sgiehl commented on August 8th 2017 Member

Still feeling this whole row with "hide X actions" is not needed and would be better removed completely as it was before.
If we keep it for now, we need to find a solution for spacing. eg. Make this row "Hide X actions" less high and then add spacing at the end of the list of actions, so that spacing looks more consistent before/after the action list. A bit like this below. Maybe can find an even better spacing so it looks good when showing > 1 visit... Ideally, we would move this list of icons into the grey panel on click on a visit as it takes one row for each visit for no real benefit IMHO... The only real benefit is showing the number of actions in the visit, but we could find another solution for showing this metric (ie. in the Visit #x title bar or so)

Sure, it looks empty now for rows without goals or ecommerce. But that's because the other icons are hidden now. But this space showing icons will be very helpful for plugins. Session-Recording plugin for example could show an icon for each visit a session was recorded (with link to it), Form plugin could show an icon if forms were converted, A/B testing could show an icon representing the test group...

All other suggestions have been updated/fixed.

@mattab commented on August 9th 2017 Owner

@sgiehl On demo2 running this branch getting an error when loading user profile:


                    <p>
                The following error just broke Piwik (v3.0.5-b2):
                <pre>Key &quot;visit&quot; for array with keys &quot;action&quot; does not exist.</pre
>
                in
                <pre>/storage/piwik-demo2/www/demo2.piwik.org/plugins/CustomDimensions/templates/_profileSummary
.twig line 17</pre>
@sgiehl commented on August 9th 2017 Member

@mattab should be fixed

@mattab commented on August 10th 2017 Owner

Feedback

Getting there. Hopefully after this set of feedback we can release :+1:

  • [x] When there is an ecommerce conversion, but no goal, currently it shows "Goal 0 [ecommerce-icon]" (see screenshot below) but instead let's only display "[ecommerce-icon] Purchased" (without the Goal 0, and adding "Purchased")
    ecommerce

  • [ ] Maybe not needed, but I would remove the border around the Goals/Ecommerce icon in this right panel

  • [x] Spacing looks wrong, see screenshot below.
    • [x] add some spacing after the last action of each visit, margin-bottom: 2em; seems OK
    • [x] make that first icons-line less high. currently the padding:10px makes the line very high but most of the time it's empty. Something like padding 5px seems better + then also needed to make the icons vertical aligned in the space (as changing to padding 5px causes icons to be stuck to the top line)

actions

  • [x] In the First/Last visit section, when referrer is a website, make the domain clickable to the actual page. Will be helpful to directly see where user came from.
  • [ ] noticed there is a double border in the middle line. Would be nice if we could make that a single border somehow (currently the left section sets a border-right and the right list of visits sets aborder-left causing the double border only in some places.) see the partial double border in screenshot:

double border

  • [x] Custom Variables are displayed in the visitor log, but not in User Profile. Let's display the custom variables in the User Profile using same mechanism as the Custom Dimensions (Visit) section. Could also display the Custom Variables (Action) similarly. Check this example segment which has data in custom variables visit scope
  • [x] When a User ID is set, write the User ID in the same font as the "Visitor Profile" title. Currently user id is displayed small. It looks good when the user id is displayed on same line, but same big font. see screenshot of current look

visitor profile

@sgiehl commented on August 10th 2017 Member

noticed there is a double border in the middle line. Would be nice if we could make that a single border somehow (currently the left section sets a border-right and the right list of visits sets aborder-left causing the double border only in some places.) see the partial double border in screenshot:

This kind of needs to be as both parts have a variable height and can be higher as the other. and in order to have border from top to bottom, both parts needs a border. (btw. that was the same before)
Also I'm not able to reproduce on Chrome or Firefox, so it's hard to fix. Could you try if it helps for you setting width of .visitor-profile-overview and .visitor-profile-visits-info to 50% each?

@sgiehl commented on August 10th 2017 Member

Will fix the remaining tests tomorrow. Everything else should be updated

@mattab commented on August 15th 2017 Owner
  • "add some spacing after the last action of each visit, margin-bottom: 2em; " still not seeing a margin bottom after last action of each visit in right panel?
  • In the title when showing user ID, do not truncate, show the user ID in full in the title

Once those two are fixed :+1: to merge this and cut the next beta and then RC :rocket:

@sgiehl commented on August 15th 2017 Member

"add some spacing after the last action of each visit, margin-bottom: 2em; " still not seeing a margin bottom after last action of each visit in right panel?

Already added those 2px you suggested, but maybe it's just to small that it's visible. can add 2px more

In the title when showing user ID, do not truncate, show the user ID in full in the title

Imho doesn't look better when the line wraps and in this case the text is still quite short. UserId can have up to 200 chars, which would break the layout.
image

We maybe could remove the text "Visitor profile" if there is a UserId, so we have one full line to show a part of the UserId

@mattab commented on August 16th 2017 Owner

"add some spacing after the last action of each visit, margin-bottom: 2em; " still not seeing a margin bottom after last action of each visit in right panel?
Already added those 2px you suggested, but maybe it's just to small that it's visible. can add 2px more

Suggested 2em not 2px, which looks quite good but needs to be tweaked a bit

In the title when showing user ID, do not truncate, show the user ID in full in the title
Imho doesn't look better when the line wraps and in this case the text is still quite short. UserId can have up to 200 chars, which would break the layout.
We maybe could remove the text "Visitor profile" if there is a UserId, so we have one full line to show a part of the UserId

OK for both at same time: show only User ID when set (remove the title), and also make it wrapped so the full value displays (it still looks good when wrapping).

@sgiehl commented on August 16th 2017 Member

@mattab changed the remaining things. If it's fine now, I would update the screenshot tests and merge afterwards tomorrow.
As soon as this is done, we should release 3.0.5-b3 and afterwards release new plugin versions for CustomDimensions and Bandwidth with this version as minimum requirement. Already prepared those version changes on the branches...

@mattab commented on August 16th 2017 Owner

We reviewed with Thomas and have some more requests:
Feedback:

  • The visitor profile is now a bit overloaded and harder to get a summary view:

    • Remove "Pages visited more than once" (if possible we can release it as a new plugin on the Marketplace to showcase how to extend the user profile)
    • Remove "Custom Dimensions (Action)" section (if possible, move it to a plugin as well?)
  • Is it possible to vertical align the icons in the Actions list, as you can see below the icons are not perfectly middle aligned:
    icons not aligned

  • Padding on the right of the Event icon is too large, we should try to have the consistent padding on the sides of all icons:
    gaps

  • First visit is marked as Visit <a href='/0'>#0</a> but should be Visit <a href='/1'>#1</a>
  • Let's move the visit duration to the second line, And change Hide X Actions to X actions in Ymin Zs, And could we make the link blue to clarify it is a link?

move visit duration to second line

  • In the goal count in each visit, remove the # character and just display icon and count (less cluttered)

  • For the goal icon, can you use the Font-icon so the icon looks the same size as the text next to it? Currently the Goal icon (and possibly others) are bigger compared to the text/font next to it
  • Remove the seconds part of the Date time in each visit (less details)
    remove the seconds in the visit datetime

  • We need to make left/right margins consistent across the right panel. In the screenshot below we have
    • made the Visit <a href='/1'>#1</a> date_time left and right spacing tiny bit less
    • made the icons rows left spacing tiny bit more (so it's aligned) and the right spacing (after X actions in YminZs) tiny bit more
    • made the Actions list left spacing tiny bit more, so everything is nicely aligned and left+right spacing are consistent

aligned icons

@sgiehl commented on August 22nd 2017 Member

@mattab I've now done a complete CSS rewrite of the visitor profile, as the existing code was kind of "grown badly". There have been so many !important rules, which shouldn't be needed at all.
While doing the rewrite I tried to make everything more consistent, like spacing and font size.

First visit is marked as Visit #0 but should be Visit #1

Can't reproduce that. Can you provide a link where that happens?

Remove the seconds part of the Date time in each visit (less details)

No, that shouldn't be changed. We currently use various predefined date formats and the currently used one is DATE_FORMAT_LONG. Those formats including time automatically use the time formats for 12 or 24 hours, but always include the seconds. To change that, we need to change/extend those date methods and import other time formats as well. Don't think it's worth the effort right now.

For the goal icon, can you use the Font-icon so the icon looks the same size as the text next to it? Currently the Goal icon (and possibly others) are bigger compared to the text/font next to it

Tried to improve it a bit. But there is no font icon that looks the same. But it would be nice if all action icons (event, goal, ecommerce, search, outlink,...) would be included in the font. But I'm not able to do that. Maybe we could create a new issue to move those icons to the font and replace them in visitor log/profile.

The visitor profile is now a bit overloaded and harder to get a summary view:

Moving some stuff to plugins would be possible, but I'm currently thinking of another way how to solve that. Will try to work on it this week.

@mattab commented on August 25th 2017 Owner

Spacing looks much better now! :+1:

What's your idea regarding the following two points?

  • Remove "Pages visited more than once" (if possible we can release it as a new plugin on the Marketplace to showcase how to extend the user profile)
  • Remove "Custom Dimensions (Action)" section and "Custom Variables (Actions)" (if possible, move it to a plugin as well?)

IMHO it's ready to release after removing those two.

@mattab commented on August 31st 2017 Owner

We reviewed with Thomas and here is additional feedback:

  • Is it possible to left-align the text even when icons may be different sizes here?
    pasted_image_at_2017_08_31_01_56_pm
  • also make the font size smaller (the same as the default font size in the visitor profile)
  • Should add some more spacing on the left/right. we reckon adding 5px on each side might be enough, then 3px above and below the "goal number / date" and "Visit #4" maybe 3px smaller. (also possibly add 1 px above and/or below each action as it is hard to separate them and to read because they are quite close to each other.) Like this:
    pasted_image_at_2017_08_31_02_12_pm

  • Speed: over many tests on demo2, it takes 1.5s for server to generate response for 100 visits on 3.x-dev, while it takes server 10s to load 100 visits on the branch. New code is about 5 to 10 times as slow as before. Had you reverted the mysql logic to run one query and fetch all data at once? If you did already, could you run a profiler session quickly to see whether any obvious thing could be improved? especially wondering the time spent in php vs mysql and whether we could run more optimised sql and run less sql queries.
@sgiehl commented on August 31st 2017 Member

I've fixed all layout issues. Regarding the speed issue: No, I hadn't reverted the sql changes. But the query is now slower for sure, as it contains four more joins for selecting the content interactions.
And the query for fetching the actions did change much in order get them for all visits instead of only one (idvisit = ? vs idvisit IN (?))

You could try to disable the contents plugin and compare the speed then (that should remove the extra joins). I'm not sure if I will have some time to profile that tomorrow.

@mattab commented on September 4th 2017 Owner

Merged! :tada:

Still we do need to look at the performance issue because it's important that Visitor log loads fast 100 visits at once. Please share what you find in Profiling? if we need to set it up on demo2 let me know.

This Pull Request was closed on September 4th 2017
Powered by GitHub Issue Mirror