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

Responsive Ads In Text Content #31

Merged
merged 5 commits into from
Mar 24, 2020
Merged

Conversation

jeffersonrabb
Copy link
Contributor

@jeffersonrabb jeffersonrabb commented Mar 19, 2020

All Submissions:

Changes proposed in this Pull Request:

#19 introduced responsive ads, but only for full-width positions: Above Header, Below Header, Above Footer. A common use case is to place ads in the content using Super Cool Ad Inserter. If media queries responded to the width of an element's container this would be an easy fix, but unfortunately this is not possible. Instead, this branch uses a series of filters to allow the theme to control the media queries based on the width of the theme's text column. These are the three new filters:

newspack_ads_maybe_use_responsive_placement: whether responsive logic should be applied to a particular Ad placement.
newspack_ads_breakpoint: allows themes to alter breakpoints based on the width of the content column.
newspack_ads_should_have_min_width: allows theme to determine the conditions where min-width should be excluded from the media query.

This PR should be tested with Automattic/newspack-theme#839,

Closes #30.

How to test the changes in this Pull Request:

  1. Create an Ad Unit in Newpack->Advertising with at least two sizes, one optimized for desktop and one for mobile. My testing focused on a unit with 728x90 and 300x250 varients.
  2. Install Super Cool Ad Inserter plugin.
  3. Navigate to Appearance->Widgets. In "Inserted Ad Position 1" insert Newspack Ads, and select the unit created in step 1.
  4. Create posts with each of the three Newspack Theme templates. Each should have at least six paragraphs to ensure the ad will be properly inserted.
  5. View all three posts at a variety of viewport widths. Verify the unit shown is appropriate to the browser width. https://cloudup.com/cmEV4zGFJIC

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@jeffersonrabb jeffersonrabb marked this pull request as ready for review March 19, 2020 22:19
@jeffersonrabb jeffersonrabb changed the title feat: responsive ads for super cool ad inserter placements Responsive Ads In Text Content Mar 19, 2020
@jeffersonrabb jeffersonrabb self-assigned this Mar 19, 2020
@jeffersonrabb
Copy link
Contributor Author

In a355613 I've simplified the logic so there is only one media query filter: newspack_ads_media_query_for_size

This is used to filter the media query for one size of an ad unit. The theme can alter the max/min width for a particular size based on theme considerations like sidebar presence and margins.

Copy link
Member

@adekbadek adekbadek left a comment

Choose a reason for hiding this comment

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

LGTM

@jeffersonrabb jeffersonrabb merged commit 2fbf558 into master Mar 24, 2020
@jeffersonrabb jeffersonrabb deleted the add/responsive-ads-for-scaiw branch March 24, 2020 16:36
matticbot pushed a commit that referenced this pull request Mar 24, 2020
# [1.1.0](v1.0.0...v1.1.0) (2020-03-24)

### Bug Fixes

* placeholder box-shadow and background color since gutenberg 7.7 ([6a96286](6a96286))

### Features

* responsive ads for super cool ad inserter placements ([#31](#31)) ([2fbf558](2fbf558))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Apr 20, 2021
# [1.1.0-alpha.1](v1.0.0...v1.1.0-alpha.1) (2021-04-20)

### Bug Fixes

* add 'supports' to block to prevent Gutenberg 8.6.0 notice ([3651cb8](3651cb8))
* correctly center aligncenter ads ([56d891a](56d891a))
* dont define slots that arent present ([f7439ff](f7439ff))
* dont show ads on viewports smaller than smallest ad size ([f726214](f726214))
* for sticky ads, use smallest ad unit size available ([#101](#101)) ([8b4da54](8b4da54))
* harden queried object check logic ([dced583](dced583))
* improve responsive ads in non-AMP mode ([02bde12](02bde12))
* initial work on amp multisize ([ae6aad6](ae6aad6))
* limit multisizes of sticky ads ([#109](#109)) ([481fccc](481fccc))
* placeholder box-shadow and background color since gutenberg 7.7 ([6a96286](6a96286))
* remove deprecated third arg from function ([fddcd06](fddcd06))
* remove early ad display calls ([95a2217](95a2217))
* remove redundant responsive ad handling ([a8b5727](a8b5727))
* unique IDs for responsive ads ([#35](#35)) ([e98b0fc](e98b0fc))
* use name for array index not code, as code can be reused ([dea4799](dea4799))

### Features

* add common ad targeting to ad units ([18dca0b](18dca0b))
* add filter for controlling whether ads are displayed ([d0d0851](d0d0851))
* add filter for customizing ad output ([ee6e442](ee6e442))
* add filters to extend ad sizing logic ([a3b396f](a3b396f))
* add Gutenberg icon and replace Material ([#88](#88)) ([2e7ddc9](2e7ddc9))
* add support for ad targeting via filter ([9abc0ae](9abc0ae))
* automatic responsive ad handling in non-AMP mode ([7d4719e](7d4719e))
* check for amp plus status ([7168e03](7168e03))
* data attributes on gam scripts for amp sanitization ([484fc82](484fc82))
* improved ad sizing for non-responsive amp ads ([9551a20](9551a20))
* optimize amp-ads for viewability ([#72](#72)) ([6a29e67](6a29e67))
* responsive ads for super cool ad inserter placements ([#31](#31)) ([2fbf558](2fbf558))
* size grouping for responsive amp ads ([0b7da43](0b7da43))
* trigger release ([af80dd2](af80dd2))
* trigger release ([441aef1](441aef1))
* update block icon ([#116](#116)) ([6831c72](6831c72))
* update block icon style and colour ([#82](#82)) ([a31297f](a31297f))
* update block placeholder and alignment options ([#121](#121)) ([744bb3d](744bb3d))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Responsive Implementation For Units Placed In Content
3 participants