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

Block Editor Stats - Generalize block variation tracking. #53606

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Jun 10, 2021

Changes proposed in this Pull Request

  • Applies a generalized solution for tracking block variations in block tracking events.
  • Instead of implementing variation tracking on an ad-hoc basis, we can update this to use getActiveBlockVariation from the core blocks store. This will ensure our variation tracking is up to date with any changes to attribute schema, variation isActive functions, and as new blocks with variations are added to the library.
  • This means that variation_slug will appear as a property for all block types, and will be populated as undefined if no variation is active.

Testing instructions

  • Follow steps to enable tracking debugging at PCYsg-nrf-p2
  • Insert an embed block variation and verify that variation_slug of the tracking event is populated with the same value as before.
  • Insert a social icons block, from there insert a social-link block variation and verify variation_slug is populated with the same value as before.
  • For blocks without variations, such as paragraph, verify variation_slug is undefined.
  • With FSE active, insert a "Header" block (variation of template part) and verify variation_slug is "header".

Related to #
#53410

@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.

@Addison-Stavlo Addison-Stavlo added [Goal] Full Site Editing [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Stats Everything related to our analytics product at /stats/ labels Jun 10, 2021
@Addison-Stavlo Addison-Stavlo changed the title use getActiveBlockVariation fn from blocks store Block Editor Stats - Generalize block variation tracking. Jun 10, 2021
Copy link
Contributor

@david-szabo97 david-szabo97 left a comment

Choose a reason for hiding this comment

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

Awesome! This is a lot better!

Tests well for me! 🚢

Copy link
Contributor

@retrofox retrofox left a comment

Choose a reason for hiding this comment

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

It's nice to see the API exposes a way to get the block variation properly. Thanks this improvement.

@Addison-Stavlo Addison-Stavlo merged commit 02641af into trunk Jun 11, 2021
@Addison-Stavlo Addison-Stavlo deleted the try/generalized-block-variation-tracking branch June 11, 2021 15:00
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jun 11, 2021
@kraftbj
Copy link
Contributor

kraftbj commented Jun 11, 2021

Note when this was released to wpcom-block-editor on wp.com, it had to be reverted due to error that getActiveBlockVariation was not a function. See p1623442537376200-slack-C010KDAPG49


return {};
return {
variation_slug: select( 'core/blocks' ).getActiveBlockVariation( block.name, block.attributes )
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that something that depends on a specific Gutenberg version being available, perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's a relatively new feature in Gutenberg. Opening a PR to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Stats Everything related to our analytics product at /stats/ [Goal] Full Site Editing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants