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

Premium Patterns: add pink dot #47896

Merged
merged 7 commits into from Dec 11, 2020

Conversation

deBhal
Copy link
Contributor

@deBhal deBhal commented Dec 1, 2020

This PR drops the premium popover from #47351, copied into this new PR in case we want it again later.

Changes proposed in this Pull Request

This PR is a temporary method to modify premium patterns in the inserter until changes come through in Gutenberg to facilitate this sort of thing, and we should replace this with those mechanisms as soon as they are available.

In the meantime, we:

  • Add Block Patterns Modifications and enqueue JavaScript for this part of the plugin
  • When the plugin is loaded, augment the __experimentalBlockPatterns in the block editor settings, to swap out the title value for a React element to add in a Premium badge
  • Add hover behaviour to show more information about a premium pattern when the user mouses over the pattern preview Update: the PatternPopover has been de-scoped.

Target Design:

screenshot-2020-11-26-at-11 30 09

WIP screenshots

Showing the pink dot on a test pattern:

image

Todos (carried over from #47351)

  • Add support for fetching patterns from a test source site so that we can safely test premium patterns.
  • Add support for an isPremium value when registering patterns so that we only render the badge when we expect to.
  • When registering patterns, add the isPremium value when a pattern has the premium tag.
  • Try expanding the pattern title container to include the whole pattern and then add hover state to display a tooltip with more information about the premium pattern. (de-scoped)
  • Fix up types so that it's slightly less hacky.
  • Hook up the pattern.description value to the PatternPopover, this will require setting it from the API response in https://github.com/automattic/wp-calypso/blob/63d7b360972c3c483b6a259c5d482250982ab8ab/apps/editing-toolkit/editing-toolkit-plugin/block-patterns/class-block-patterns-from-api.php#L96 (currently we're using the title as the description, too) (de-scoped)
  • Make sure the popover works correctly on mobile (de-scoped)
  • Change premium check to look for is_premium within the pattern_meta key of the response from the ptk/patterns endpoint (patterm_meta key added in D53825-code)
  • Passing the pattern name into the generated blocks in order to get the pattern name into the jetpack_editor_block_upgrade_click event
  • Add A/B test flag (replaced with a filter, and the A/B test is handled separately — D53680-code)
  • Add an e2e or lint test that (somehow!) tests for the existence of the __experimentalBlockPatterns key of getSettings() and document instructions of what to do if it fails (e.g. ping Ganon) (more context p4TIVU-9xW-p2#comment-10763) (being worked on separately in Add __experimentalBlockPatterns tests #48032 / Update experimental features tests #48164)

Testing instructions

  • In your sandbox's 0-sandbox.php file add the following to get your sandboxed test site to request patterns from a test source site. A good one to use is andysfakeblockpatternsourcesite (there's a premium pattern under the Premium category) followed by the domain, but any valid source site with patterns tagged with is_premium in Pattern Meta will do:
add_filter( 'a8c_override_patterns_source_site', function () { return '<your_test_site_as_a_string>'; } );

Also, you will need to switch on the feature in your sandbox with:

add_filter( 'a8c_enable_block_patterns_modifications', '__return_true' );
  • In the pattern inserter, if you're using the andysfakeblockpatternsourcesite source site, you should see a premium pattern under the Premium category.
  • In the standard dotcompatterns source site, there is one pattern tagged with is_premium under the Audio category, however due to caching it will likely take a while for this to show up on a test site.
  • Help us confidence check — does the hacky approach of updating __experimentalBlockPatterns in the way that we have look safe enough for the purposes of our experiment? Is there any noticeable slow down to opening the pattern inserter?

Fixes #47225

@matticbot
Copy link
Contributor

@deBhal deBhal self-assigned this Dec 1, 2020
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D53559-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@andrewserong
Copy link
Member

Thanks for creating the new PR and updating to the pink dot @deBhal! It's looking good on my test site:

image

From re-reading the update on pbAok1-1D1-p2#comment-3410 it sounds like the popover has been de-scoped at this stage, so I think we can remove that, too (though I was happy with the work we did to make it possible!)

@andrewserong andrewserong changed the title Premium Patterns: add pink dot and popover Premium Patterns: add pink dot Dec 2, 2020
@andrewserong
Copy link
Member

PatternPopover removed in: 45a1672

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 10, 2020
if ( $this->can_register_pattern( $pattern ) ) {
$is_premium = in_array( 'premium_block_pattern', array_keys( $pattern['tags'] ), true );
Copy link
Member

Choose a reason for hiding this comment

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

Once D53825-code lands, we'll just need to update this line to check for $pattern['pattern_meta']['is_premium'].

@andrewserong
Copy link
Member

@deBhal it's probably worth giving this one a rebase against trunk just to make sure it's all still working properly since there've been a couple of ETK plugin releases / version bumps over the past week.

@andrewserong andrewserong force-pushed the add/premium-patterns-inserter-pink-dot branch from 144ab02 to 2769e9f Compare December 10, 2020 23:39
@andrewserong andrewserong requested a review from a team December 11, 2020 00:54
@ramonjd
Copy link
Member

ramonjd commented Dec 11, 2020

I can't comment on the __experimentalBlockPatterns usage but I see you have a test in #48032 as recommended.

It's working well ❤️

Screen Shot 2020-12-11 at 1 48 58 pm

@ramonjd
Copy link
Member

ramonjd commented Dec 11, 2020

Passing the pattern name into the generated blocks in order to get the pattern name into the jetpack_editor_block_upgrade_click event

Screen Shot 2020-12-11 at 2 03 14 pm

Is this still being fired now that we have the in-editor upgrade button? I can't find it in my network requests anymore.

Maybe we'll need to update the openCheckout event?

If we have the A/B test running is this still important. I see how it will be still useful to know which block has prompted the user to upgrade. What do you think @razvanpapadopol ? (cc'ing you because you featured in the annotations :) )

Copy link
Member

@ramonjd ramonjd left a comment

Choose a reason for hiding this comment

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

I tested this according to the instructions, and then again in conjunction with D53680-code

I don't have any specific comments in relation to the code, but functionality-wise, it looks great to me

@andrewserong
Copy link
Member

andrewserong commented Dec 11, 2020

Thanks for testing @ramonjd! I'll merge this in shortly, and I thought we could look at improvements to which events we fire in follow ups.

@andrewserong andrewserong merged commit dfc3958 into trunk Dec 11, 2020
@andrewserong andrewserong deleted the add/premium-patterns-inserter-pink-dot branch December 11, 2020 03:27
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 11, 2020
@razvanpapadopol
Copy link

Passing the pattern name into the generated blocks in order to get the pattern name into the jetpack_editor_block_upgrade_click event

Is this still being fired now that we have the in-editor upgrade button? I can't find it in my network requests anymore.

Yes, this is still fired and it has a block prop as well. Passing the pattern name might be helpful 🤷

Maybe we'll need to update the openCheckout event?

No, we're not using openCheckout here yet but instead we're calling the a8c.wpcom-block-editor.openCheckoutModal hook to open In-editor checkout from Jetpack: Automattic/jetpack#17403

If we have the A/B test running is this still important. I see how it will be still useful to know which block has prompted the user to upgrade. What do you think @razvanpapadopol ? (cc'ing you because you featured in the annotations :) )

As mentioned above, we know already on which block the upgrade button was pressed. However, the volume is still quite low to get any conclusions.

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.

Premium patterns: add badge in the inserter to flag that a pattern is premium
6 participants