shopware / production

Shopware 6 production template
https://shopware.com
177 stars 151 forks source link

Adding another grid-breakpoint (viewport) breaks the ProductSlider #137

Closed RafaelKr closed 2 years ago

RafaelKr commented 2 years ago

PHP Version

7.4

Shopware Version

6.4.4.1

Expected behaviour

I created a theme which inherits from the Responsive theme (actually there is an intermediate theme which inherits from the Responsive theme, but that shouldn't matter).

I now added a new breakpoint in my overrides.scss

// Grid breakpoints
//
// Define the minimum dimensions at which your layout will change,
// adapting to different screen sizes, for use in media queries.

$grid-breakpoints: (
    xs: 0,
    sm: 576px,
    md: 768px,
    lg: 992px,
    xl: 1200px,
    2xl: 1920px,
);

// Grid containers
//
// Define the maximum width of `.container` for different screen sizes.

$container-max-widths: (
    sm: 540px,
    md: 720px,
    lg: 960px,
    xl: 1140px,
    2xl: 1400px,
);

Everything should work as before.

Actual behaviour

Unfortunately now the product slider component is not initialized when the page is initially loaded with a width >= 1920px (breakpoint 2xl). When the page is loaded with < 1920px everything works as expected. I can even resize to over 1920px then and it doesn't break, it still updates the visible slide count when resizing back between the other breakpoints.

When loading the page with >= 1920px it shows the console error Uncaught TypeError: can't convert undefined to object.

I also tried adding my breakpoint with the following block in base.html.twig (https://github.com/shopware/platform/blob/ceee4b728a115177a3c0ecdb0342df18fd03703a/src/Storefront/Resources/views/storefront/base.html.twig#L140)

{# @deprecated tag:v6.5.0 - Will be moved to `layout/meta.html.twig` - Use block `layout_head_javascript_breakpoints` instead #}
{% block base_script_breakpoints %}
    {{ parent() }}

    <script>
        window.breakpoints['2xl'] = 1920;
    </script>
{% endblock %}

My debugging session revealed that the getCurrentViewport method of the ViewportDetection helper returns none for the new breakpoint (even when adding it with the base_script_breakpoints block). https://github.com/shopware/platform/blob/ceee4b728a115177a3c0ecdb0342df18fd03703a/src/Storefront/Resources/app/storefront/src/helper/viewport-detection.helper.js#L155

This eventually breaks the slider with can't convert undefined to object which is thrown in the method getViewportSettings of the SliderSettingsHelper when deepmerge tries to merge an obect with undefined: https://github.com/shopware/platform/blob/01d2da25be8d437bae0e6617c6e11b102c9ece6d/src/Storefront/Resources/app/storefront/src/plugin/slider/helper/slider-settings.helper.js#L13

I was able to hack a workaround for this case by using this block instead:

{# @deprecated tag:v6.5.0 - Will be moved to `layout/meta.html.twig` - Use block `layout_head_javascript_breakpoints` instead #}
{% block base_script_breakpoints %}
    {{ parent() }}

    <script>
        window.breakpoints['2xl'] = window.breakpoints['xl'];
        window.breakpoints['none'] = window.breakpoints['xl'];
    </script>
{% endblock %}

This sets breakpoint none to 1200 which doesn't break the slider.
Note: I also did this for my 2xl breakpoint, not sure if this could save me from another error anywhere. This is unrelated to the slider error.

How to reproduce

  1. Add another grid breakpoint in the overrides.scss as shown above. The breakpoint must be larger than 1200px.
  2. Add a product slider to a page.
  3. Open that page with a larger width than set for the new breakpoint. (4. ???) (5. Profit)
RafaelKr commented 2 years ago

closed due to wrong repository. Created the issue here: https://github.com/shopware/platform/issues/2320