Skip to content

Simplify stats graph to use <img> tag #12999

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
Merged
jeherve merged 2 commits into from
Jul 29, 2019
Merged

Simplify stats graph to use <img> tag #12999

jeherve merged 2 commits into from
Jul 29, 2019

Conversation

@paulschreiber
Copy link
Contributor

@paulschreiber paulschreiber commented Jul 8, 2019 •
edited
Loading

Rewrite stats_admin_bar_menu() to use <img> tag instead of document.write. Eliminates blocking JavaScript.

Fixes #12944

Testing instructions:

  • log in (and view admin bar)

Proposed changelog entry for your changes:

  • no changelog

@paulschreiber paulschreiber requested a review from a team July 8, 2019 14:11
@jetpackbot
Copy link
Collaborator

jetpackbot commented Jul 8, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against e80cbb3

@simison simison added the [Status] Needs Review This PR is ready for review. label Jul 9, 2019
@simison simison added this to the 7.6 milestone Jul 9, 2019
@simison simison added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Stats Data Feature that enables users to track their site's traffic and gain insights on popular content. labels Jul 9, 2019
jeherve
jeherve requested changes Jul 17, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well for me, but you would need to add a closing tag for the div.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Jul 17, 2019
@jeherve jeherve removed this from the 7.6 milestone Jul 17, 2019
@jeherve
Copy link
Member

jeherve commented Jul 17, 2019

For reference, this was originally implemented this way in r55214-wpcom

@paulschreiber @jeherve
Update modules/stats.php
e80cbb3 
Co-Authored-By: Jeremy Herve <[email protected]>
@jeherve jeherve added this to the 7.6 milestone Jul 17, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Jul 17, 2019
@jeherve jeherve merged commit 63ff4ad into Automattic:master Jul 29, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Jul 29, 2019
jeherve added a commit that referenced this pull request Jul 29, 2019
@jeherve
Changelog: add #12999
fd6a3f2
jeherve added a commit that referenced this pull request Jul 30, 2019
@jeherve
7.6: running changelog (#12940)
f1d6377 
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
jeherve added a commit that referenced this pull request Jul 30, 2019
@jeherve
7.6: running changelog (#12940)
37aabd8 
* Add initial changelog / testing list changes for 7.6

* Update stable tag to 7.5.3

* changelog: add #12957

* Changelog: add #12932

* Changelog: add #12867

* Changelog: add #12823

* changelog: add #12969

* changelog: add #13012

* changelog: add #12974

* Changelog: add #13059

* Changelog: add #13079

* Changelog: add #12924

* changelog: add #12954

* Changelog: add #12959

* Changelog: add #12977

* Changelog: add #12830

* Changelog: add #12926

* Changelog: add #12958

* Changelog: add #12999

* Changelog: add #13077

* Changelog: add #13083

* Changelog: add #13087

* Changelog: add #13110

* Changelog: add #13116

* Changelog: add #13117

* Changelog: add #12821

* Changelog: add #13120

* changelog: add #13139

* Changelog: add #13143

* Changelog: add #13147

* Testing list: add section about sync
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

No one assigned

Labels

[Feature] Stats Data Feature that enables users to track their site's traffic and gain insights on popular content. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Milestone

7.6

Development

Successfully merging this pull request may close these issues.

Stats graph on admin bar uses document.write

5 participants

@paulschreiber @jetpackbot @jeherve @simison @matticbot