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

New icon in all reports: let me segment by visitors matching this row's criteria #7192

Merged
merged 4 commits into from Feb 13, 2015

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Feb 13, 2015

refs #4633

works for most reports. It won't fix the issue completely but it would be usable. Once this is merged I'd like to see if it is possible the solve it in core and not in each report but not sure it will be.

The width of the visitor log is a bit of a problem but that's kinda similar to widget area etc. It works though.

Segment filter generation works in two steps. Either we directly set a segmentFilter metadata or first a segmentValue metadata which then gets converted to a segmentFilter in post process data table. We could always generate a segmentFilter directly but it is much less code and easier for now. Will see later again as it is harder to understand this two step thing. If we'd always want to generate a segmentFilter directly we have to instantiate hardcoded reports in each API method and declare the segmentValue for each API method instead of centralised in some getDataTable methods etc.

@tsteur tsteur added the Needs Review PRs that need a code review label Feb 13, 2015
@tsteur
Copy link
Member Author

tsteur commented Feb 13, 2015

@mattab do you mind having a look? And maybe also test it?

@mattab
Copy link
Member

mattab commented Feb 13, 2015

this is an EPIC PULL REQUEST

@mattab
Copy link
Member

mattab commented Feb 13, 2015

really great... this is such a killer feature!! 👍

Only feedback

  • rename <segmentFilter> to <segment>

Merging and will deploy on demo

mattab pushed a commit that referenced this pull request Feb 13, 2015
New icon in all reports: let me segment by visitors matching this row's criteria
@mattab mattab merged commit f61f697 into master Feb 13, 2015
@tsteur tsteur deleted the 4633 branch February 13, 2015 06:47
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Feb 16, 2015
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
Development

Successfully merging this pull request may close these issues.

None yet

2 participants