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.

Sign up for GitHub

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

Jump to bottom

Stats: Use Redux Stats subtree instead of StatsList for StatsReferrers #10657

Merged
merged 2 commits into from Jan 18, 2017
Merged

Stats: Use Redux Stats subtree instead of StatsList for StatsReferrers #10657

merged 2 commits into from Jan 18, 2017

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Jan 16, 2017 •
edited

Closes #10624

In this PR, I moved the parser logic for StatsReferrers from StatsList into the redux stats subtree.
I also updated the referrers stats module to use the StatsConnectedModule Component.

Testing instructions

  • Navigate to the stats insights page /stats/day/$site
  • The Referrers panel should work properly (same as master)
  • Click on the header of the panel to navigate to the summary page
  • The referrers stats should show up (same as master)

@youknowriad youknowriad added Stats Everything related to our analytics product at /stats/ [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 16, 2017
@youknowriad youknowriad self-assigned this Jan 16, 2017
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=update/stats-referrers

timmyc
timmyc suggested changes Jan 17, 2017
View reviewed changes
Copy link
Contributor

@timmyc timmyc left a comment

Choose a reason for hiding this comment

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

This is testing out well for me. The only issue I did find was that the 'View All' link was not being displayed when expected. So once that prop is added this should be good to go.

client/my-sites/stats/site.jsx
date={ queryDate }
beforeNavigate={ this.updateScrollPosition } />
statType="statsReferrers" />
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add showSummaryLink={ true } prop here so the "View All" link in the footer will be shown when applicable.

@timmyc
Copy link
Contributor

timmyc commented Jan 17, 2017

LGTM

@timmyc timmyc added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 17, 2017
@youknowriad youknowriad merged commit 6536eca into master Jan 18, 2017
@youknowriad youknowriad deleted the update/stats-referrers branch January 18, 2017 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timmyc timmyc timmyc approved these changes

Assignees

@youknowriad youknowriad

Labels
Stats Everything related to our analytics product at /stats/ [Type] Janitorial
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@youknowriad @matticbot @timmyc @lancewillett