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

feat: "fixed height" a global setting with max threshold #590

Merged
merged 2 commits into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions includes/class-core.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ private function includes() {
include_once NEWSPACK_ADS_ABSPATH . '/includes/providers/broadstreet/class-broadstreet-provider.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/class-settings.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/class-custom-label.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/class-fixed-height.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/class-providers.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/class-bidding.php';
include_once NEWSPACK_ADS_ABSPATH . '/includes/bidders/class-medianet.php';
Expand Down
71 changes: 71 additions & 0 deletions includes/class-fixed-height.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php
/**
* Newspack Ads Fixed Height Settings.
*
* @package Newspack
*/

namespace Newspack_Ads;

use Newspack_Ads\Core;
use Newspack_Ads\Settings;

/**
* Newspack Ads Fixed Height Class.
*/
final class Fixed_Height {

const SECTION = 'fixed_height';
/**
* Initialize hooks.
*/
public static function init() {
add_action( 'newspack_ads_settings_list', [ __CLASS__, 'register_settings' ] );
}

/**
* Register Fixed Height settings.
*
* @param array $settings_list List of settings.
*
* @return array Updated list of settings.
*/
public static function register_settings( $settings_list ) {
if ( Core::is_amp() ) {
return $settings_list;
}
return array_merge(
$settings_list,
[
[
'description' => __( 'Fixed height', 'newspack-ads' ),
'help' => __( 'If enabled, ad slots will be rendered with a fixed height determined by the unit\'s sizes configuration.', 'newspack-ads' ),
'section' => self::SECTION,
'key' => 'active',
'type' => 'boolean',
'default' => true,
'public' => true,
],
[
'description' => __( 'Use maximum fixed height', 'newspack-ads' ),
miguelpeixe marked this conversation as resolved.
Show resolved Hide resolved
'help' => __( 'If enabled, rendered creatives larger than "maximum fixed height" will adjust the height accordingly and cause layout shift.', 'newspack-ads' ),
'section' => self::SECTION,
'key' => 'use_max_height',
'type' => 'boolean',
'default' => true,
'public' => true,
],
[
'description' => __( 'Maximum fixed height', 'newspack-ads' ),
'help' => __( 'Maximum value for the fixed height applied to the ad slot, in pixels.', 'newspack-ads' ),
'section' => self::SECTION,
'key' => 'max_height',
'type' => 'int',
'default' => 100,
'public' => true,
],
]
);
}
}
Fixed_Height::init();
12 changes: 1 addition & 11 deletions includes/class-placements.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ public static function register_api_endpoints() {
'bidders_ids' => [
'sanitize_callback' => [ __CLASS__, 'sanitize_bidders_ids' ],
],
'fixed_height' => [
'sanitize_callback' => 'rest_sanitize_boolean',
],
'hooks' => [
'sanitize_callback' => [ __CLASS__, 'sanitize_hooks_data' ],
],
Expand Down Expand Up @@ -135,10 +132,6 @@ public static function sanitize_placement( $data ) {
$sanitized_data['ad_unit'] = sanitize_text_field( $data['ad_unit'] );
}

if ( isset( $data['fixed_height'] ) ) {
$sanitized_data['fixed_height'] = rest_sanitize_boolean( $data['fixed_height'] );
}

if ( isset( $data['bidders_ids'] ) ) {
$sanitized_data['bidders_ids'] = self::sanitize_bidders_ids( $data['bidders_ids'] );
}
Expand Down Expand Up @@ -235,7 +228,6 @@ public static function api_update_placement( $request ) {
'ad_unit' => $request['ad_unit'],
'bidders_ids' => $request['bidders_ids'],
'hooks' => $request['hooks'],
'fixed_height' => $request['fixed_height'],
'stick_to_top' => $request['stick_to_top'],
];
$result = self::update_placement( $request['placement'], $data );
Expand Down Expand Up @@ -727,8 +719,6 @@ public static function inject_placement_ad( $placement_key, $hook_key = '' ) {
$placement_data = $placement['data'];
}

$placement_data['fixed_height'] = isset( $placement_data['fixed_height'] ) ? (bool) $placement_data['fixed_height'] : false;

if ( ! isset( $placement_data['ad_unit'] ) || empty( $placement_data['ad_unit'] ) ) {
return;
}
Expand Down Expand Up @@ -765,7 +755,7 @@ public static function inject_placement_ad( $placement_key, $hook_key = '' ) {
$placement_key . '-' . $hook_key => ! empty( $hook_key ),
'hook-' . $hook_key => ! empty( $hook_key ),
'stick-to-top' => $stick_to_top,
'fixed-height' => $placement_data['fixed_height'],
'fixed-height' => Settings::get_setting( 'fixed_height', 'active' ),
],
$placement_key,
$hook_key,
Expand Down
40 changes: 0 additions & 40 deletions includes/customizer/class-placement-customize-control.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,22 +123,6 @@ private function get_ad_unit_value( $hook_key = '' ) {
return isset( $data['ad_unit'] ) ? $data['ad_unit'] : '';
}

/**
* Get the value of "fixed height" feature given its hook key.
*
* @param string $hook_key Optional hook key, will look root placement otherwise.
*
* @return string Ad unit ID or empty string if not found.
*/
private function get_fixed_height_value( $hook_key = '' ) {
$value = json_decode( $this->value(), true );
$data = $value;
if ( ! empty( $hook_key ) && isset( $value['hooks'], $value['hooks'][ $hook_key ] ) ) {
$data = $value['hooks'][ $hook_key ];
}
return isset( $data['fixed_height'] ) ? (bool) $data['fixed_height'] : false;
}

/**
* Get the value of "stick to top" feature given its hook key.
*
Expand Down Expand Up @@ -278,29 +262,6 @@ private function render_bidder_input( $bidder_id, $value, $hook_key = '' ) {
<?php
}

/**
* Render the control's "fixed height" checkbox.
*
* @param bool $value The current value of the control.
* @param string $hook_key The hook key.
*/
private function render_fixed_height_checkbox( $value = false, $hook_key = '' ) {
$id_args = [ 'fixed-height' ];
if ( $hook_key ) {
$id_args[] = $hook_key;
}
$label = __( 'Fixed height', 'newspack-ads' );
$input_id = $this->get_element_id( 'input', $id_args );
$desc_id = $this->get_element_id( 'description', $id_args );
?>
<span class="customize-control fixed-height-checkbox">
<input id="<?php echo esc_attr( $input_id ); ?>" type="checkbox" value="1" <?php checked( $value, '1' ); ?> />
<label for="<?php echo esc_attr( $input_id ); ?>"><?php echo esc_html( $label ); ?></label>
<span id="<?php echo esc_attr( $desc_id ); ?>" class="description customize-control-description"><?php esc_html_e( 'Avoid content layout shift by using the ad unit height as fixed height for this placement. This is recommended if an ad is guaranteed to be shown across all devices.', 'newspack-ads' ); ?></span>
</span>
<?php
}

/**
* Render the control's "stick to top" checkbox.
*
Expand Down Expand Up @@ -380,7 +341,6 @@ public function render_content() {
$this->render_placement_hook_control( $hook_key, $this->placement );
}
}
$this->render_fixed_height_checkbox( $this->get_fixed_height_value() );
if ( in_array( 'stick_to_top', $this->placement['supports'], true ) ) {
$this->render_stick_to_top_checkbox( $this->get_stick_to_top_value() );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use Newspack_Ads\Providers\Provider;
use Newspack_Ads\Core;
use Newspack_Ads\Settings;

defined( 'ABSPATH' ) || exit;

Expand Down Expand Up @@ -111,7 +112,7 @@ public function get_ad_code( $unit_id, $placement_key = '', $hook_key = '', $pla
if ( ! self::is_plugin_active() || ! method_exists( '\Broadstreet_Utility', 'getZoneCode' ) ) {
return '';
}
$fixed_height = isset( $placement_data['fixed_height'] ) ? (bool) $placement_data['fixed_height'] : false;
$fixed_height = Settings::get_setting( 'fixed_height', 'active' );
$attrs = [];
$zones = $this->get_units();
$zone_idx = array_search( $unit_id, array_column( $zones, 'value' ) );
Expand Down
12 changes: 5 additions & 7 deletions includes/providers/gam/class-gam-model.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,9 @@ public static function get_ad_unit_for_display( $id, $config = array() ) {
);
}

$unique_id = $config['unique_id'] ?? uniqid();
$placement = $config['placement'] ?? '';
$context = $config['context'] ?? '';
$fixed_height = $config['fixed_height'] ?? false;
$unique_id = $config['unique_id'] ?? uniqid();
$placement = $config['placement'] ?? '';
$context = $config['context'] ?? '';

$ad_units = self::get_ad_units( false );

Expand All @@ -228,9 +227,8 @@ public static function get_ad_unit_for_display( $id, $config = array() ) {
}
$ad_unit = $ad_units[ $index ];

$ad_unit['placement'] = $placement;
$ad_unit['context'] = $context;
$ad_unit['fixed_height'] = $fixed_height;
$ad_unit['placement'] = $placement;
$ad_unit['context'] = $context;

$ad_unit['ad_code'] = self::get_ad_unit_code( $ad_unit, $unique_id );
$ad_unit['amp_ad_code'] = self::get_ad_unit_amp_code( $ad_unit, $unique_id );
Expand Down
5 changes: 2 additions & 3 deletions includes/providers/gam/class-gam-provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ public function get_ad_code( $unit_id, $placement_key = '', $hook_key = '', $pla
$ad_unit = GAM_Model::get_ad_unit_for_display(
$unit_id,
array(
'unique_id' => $placement_data['id'],
'placement' => $placement_key,
'fixed_height' => isset( $placement_data['fixed_height'] ) ? (bool) $placement_data['fixed_height'] : false,
'unique_id' => $placement_data['id'],
'placement' => $placement_key,
)
);
if ( \is_wp_error( $ad_unit ) ) {
Expand Down
29 changes: 22 additions & 7 deletions includes/providers/gam/class-gam-scripts.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public static function insert_gpt_footer_script() {
}

$network_code = GAM_Model::get_active_network_code();
$fixed_height = Settings::get_settings( 'fixed_height' );

$prepared_unit_data = [];
foreach ( GAM_Model::$slots as $unique_id => $ad_unit ) {
Expand Down Expand Up @@ -110,7 +111,7 @@ public static function insert_gpt_footer_script() {
'code' => esc_attr( $ad_unit['code'] ),
'sizes' => $sizes,
'fluid' => (bool) $ad_unit['fluid'],
'fixed_height' => (bool) $ad_unit['fixed_height'],
'fixed_height' => $fixed_height,
'targeting' => $ad_targeting,
'sticky' => GAM_Model::is_sticky( $ad_unit ),
'size_map' => GAM_Model::get_ad_unit_size_map( $ad_unit ),
Expand Down Expand Up @@ -250,8 +251,9 @@ function inOrPastViewport( element ) {
* outside of the viewport will have 'auto' height.
*/
?>
if ( ad_unit.fixed_height ) {
if ( ad_unit.fixed_height.active ) {
var height = 'auto';
var prop = 'height';
if ( ad_unit.in_viewport ) {
for ( viewportWidth in ad_unit.size_map ) {
if ( viewportWidth < availableWidth ) {
Expand All @@ -261,9 +263,13 @@ function inOrPastViewport( element ) {
}
}
}
if ( ad_unit.fixed_height.use_max_height && ad_unit.fixed_height.max_height < height ) {
height = ad_unit.fixed_height.max_height;
prop = 'min-height';
}
height = height + 'px';
}
container.parentNode.style.height = height;
container.parentNode.style[prop] = height;
}
}
googletag.cmd.push(function() {
Expand Down Expand Up @@ -357,7 +363,7 @@ function inOrPastViewport( element ) {
* creatives to render on refresh control.
*/
?>
if ( ad_unit.fixed_height && container.parentNode.style.height === 'auto' && event.size ) {
if ( ad_unit.fixed_height.active && container.parentNode.style.height === 'auto' && event.size ) {
container.parentNode.style.height = event.size[1] + 'px';
event.slot.defineSizeMapping( googletag.sizeMapping().addSize( [ 0, 0 ], event.size ).build() );
}
Expand All @@ -366,7 +372,7 @@ function inOrPastViewport( element ) {
* Handle slot visibility.
*/
?>
if ( event.isEmpty && ( ! ad_unit.fixed_height || ( ad_unit.fixed_height && ! ad_unit.in_viewport ) ) ) {
if ( event.isEmpty && ( ! ad_unit.fixed_height.active || ( ad_unit.fixed_height.active && ! ad_unit.in_viewport ) ) ) {
container.parentNode.style.display = 'none';
} else {
container.parentNode.style.display = 'flex';
Expand Down Expand Up @@ -408,10 +414,19 @@ public static function print_fixed_height_css( $placement_key, $hook_key, $place
if ( 'gam' !== $placement_data['provider'] ) {
return;
}
if ( ! isset( $placement_data['fixed_height'] ) || ! $placement_data['fixed_height'] ) {

$fixed_height = Settings::get_settings( 'fixed_height' );
if ( ! $fixed_height['active'] ) {
return;
}

$get_height = function( $height ) use ( $fixed_height ) {
if ( $fixed_height['use_max_height'] && ! empty( $fixed_height['max_height'] ) ) {
$height = min( $height, $fixed_height['max_height'] );
}
return sprintf( '%spx', $height );
};

$ad_units = GAM_Model::get_ad_units( false );
$ad_unit_idx = array_search( $placement_data['ad_unit'], array_column( $ad_units, 'id' ) );
if ( false === $ad_unit_idx ) {
Expand All @@ -423,7 +438,7 @@ public static function print_fixed_height_css( $placement_key, $hook_key, $place
echo '<style>';
foreach ( $size_map as $viewport_width => $sizes ) {
$height = max( array_column( $sizes, 1 ) );
$css = sprintf( ' @media ( min-width: %1$dpx ) { %2$s { height: %3$dpx; } } ', $viewport_width, $container_id, $height );
$css = sprintf( ' @media ( min-width: %1$dpx ) { %2$s { min-height: %3$dpx; } } ', $viewport_width, $container_id, $get_height( $height ) );
echo esc_html( $css );
}
echo '</style>';
Expand Down
4 changes: 0 additions & 4 deletions src/blocks/ad-unit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ export const settings = {
type: 'object',
default: {},
},
fixed_height: {
type: 'boolean',
default: false,
},
// Legacy attribute.
activeAd: {
type: 'string',
Expand Down
3 changes: 0 additions & 3 deletions src/customizer/control.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ import { set, debounce } from 'lodash';
$toggle.on( 'change', function () {
updateValue( '', 'enabled', $( this ).is( ':checked' ) );
} );
container.on( 'change', '.fixed-height-checkbox input[type=checkbox]', function () {
updateValue( '', 'fixed_height', $( this ).is( ':checked' ) );
} );
container.on( 'change', '.stick-to-top-checkbox input[type=checkbox]', function () {
updateValue( '', 'stick_to_top', $( this ).is( ':checked' ) );
} );
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
&.fixed-height {
padding: 16px 0;
box-sizing: content-box;
> * {
margin: 0;
}
}
}

Expand Down
11 changes: 1 addition & 10 deletions src/placements/placement-control.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
/**
* WordPress dependencies
*/
import { Notice, SelectControl, ToggleControl, TextControl } from '@wordpress/components';
import { Notice, SelectControl, TextControl } from '@wordpress/components';
import { Fragment, useState, useEffect } from '@wordpress/element';
import { __, sprintf } from '@wordpress/i18n';

Expand Down Expand Up @@ -169,15 +169,6 @@ const PlacementControl = ( {
}
return null;
} ) }
<ToggleControl
label={ __( 'Use fixed height', 'newspack-ads' ) }
help={ __(
'Avoid content layout shift by using the ad unit height as fixed height for this placement. This is recommended if an ad is guaranteed to be shown across all devices.',
'newspack-ads'
) }
checked={ !! value.fixed_height }
onChange={ data => onChange( { ...value, fixed_height: data } ) }
/>
</Fragment>
);
};
Expand Down