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

Show unique views on advanced stats page #376

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

overint
Copy link
Contributor

@overint overint commented Oct 22, 2017

As per #346 I have added a unique hit counter to the title above the chart.

@@ -55,7 +55,7 @@

<div class="row bottom-padding">
<div class="col-md-8">
<h4>Traffic over Time</h4> (total: {{ $link->clicks }})
<h4>Traffic over Time</h4> (total: {{ $link->clicks }}) (unique: {{ $uniques }})
Copy link
Owner

@cydrobolt cydrobolt Oct 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Placing the "unique" number of clicks next to the "total" number of clicks is a bit deceiving. The uniques count is calculated using the base rows from our existing stats, which only fetches rows from the given time period, whereas the "total" number is a count that encompasses all time clicks. If this is the behaviour we want, we should clarify this point, or move the count somewhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated it to not take into account the date range.

@overint
Copy link
Contributor Author

overint commented Oct 28, 2017

I changed it to just show the total unique views of all time, but I think it's a bit confusing since it says "Traffic Over Time" right with the total number, right above a graph that only shows your selected date range.

@overint
Copy link
Contributor Author

overint commented Nov 16, 2017

Are you happy with that change?

@overint
Copy link
Contributor Author

overint commented Mar 27, 2018

@cydrobolt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants