From 756dc7bbae8f7fd00b1a5524933c760862222cac Mon Sep 17 00:00:00 2001 From: webchick <drupal@webchick.net> Date: Wed, 15 Jul 2015 12:53:14 -0700 Subject: [PATCH] Issue #2424805 by mdrummond, YesCT, vijaycs85, xjm, marcoscano, ifrik, joelpittet, Jelle_S, attiks, lauriii: Toolbar can no longer switch horizontal and vertical -- expects breakpoints ordered from smallest to largest; responsive images need largest to smallest --- .../breakpoint/src/BreakpointManager.php | 1 + .../src/Tests/BreakpointDiscoveryTest.php | 44 +++++++++---------- .../breakpoint_module_test.breakpoints.yml | 6 +-- .../breakpoint_theme_test.breakpoints.yml | 10 ++--- .../responsive_image/responsive_image.module | 9 +++- .../src/ResponsiveImageStyleForm.php | 7 ++- core/modules/toolbar/toolbar.breakpoints.yml | 4 +- core/themes/bartik/bartik.breakpoints.yml | 4 +- core/themes/seven/seven.breakpoints.yml | 4 +- core/themes/stark/stark.breakpoints.yml | 4 +- 10 files changed, 51 insertions(+), 42 deletions(-) diff --git a/core/modules/breakpoint/src/BreakpointManager.php b/core/modules/breakpoint/src/BreakpointManager.php index b23418f4111e..66a0830e0f9e 100644 --- a/core/modules/breakpoint/src/BreakpointManager.php +++ b/core/modules/breakpoint/src/BreakpointManager.php @@ -169,6 +169,7 @@ public function getBreakpointsByGroup($group) { $this->breakpointsByGroup[$group] = $breakpoints; } } + $instances = array(); foreach ($this->breakpointsByGroup[$group] as $plugin_id => $definition) { if (!isset($this->instances[$plugin_id])) { diff --git a/core/modules/breakpoint/src/Tests/BreakpointDiscoveryTest.php b/core/modules/breakpoint/src/Tests/BreakpointDiscoveryTest.php index b39c2a81429f..0668c86f71a1 100644 --- a/core/modules/breakpoint/src/Tests/BreakpointDiscoveryTest.php +++ b/core/modules/breakpoint/src/Tests/BreakpointDiscoveryTest.php @@ -34,51 +34,51 @@ protected function setUp() { public function testThemeBreakpoints() { // Verify the breakpoint group for breakpoint_theme_test was created. $expected_breakpoints = array( - 'breakpoint_theme_test.tv' => array( - 'label' => 'tv', - 'mediaQuery' => 'only screen and (min-width: 1220px)', + 'breakpoint_theme_test.mobile' => array( + 'label' => 'mobile', + 'mediaQuery' => '(min-width: 0px)', 'weight' => 0, 'multipliers' => array( '1x', ), 'provider' => 'breakpoint_theme_test', - 'id' => 'breakpoint_theme_test.tv', + 'id' => 'breakpoint_theme_test.mobile', 'group' => 'breakpoint_theme_test', 'class' => 'Drupal\\breakpoint\\Breakpoint', ), - 'breakpoint_theme_test.wide' => array( - 'label' => 'wide', - 'mediaQuery' => '(min-width: 851px)', + 'breakpoint_theme_test.narrow' => array( + 'label' => 'narrow', + 'mediaQuery' => '(min-width: 560px)', 'weight' => 1, 'multipliers' => array( '1x', ), 'provider' => 'breakpoint_theme_test', - 'id' => 'breakpoint_theme_test.wide', + 'id' => 'breakpoint_theme_test.narrow', 'group' => 'breakpoint_theme_test', 'class' => 'Drupal\\breakpoint\\Breakpoint', ), - 'breakpoint_theme_test.narrow' => array( - 'label' => 'narrow', - 'mediaQuery' => '(min-width: 560px)', + 'breakpoint_theme_test.wide' => array( + 'label' => 'wide', + 'mediaQuery' => '(min-width: 851px)', 'weight' => 2, 'multipliers' => array( '1x', ), 'provider' => 'breakpoint_theme_test', - 'id' => 'breakpoint_theme_test.narrow', + 'id' => 'breakpoint_theme_test.wide', 'group' => 'breakpoint_theme_test', 'class' => 'Drupal\\breakpoint\\Breakpoint', ), - 'breakpoint_theme_test.mobile' => array( - 'label' => 'mobile', - 'mediaQuery' => '(min-width: 0px)', + 'breakpoint_theme_test.tv' => array( + 'label' => 'tv', + 'mediaQuery' => 'only screen and (min-width: 1220px)', 'weight' => 3, 'multipliers' => array( '1x', ), 'provider' => 'breakpoint_theme_test', - 'id' => 'breakpoint_theme_test.mobile', + 'id' => 'breakpoint_theme_test.tv', 'group' => 'breakpoint_theme_test', 'class' => 'Drupal\\breakpoint\\Breakpoint', ), @@ -102,7 +102,7 @@ public function testCustomBreakpointGroups () { 'breakpoint_theme_test.group2.narrow' => array( 'label' => 'narrow', 'mediaQuery' => '(min-width: 560px)', - 'weight' => 2, + 'weight' => 0, 'multipliers' => array( '1x', '2x', @@ -128,7 +128,7 @@ public function testCustomBreakpointGroups () { 'breakpoint_module_test.breakpoint_theme_test.group2.tv' => array( 'label' => 'tv', 'mediaQuery' => '(min-width: 6000px)', - 'weight' => 0, + 'weight' => 2, 'multipliers' => array( '1x', ), @@ -153,7 +153,7 @@ public function testModuleBreakpoints() { 'breakpoint_module_test.mobile' => array( 'label' => 'mobile', 'mediaQuery' => '(min-width: 0px)', - 'weight' => 1, + 'weight' => 0, 'multipliers' => array( '1x', ), @@ -165,7 +165,7 @@ public function testModuleBreakpoints() { 'breakpoint_module_test.standard' => array( 'label' => 'standard', 'mediaQuery' => '(min-width: 560px)', - 'weight' => 0, + 'weight' => 1, 'multipliers' => array( '1x', '2x', @@ -178,9 +178,7 @@ public function testModuleBreakpoints() { ); $breakpoints = \Drupal::service('breakpoint.manager')->getBreakpointsByGroup('breakpoint_module_test'); - foreach ($expected_breakpoints as $id => $expected_breakpoint) { - $this->assertEqual($expected_breakpoint, $breakpoints[$id]->getPluginDefinition()); - } + $this->assertEqual(array_keys($expected_breakpoints), array_keys($breakpoints)); } /** diff --git a/core/modules/breakpoint/tests/modules/breakpoint_module_test/breakpoint_module_test.breakpoints.yml b/core/modules/breakpoint/tests/modules/breakpoint_module_test/breakpoint_module_test.breakpoints.yml index e1edeb2356c8..c490e5f6f557 100644 --- a/core/modules/breakpoint/tests/modules/breakpoint_module_test/breakpoint_module_test.breakpoints.yml +++ b/core/modules/breakpoint/tests/modules/breakpoint_module_test/breakpoint_module_test.breakpoints.yml @@ -1,12 +1,12 @@ breakpoint_module_test.mobile: label: mobile mediaQuery: '(min-width: 0px)' - weight: 1 + weight: 0 # Don't include multipliers. A 1x multiplier this will be enforced by default. breakpoint_module_test.standard: label: standard mediaQuery: '(min-width: 560px)' - weight: 0 + weight: 1 # Don't include a 1x multiplier this will be enforced by default. multipliers: - 2x @@ -15,7 +15,7 @@ breakpoint_module_test.standard: breakpoint_module_test.breakpoint_theme_test.group2.tv: label: tv mediaQuery: '(min-width: 6000px)' - weight: 0 + weight: 2 multipliers: - 1x group: breakpoint_theme_test.group2 diff --git a/core/modules/breakpoint/tests/themes/breakpoint_theme_test/breakpoint_theme_test.breakpoints.yml b/core/modules/breakpoint/tests/themes/breakpoint_theme_test/breakpoint_theme_test.breakpoints.yml index 3e565e18f935..f07aba8dc0f6 100644 --- a/core/modules/breakpoint/tests/themes/breakpoint_theme_test/breakpoint_theme_test.breakpoints.yml +++ b/core/modules/breakpoint/tests/themes/breakpoint_theme_test/breakpoint_theme_test.breakpoints.yml @@ -1,32 +1,32 @@ breakpoint_theme_test.mobile: label: mobile mediaQuery: '(min-width: 0px)' - weight: 3 + weight: 0 multipliers: - 1x breakpoint_theme_test.narrow: label: narrow mediaQuery: '(min-width: 560px)' - weight: 2 + weight: 1 multipliers: - 1x # Out of order breakpoint to test sorting. breakpoint_theme_test.tv: label: tv mediaQuery: 'only screen and (min-width: 1220px)' - weight: 0 + weight: 3 multipliers: - 1x breakpoint_theme_test.wide: label: wide mediaQuery: '(min-width: 851px)' - weight: 1 + weight: 2 multipliers: - 1x breakpoint_theme_test.group2.narrow: label: narrow mediaQuery: '(min-width: 560px)' - weight: 2 + weight: 0 multipliers: - 1x - 2x diff --git a/core/modules/responsive_image/responsive_image.module b/core/modules/responsive_image/responsive_image.module index bae2aecc8117..e7e246501079 100644 --- a/core/modules/responsive_image/responsive_image.module +++ b/core/modules/responsive_image/responsive_image.module @@ -133,8 +133,13 @@ function template_preprocess_responsive_image(&$variables) { $image = \Drupal::service('image.factory')->get($variables['uri']); $responsive_image_style = ResponsiveImageStyle::load($variables['responsive_image_style_id']); - // All breakpoints and multipliers. - $breakpoints = \Drupal::service('breakpoint.manager')->getBreakpointsByGroup($responsive_image_style->getBreakpointGroup()); + // Retrieve all breakpoints and multipliers and reverse order of breakpoints. + // By default, breakpoints are ordered from smallest weight to largest: + // the smallest weight is expected to have the smallest breakpoint width, + // while the largest weight is expected to have the largest breakpoint + // width. For responsive images, we need largest breakpoint widths first, so + // we need to reverse the order of these breakpoints. + $breakpoints = array_reverse(\Drupal::service('breakpoint.manager')->getBreakpointsByGroup($responsive_image_style->getBreakpointGroup())); foreach ($responsive_image_style->getKeyedImageStyleMappings() as $breakpoint_id => $multipliers) { if (isset($breakpoints[$breakpoint_id])) { $variables['sources'][] = responsive_image_build_source_attributes($image, $variables, $breakpoints[$breakpoint_id], $multipliers); diff --git a/core/modules/responsive_image/src/ResponsiveImageStyleForm.php b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php index 5763bc85f739..d7df1341e8b8 100644 --- a/core/modules/responsive_image/src/ResponsiveImageStyleForm.php +++ b/core/modules/responsive_image/src/ResponsiveImageStyleForm.php @@ -109,7 +109,12 @@ public function form(array $form, FormStateInterface $form_state) { '#required' => TRUE, ); - $breakpoints = $this->breakpointManager->getBreakpointsByGroup($responsive_image_style->getBreakpointGroup()); + // By default, breakpoints are ordered from smallest weight to largest: + // the smallest weight is expected to have the smallest breakpoint width, + // while the largest weight is expected to have the largest breakpoint + // width. For responsive images, we need largest breakpoint widths first, so + // we need to reverse the order of these breakpoints. + $breakpoints = array_reverse($this->breakpointManager->getBreakpointsByGroup($responsive_image_style->getBreakpointGroup())); foreach ($breakpoints as $breakpoint_id => $breakpoint) { foreach ($breakpoint->getMultipliers() as $multiplier) { $label = $multiplier . ' ' . $breakpoint->getLabel() . ' [' . $breakpoint->getMediaQuery() . ']'; diff --git a/core/modules/toolbar/toolbar.breakpoints.yml b/core/modules/toolbar/toolbar.breakpoints.yml index df9f61b5cac6..a68784a80e60 100644 --- a/core/modules/toolbar/toolbar.breakpoints.yml +++ b/core/modules/toolbar/toolbar.breakpoints.yml @@ -1,7 +1,7 @@ toolbar.narrow: label: narrow mediaQuery: 'only screen and (min-width: 16.5em)' - weight: 2 + weight: 0 multipliers: - 1x toolbar.standard: @@ -13,6 +13,6 @@ toolbar.standard: toolbar.wide: label: wide mediaQuery: 'only screen and (min-width: 61em)' - weight: 0 + weight: 2 multipliers: - 1x diff --git a/core/themes/bartik/bartik.breakpoints.yml b/core/themes/bartik/bartik.breakpoints.yml index cf28bed9cff5..66a8a74f6819 100644 --- a/core/themes/bartik/bartik.breakpoints.yml +++ b/core/themes/bartik/bartik.breakpoints.yml @@ -1,7 +1,7 @@ bartik.mobile: label: mobile mediaQuery: '' - weight: 2 + weight: 0 multipliers: - 1x bartik.narrow: @@ -13,6 +13,6 @@ bartik.narrow: bartik.wide: label: wide mediaQuery: 'all and (min-width: 851px)' - weight: 0 + weight: 2 multipliers: - 1x diff --git a/core/themes/seven/seven.breakpoints.yml b/core/themes/seven/seven.breakpoints.yml index 9cc1fda404f0..1b6bd2fb43ea 100644 --- a/core/themes/seven/seven.breakpoints.yml +++ b/core/themes/seven/seven.breakpoints.yml @@ -1,12 +1,12 @@ seven.mobile: label: mobile mediaQuery: '(min-width: 0em)' - weight: 1 + weight: 0 multipliers: - 1x seven.wide: label: wide mediaQuery: 'screen and (min-width: 40em)' - weight: 0 + weight: 1 multipliers: - 1x diff --git a/core/themes/stark/stark.breakpoints.yml b/core/themes/stark/stark.breakpoints.yml index 92d33b297a34..d1cdd9b4ee21 100644 --- a/core/themes/stark/stark.breakpoints.yml +++ b/core/themes/stark/stark.breakpoints.yml @@ -1,7 +1,7 @@ stark.mobile: label: mobile mediaQuery: '(min-width: 0px)' - weight: 2 + weight: 0 multipliers: - 1x stark.narrow: @@ -13,6 +13,6 @@ stark.narrow: stark.wide: label: wide mediaQuery: 'all and (min-width: 960px)' - weight: 0 + weight: 2 multipliers: - 1x -- GitLab