Breakpoint-specific options lost if same as defaults

Could you please dump your settings export? Thanks.

It didn't happen when I disable Mobile first option, but I did notice a screwup when it is. It was a new option I must have missed a proper test.
UPDATE: I spoke too soon, it is all right, too. I was just distracted by the change of the slides amount when toggling this particular option.

Regarding your patch, I tried to avoid putting dup markups if I can fix them using JS. I will see if JS can help with it when I can reproduce your issue.

I can not reproduce it yet, sorry.
If you could send me your optionset exports that will be helpful, so I can reproduce it.
Thanks.

Feel free to re-open if still an issue. Thanks.

This issue is still alive in 8.x

Here are the steps to reproduce:

1. Create a new option set and activate "mobile first"
2. Uncheck the "arrow" option
3. Create a breakpoint, for example 766px
4. For this breakpoint, activate the "arrow" option

If you try the slideshow on mobile, you have the arrow invisible, but they do not show on large displays...

When the data-slick is built, the data are cleaned against the Slick default values. But if those values have been overridden in the main configuration and reset to their default values in a breakpoint, the values in this last breakpoint are not kept.

This is caused by the comparison made in the method Drupal\slick\Entity\Slick::removeDefaultValues() with the Slick default values, when dealing with responsive breakpoints:
$cleaned[$key]['settings'] = array_diff_assoc($responsives[$key]['settings'], $defaults);

To fix this, there are 2 options:

• - simply not make an array_diff. The problem is that the result may be really verbose
• - make the diff using another reference, created from the default Slick options overridden by the main configuration

The second approach seems better, but I have a doubt about it: are the breakpoint settings piled up or the reference is always the main configuration? In the first case, it may be complicated to have the correct reference for each breakpoint as we would have to recreate the reference after processing each breakpoint, hoping the breakpoints to be processed in the correct order...
But if, as I think, the breakpoint options are always summed with the Slick default options, I could be really simple to fix this issue.

I do not have analysed the Slick JavaScript, so I do not know the right answer. Maybe a maintainer could give me the point.

Finally I had to analyse the JS files. And actually it appeared that the issue was not caused by the clean up of the settings, but by a JS callback that do not take care of the current active breakpoint.

The setPosition() callback use the o variable (main options) but not the correct options, that depend on the current breakpoint.

The patch below solves the issue. I provided the uncompressed JS and the minified one, but I certainly did not used the main minifier library as the module maintainer.

Wrong file... Here is the correct patch.

Thanks! Truly sorry for delay and inconvenience.
Allow me sometime to recap and tag pending patches first before I can commit anything as I have been away for quite a while.

Committed. Thanks!

The changes made here are breaking changes, it seems that this will destroy all existing slick optionsets using breakpoints.
Correct me if I'm wrong, but now we have to set all the settings from the "settings" (defaults) tab again in every single responsive tab. Otherwise even all settings in the responsive tabs, which have default values, will override the default values.

Example: If you have set "arrows" to true in the defaults tab and set up "nothing" in the breakpoint settings, the arrows will be FALSE on this breakpoint, because the checkbox isn't checked by default.

Hint: We always use the "mobile first" setting - this is probably relevant for testing.

To solve this, the form probably needs to be restructured? Checkboxes / booleans are a problem here, because you can't figure out what's a default value and what is a changed setting. So maybe change those checkboxes to selects. This way we can have something like "default" / "inherit".

If my expectations are true, this patch should be reverted to prevent broken sites.

Thanks!
Could anyone, or @B-Prod, kindly cross-check against @thomas.frobieter, please?
We can always revert, but not until hearing from the original patcher, or till I have time to see to it again, but likely a terrible later.
I will re-open this issue again for other contributors to give a hand.

To solve this, the form probably needs to be restructured? Checkboxes / booleans are a problem here, because you can't figure out what's a default value and what is a changed setting. So maybe change those checkboxes to selects. This way we can have something like "default" / "inherit".

This is a brilliant idea, thank you, as it offers greater possibility, yet more precise. Of course the select box should only be provided at breakpoints, not the main settings while keeping BC in mind.

Patches are very much welcome, as I may not work with this for quite a while.
Feel free to assign it to you if you are working with this. Thanks!

The proposed patch appeared to do the promised fix. Setting this back to fixed due to lack of activity.

The mentioned idea can go into another thread if anyone is interested.

Feel free to re-open if still an issue. Kindly provide more detailed re-production or accessible environment which can re-produce the already fixed issue. Thank you!