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

Indent actions belonging to a pageview #14063

Merged
merged 34 commits into from May 14, 2019
Merged

Conversation

diosmosis
Copy link
Member

@diosmosis diosmosis commented Feb 1, 2019

This will have to change when the visitor timeline is put in.

Fixes #13136
Fixes #12894

@diosmosis diosmosis added Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Feb 1, 2019
@diosmosis diosmosis added this to the 3.9.0 milestone Feb 1, 2019
@diosmosis
Copy link
Member Author

Merged w/ 3.x-dev, and here's what it looks like w/ a small visit:

image

and a large visit:

mamp_8888_index php_module corehome action index idsite 1 period day date 2019-02-06 updated 3

and for content actions:

image

image

@diosmosis diosmosis 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 Feb 23, 2019
@sgiehl
Copy link
Member

sgiehl commented Feb 25, 2019

@diosmosis I've merged in the latest 3.x-dev changes. But it seems it's currently broken. Always seeing the error No entry or class found for 'Live.pageViewActionsToDisplayCollapsed'. Guess that's caused here: https://github.com/matomo-org/matomo/pull/14063/files#diff-541de4bc6a9bb0d9cad5f733b4b853eeR122
Did you maybe forget to commit/add something?

@diosmosis
Copy link
Member Author

@sgiehl added the missing file.

@sgiehl
Copy link
Member

sgiehl commented Mar 11, 2019

@diosmosis Seems the visitor profile is broken now. Getting this error when opening one:

Variable "actionGroups" does not exist.
in ./plugins/Live/templates/_actionsList.twig line 2            

@diosmosis
Copy link
Member Author

@sgiehl updated to work in visitor profile. not sure if the implementation is the best or not, let me know if you see room for improvement.

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

image
not sure if colours were adjusted in this or another PR but seeing there are icons from premium features in different colour. Is it better to fix this in core or in the premium features?

@diosmosis
Copy link
Member Author

@tsteur The css class used is determined by the plugin due to how the template is defined, so better in the plugin i'd say.

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

when I click on "show more actions" it doesn't show more actions.

image

Also I have a visit where it sometimes shows the "show more link" after 7 actions within a pageview, and sometimes only after 20 shows the "show more link":
image

In general it looks a bit "bloated" I wonder if some additional spacing might help? We could maybe also ask our designers to tweak it maybe.

image

Also I see action images were changed to SVG? that's a problem cause it breaks the mobile app and possibly other consumers. They are supposed to return actual images. Maybe in Matomo we can use SVG but the API should still return proper images. I suppose this was maybe changed in another PR.

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

@tsteur The css class used is determined by the plugin due to how the template is defined, so better in the plugin i'd say.

@diosmosis OK I will check to see how to maybe detect the Matomo version and then change the colour of the icon depending on the version cause for older installations we still want to show the black color

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

also just seeing for some reason it doesn't show form interactions within the page view. Is that possible?

image

@diosmosis
Copy link
Member Author

@tsteur

when I click on "show more actions" it doesn't show more actions.

I assume you did a hard reload? I've seen this issue, but should have been working... same for the show more link for 7 actions. Then again, maybe the selector is using the core .action... will look.

also just seeing for some reason it doesn't show form interactions within the page view. Is that possible?

If there's no pageview ID set for the action (or the wrong pageview ID is set), then it won't be grouped. Are you looking at the demo data? pageview IDs aren't always set correctly there.

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

Another minor issue probably also from the other PR. The goal icon is not quite aligned on the left with the icons from the timeline

image

@tsteur
Copy link
Member

tsteur commented Mar 12, 2019

@diosmosis could we add color: #999 to .action-list-action-icon {} then it would just render the color of the icons fine for all premium features as well and wouldn't need to define it individually in each premium feature.

@tsteur
Copy link
Member

tsteur commented Mar 13, 2019

Yes looks good 👍

@diosmosis
Copy link
Member Author

Created iconSvg issue here: #14198

@diosmosis
Copy link
Member Author

@tsteur / @mattab updated to show refreshes and allow expanding, maybe you guys want to take a look before merging?

@mattab mattab modified the milestones: 3.9.0, 3.10.0 Mar 18, 2019
@tsteur
Copy link
Member

tsteur commented Apr 11, 2019

I suppose we could merge it now and make further changes if needed after using it for a while @mattab @diosmosis ?

@mattab
Copy link
Member

mattab commented Apr 11, 2019

I suppose we could merge it now and make further changes if needed after using it for a while

👍

@diosmosis diosmosis merged commit 0af1f22 into 3.x-dev May 14, 2019
@diosmosis diosmosis deleted the 13136-compact-action-view branch May 14, 2019 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
4 participants