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

Stats: make nudges collapsible #20765

Merged
merged 8 commits into from
Aug 30, 2021
Merged

Stats: make nudges collapsible #20765

merged 8 commits into from
Aug 30, 2021

Conversation

kbrown9
Copy link
Member

@kbrown9 kbrown9 commented Aug 19, 2021

Changes proposed in this Pull Request:

  • Add the ability to collapse the nudges UI.

Jetpack product discussion

pbNhbs-189-p2#comment-3564

Does this pull request change what data or activity we track or use?

  • no

Testing instructions:

  1. Install this branch of Jetpack. Activate Jetpack and connect (a site or user connection is fine).
  2. In wp-admin, navigate to Jetpack -> Site Stats.
    • If the stats chart isn't available yet, the collapse toggle isn't displayed.
    • If the stats chart is available, the collapse toggle is displayed.
  3. The Nudges UI should be expanded by default.
  4. Collapse the nudges UI by clicking on the triangle icon.
  5. Reload the page to verify that the collapsed state persists.
  6. Expand the nudges UI by clicking on the triangle icon.
  7. Reload the page to verify that the expanded state persists.
Update Jetpack

If Jetpack is updated, the nudges collapse state will be reset.

  1. In wp-admin, navigate to Jetpack -> Site Stats. Collapse the nudges.
  2. Increase the Jetpack version in the jetpack.php file here.
  3. In wp-admin, navigate to Jetpack -> Site Stats. Verify that the nudges are expanded.

Screenshots

Expanded

Screen%20Shot%20on%202021-08-23%20at%2015-23-23

Collapsed

Screen%20Shot%20on%202021-08-23%20at%2015-23-49

@kbrown9 kbrown9 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. [Status] In Progress labels Aug 19, 2021
@kbrown9 kbrown9 self-assigned this Aug 19, 2021
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Aug 19, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Aug 19, 2021

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: September 7, 2021.
  • Scheduled code freeze: August 30, 2021.

@kbrown9 kbrown9 added this to the jetpack/10.1 milestone Aug 25, 2021
@kbrown9 kbrown9 added the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Aug 25, 2021
@kbrown9 kbrown9 requested a review from leogermani August 25, 2021 20:57
projects/plugins/jetpack/modules/stats.php Outdated Show resolved Hide resolved
@@ -2670,6 +2670,13 @@ public static function get_updateable_data_list( $selector = '' ) {
'validate_callback' => __CLASS__ . '::validate_posint',
'jp_group' => 'stats',
),
'collapse_nudges' => array(
'description' => esc_html__( 'Collapse upsell nudges', 'jetpack' ),
Copy link
Member

@jeherve jeherve Aug 26, 2021

Choose a reason for hiding this comment

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

I'd suggest using "upgrade" instead of "upsell" everywhere, to remain consistent.

Suggested change
'description' => esc_html__( 'Collapse upsell nudges', 'jetpack' ),
'description' => esc_html__( 'Collapse upgrade nudges', 'jetpack' ),

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion - fixed in c9d0b67.

@@ -3390,6 +3390,13 @@ public static function do_version_bump( $version, $old_version ) {

// If an admin page is visited after the update, the 'current_screen' action will fire.
add_action( 'current_screen', 'Jetpack::set_update_modal_display' );

// Expand stats page nudges.
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about hooking into updating_jetpack_version action and keeping all the logic in the Nudges class, instead of adding this code here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's much better to keep this logic in the Nudges class as you suggested. I made that change in commit c9d0b67.

I had to set up the updating_jetpack_version hook in the stats_load method, because the Nudges initialization happens too late.

@kbrown9 kbrown9 added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 26, 2021
* Make text consistent: upsell -> upgrade.
* Move version update hook to the stats module files.
@kbrown9 kbrown9 added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Aug 27, 2021
Copy link
Contributor

@leogermani leogermani left a comment

Choose a reason for hiding this comment

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

Tests well for me with the latest changes

@leogermani leogermani added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Aug 30, 2021
@sdixon194 sdixon194 enabled auto-merge (squash) August 30, 2021 14:45
@sdixon194 sdixon194 merged commit cea0d3e into master Aug 30, 2021
@sdixon194 sdixon194 deleted the update/stats_upsell_collapse branch August 30, 2021 14:56
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Aug 30, 2021
davidlonjon added a commit that referenced this pull request Aug 31, 2021
* master: (34 commits)
  Fix issue templates (#20880)
  Social Icon Widget: Fix Icons Not Saving (#20862)
  Dashboard: Add a specific message for a site that is too new to have a rewind status (#20863)
  Add additional properties to WC Analytics events (#20812)
  Connection: add spinner for the "Connect" button (#20872)
  Update storybook monorepo to v6.3.7 (#20877)
  Lightweight PHPCS (#20876)
  Recommendations: Add Jetpack Product Suggestions step (#20814)
  Update babel monorepo (#20875)
  Update wordpress monorepo (#20873)
  cli: Check subprocess exit statuses in docker commands (#20861)
  Facebook Widget: Fix URL error when adding widget (#20864)
  Stats: make nudges collapsible (#20765)
  Remove temporary Newspack block override css (#20868)
  Allow ZIP uploads via Calypso (#20860)
  Sync Package Release v1.26.0 (#20870)
  Connection: remove in-place in secondary flows (#20739)
  BUILD_DIR -> BUILD_BASE in initial checks (#20857)
  E2E tests: Fix missing action for atomic workflow (#20866)
  Boost: Fix skip proxy origins to load css during critical CSS generation (#20793)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Stats Data Feature that enables users to track their site's traffic and gain insights on popular content. [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants