Skip to content

Ads Gutenblock: Add "Hide ad on mobile views" toggle#11644

Merged
kraftbj merged 2 commits intomasterfrom
update/ad-block-remove-on-mobile
Mar 21, 2019
Merged

Ads Gutenblock: Add "Hide ad on mobile views" toggle#11644
kraftbj merged 2 commits intomasterfrom
update/ad-block-remove-on-mobile

Conversation

@dbspringer
Copy link
Copy Markdown
Member

@dbspringer dbspringer commented Mar 21, 2019

Sometimes a tall/wide ad unit makes sense on Desktop, but not a mobile impression. It makes sense to include an option to disable the ad on a mobile impression or replace it with one that's more mobile friendly.

Calypso PR: Automattic/wp-calypso#31608
Addresses: Automattic/wp-calypso#30870

Screen Shot 2019-03-20 at 3 06 37 PM

Changes proposed in this Pull Request:

  • Add "Hide ad on mobile views" toggle

Testing instructions:

  • Before installing the update make a post containing some "old" Ads blocks.
  • Download & build Jetpack branch: https://github.com/Automattic/jetpack/tree/update/ad-block-remove-on-mobile
  • Open Calypso branch, install dependencies with (npx lerna bootstrap --concurrency=2 --scope '@automattic/jetpack-blocks'), and then compile blocks e.g. npx lerna run build --stream --scope='@automattic/jetpack-blocks' && rsync -a --delete packages/jetpack-blocks/dist/ ../jetpack/_inc/blocks/
  • On WordAds approved site add a unit and toggle Hide ad on mobile views
  • View page via favorite mobile impression tester, see ad is missing.
  • Edit post with "old" blocks and verify the blocks have updated successfully.

Proposed changelog entry for your changes:

  • Added toggle to remove ad blocks on mobile views

@dbspringer dbspringer added this to the 7.2 milestone Mar 21, 2019
@dbspringer dbspringer self-assigned this Mar 21, 2019
@dbspringer dbspringer requested a review from a team March 21, 2019 18:07
@matticbot
Copy link
Copy Markdown
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello dbspringer! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D25858-code before merging this PR. Thank you!

@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Mar 21, 2019

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: April 2, 2019.
Scheduled code freeze: March 26, 2019

Generated by 🚫 dangerJS against ed3ad24

Copy link
Copy Markdown
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.

Quick style question.

@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 Mar 21, 2019
Co-Authored-By: dbspringer <derekspringer@gmail.com>
@matticbot
Copy link
Copy Markdown
Contributor

dbspringer, Your synced wpcom patch D25858-code has been updated.

1 similar comment
@matticbot
Copy link
Copy Markdown
Contributor

dbspringer, Your synced wpcom patch D25858-code has been updated.

@jeherve jeherve added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Mar 21, 2019
@dbspringer dbspringer added [Status] Needs Review This PR is ready for review. and removed [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 21, 2019
@jeherve jeherve added the [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack label Mar 21, 2019
Copy link
Copy Markdown
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. 👍

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 21, 2019
@kraftbj kraftbj merged commit 2d4fcb8 into master Mar 21, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 21, 2019
@kraftbj kraftbj deleted the update/ad-block-remove-on-mobile branch March 21, 2019 19:30
@kraftbj
Copy link
Copy Markdown
Contributor

kraftbj commented Mar 21, 2019

Please test and commit on WP.com when you're ready @dbspringer D25858-code

kraftbj added a commit that referenced this pull request Mar 25, 2019
kraftbj added a commit that referenced this pull request Mar 25, 2019
kraftbj added a commit that referenced this pull request Mar 25, 2019
kraftbj added a commit that referenced this pull request Mar 27, 2019
* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Add CL for #11224

* Add CL for #11426

* Add CL for #11442

* Add testing instructions for #11224

* Add CL for #11451

* Reclassify CL item

* Add testing instructions for #11451

* Add CL for #11486

* Add CL for #11418

* Add CL for #11524

* Add CL and testing instructions for #11449

* Add CL for #11460

* Add CL for #11520 and #11582

* Add CL for #11531

* Add CL #11644

* Add testing instructions for #11644

* Add testing instructions for #11644

* Add CL for #11618

* Uniform changelog lines

* CL #11679

* CL #11661

* CL #11654

* CL #11645

* CL #11643

* CL #11636

* CL #11635 and for other PHPCS commits

* CL #11627

* CL #11626

* CL #11598

* CL #11596

* Remove nested items for shortcopy. I don't believe the detailed list is helpful

* CL #11570

* CL #11569

* CL #11560

* CL #11558

* CL #11555

* CL #6704

* CL #11298

* CL #11324

* CL #11443

* CL #11484

* CL #11516

* CL #11529

* Expand Ads block enhancement CL item
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Ad [Feature] WordAds [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants