From 600388b3c0cbcc0c34e0d3f188ad953d18639d96 Mon Sep 17 00:00:00 2001 From: Marcelo Vani Date: Thu, 9 Aug 2018 12:29:27 +0100 Subject: [PATCH 1/5] Applied patch --- src/Form/BreakpointFormTrait.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Form/BreakpointFormTrait.php b/src/Form/BreakpointFormTrait.php index 4b0bbfc..da42a4b 100644 --- a/src/Form/BreakpointFormTrait.php +++ b/src/Form/BreakpointFormTrait.php @@ -22,13 +22,13 @@ public static function breakpointFormValidate(array $element, FormStateInterface $form_state->setError($element['browser_size'], t('The browser size cannot be empty if ad size(s) exists.')); } elseif (!empty($element['browser_size']['#value']) && empty($element['ad_sizes']['#value'])) { - $form_state->setError($element['ad_sizes'], t('The ad size(s) cannot be empty if a browser size exists.')); + $form_state->setError($element['ad_sizes'], t('The ad size(s) cannot be empty if a browser size exists. Use @none for displaying no ads.', ['@none' => ''])); } if (!empty($element['browser_size']['#value']) && !empty($element['ad_sizes']['#value'])) { if (preg_match('/[^x|0-9]/', $element['browser_size']['#value'])) { $form_state->setError($element['browser_size'], t('The browser size can only contain numbers and the character x.')); } - elseif (preg_match('/[^x|,|0-9]/', $element['ad_sizes']['#value'])) { + elseif ($element['ad_sizes']['#value'] != '' && preg_match('/[^x|,|0-9]/', $element['ad_sizes']['#value'])) { $form_state->setError($element['ad_sizes'], t('The ad size(s) can only contain numbers, the character x and commas.')); } } @@ -54,6 +54,10 @@ protected static function breakpointsTrim(array &$values, $parent = 'breakpoints if (empty($val[$k]['browser_size']) && empty($val[$k]['ad_sizes'])) { unset($val[$k]); } + // Set as empty if no ads should be displayed. + if (isset($val[$k]['ad_sizes']) && $val[$k]['ad_sizes'] == '') { + $val[$k]['ad_sizes'] = ''; + } } // Reset the array indexes to prevent wierd behavior caused by a // breakpoint being removed in the middle of the array. @@ -158,7 +162,7 @@ protected function addBreakpointForm(array &$form, $key, array $data = []) { ]; if (empty($data)) { $form['breakpoints']['table'][$key]['browser_size']['#description'] = $this->t('Example: 1024x768'); - $form['breakpoints']['table'][$key]['ad_sizes']['#description'] = $this->t('Example: 300x600,300x250'); + $form['breakpoints']['table'][$key]['ad_sizes']['#description'] = $this->t('Example: 300x600, 300x250 or @none', ['@none' => '']); } } From 0cf36cbe374736021114303bf0cc35e238e5815b Mon Sep 17 00:00:00 2001 From: Marcelo Vani Date: Thu, 9 Aug 2018 15:04:42 +0100 Subject: [PATCH 2/5] Used same messages as in issue 2383919 --- src/Form/BreakpointFormTrait.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Form/BreakpointFormTrait.php b/src/Form/BreakpointFormTrait.php index da42a4b..ecb3364 100644 --- a/src/Form/BreakpointFormTrait.php +++ b/src/Form/BreakpointFormTrait.php @@ -22,14 +22,22 @@ public static function breakpointFormValidate(array $element, FormStateInterface $form_state->setError($element['browser_size'], t('The browser size cannot be empty if ad size(s) exists.')); } elseif (!empty($element['browser_size']['#value']) && empty($element['ad_sizes']['#value'])) { - $form_state->setError($element['ad_sizes'], t('The ad size(s) cannot be empty if a browser size exists. Use @none for displaying no ads.', ['@none' => ''])); + $form_state->setError($element['ad_sizes'], t( + 'The ad size(s) cannot be empty if a browser size exists. If you wish to suppress an ad slot for a given browser size, you can enter "@none" in the ad size(s) field.', + array('@none' => '') + ) + ); } if (!empty($element['browser_size']['#value']) && !empty($element['ad_sizes']['#value'])) { if (preg_match('/[^x|0-9]/', $element['browser_size']['#value'])) { $form_state->setError($element['browser_size'], t('The browser size can only contain numbers and the character x.')); } elseif ($element['ad_sizes']['#value'] != '' && preg_match('/[^x|,|0-9]/', $element['ad_sizes']['#value'])) { - $form_state->setError($element['ad_sizes'], t('The ad size(s) can only contain numbers, the character x and commas.')); + $form_state->setError($element['ad_sizes'], t( + 'The ad size(s) string can only contain numbers, the character x and commas (unless it is the special keyword "@none").', + array('@none' => '') + ) + ); } } } @@ -162,7 +170,10 @@ protected function addBreakpointForm(array &$form, $key, array $data = []) { ]; if (empty($data)) { $form['breakpoints']['table'][$key]['browser_size']['#description'] = $this->t('Example: 1024x768'); - $form['breakpoints']['table'][$key]['ad_sizes']['#description'] = $this->t('Example: 300x600, 300x250 or @none', ['@none' => '']); + $form['breakpoints']['table'][$key]['ad_sizes']['#description'] = $this->t( + 'Example: 300x600, 300x250. Enter "@none" to suppress this slot for a given browser size.', + array('@none' => '') + ); } } From 34df3c83803c1ad529d0ee90e755697d075a7a92 Mon Sep 17 00:00:00 2001 From: Marcelo Vani Date: Thu, 9 Aug 2018 15:26:29 +0100 Subject: [PATCH 3/5] Removed the code that causes the value to be wipped from the admin ui --- src/Form/BreakpointFormTrait.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Form/BreakpointFormTrait.php b/src/Form/BreakpointFormTrait.php index ecb3364..631da76 100644 --- a/src/Form/BreakpointFormTrait.php +++ b/src/Form/BreakpointFormTrait.php @@ -62,10 +62,6 @@ protected static function breakpointsTrim(array &$values, $parent = 'breakpoints if (empty($val[$k]['browser_size']) && empty($val[$k]['ad_sizes'])) { unset($val[$k]); } - // Set as empty if no ads should be displayed. - if (isset($val[$k]['ad_sizes']) && $val[$k]['ad_sizes'] == '') { - $val[$k]['ad_sizes'] = ''; - } } // Reset the array indexes to prevent wierd behavior caused by a // breakpoint being removed in the middle of the array. From a48e6b560875513681e7b4a4af5d91b5df8c317b Mon Sep 17 00:00:00 2001 From: Marcelo Vani Date: Thu, 9 Aug 2018 15:51:10 +0100 Subject: [PATCH 4/5] Display empty size for ad slots with --- src/View/TagView.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/View/TagView.php b/src/View/TagView.php index 9b4cc10..b8ccd42 100644 --- a/src/View/TagView.php +++ b/src/View/TagView.php @@ -329,8 +329,15 @@ public static function formatSize($size) { $sizes = explode(',', $size); foreach ($sizes as $size) { - $formatted_size = explode('x', trim($size)); - $formatted_sizes[] = '[' . implode(', ', $formatted_size) . ']'; + if ($size == '') { + // If the ad sizes string contains the special keyword "," use an + // empty size list in order to suppress slot display. + $formatted_sizes[] = '[]'; + } + else { + $formatted_size = explode('x', trim($size)); + $formatted_sizes[] = '[' . implode(', ', $formatted_size) . ']'; + } } return count($formatted_sizes) == 1 ? $formatted_sizes[0] : '[' . implode(', ', $formatted_sizes) . ']'; From 5eda5798d2bbc746ae0238813fe50c9733cf48d4 Mon Sep 17 00:00:00 2001 From: Marcelo Vani Date: Thu, 9 Aug 2018 17:13:02 +0100 Subject: [PATCH 5/5] Updated tests --- src/Tests/DisplayTagTest.php | 9 ++++++++- tests/src/Unit/View/TagViewTest.php | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/Tests/DisplayTagTest.php b/src/Tests/DisplayTagTest.php index 261359f..9d4ff33 100644 --- a/src/Tests/DisplayTagTest.php +++ b/src/Tests/DisplayTagTest.php @@ -69,7 +69,14 @@ public function testDisplayTagWithMapping() { $edit['breakpoints[0][browser_size]'] = $this->dfpGenerateSize(); $edit['breakpoints[0][ad_sizes]'] = '100y100,200x200'; $this->dfpEditTag($tag->id(), $edit); - $this->assertText(t('The ad size(s) can only contain numbers, the character x and commas.'), 'An error was correctly thrown when invalid characters.'); + $this->assertText(t('The ad size(s) string can only contain numbers, the character x and commas (unless it is the special keyword "<none>").'), 'An error was correctly thrown when invalid characters.'); + + // Test tags with ad_size set to . + $edit['breakpoints[0][browser_size]'] = '0x0'; + $edit['breakpoints[0][ad_sizes]'] = ''; + $this->dfpEditTag($tag->id(), $edit); + $this->drupalGet(''); + $this->assertRaw('addSize([0, 0], [])'); } /** diff --git a/tests/src/Unit/View/TagViewTest.php b/tests/src/Unit/View/TagViewTest.php index 95f644f..9d402d5 100644 --- a/tests/src/Unit/View/TagViewTest.php +++ b/tests/src/Unit/View/TagViewTest.php @@ -32,6 +32,7 @@ public function testFormatSize($size, $expected) { */ public function formatSizeProvider() { return [ + ['', '[]'], ['300x250 ', '[300, 250]'], ['300x250, 728x90 ', '[[300, 250], [728, 90]]'], ];