Problem/Motivation

Many of Tiny Slider's options are not exposed as fields in the views configuration settings. Some settings are not relevant to Tiny Slider.

Steps to reproduce

Proposed resolution

Rather than exposing config as form API fields, just accept JSON.

Issue fork tiny_slider-3382375

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

grasmash created an issue. See original summary.

ipwa’s picture

ipwa commented

Marking #3327033: Possibility to make manual slider settings. as duplicate since this has more details.

grasmash’s picture

grasmash commented

I’m working on a merge request.

ipwa’s picture

ipwa commented

That sound great Matthew we really appreciate this contribution! We do want to keep the config we've exposed as fields we will be adding a lot more options but having the ability to add your own config as JSON sounds really great to us.

grasmash opened merge request !13

grasmash’s picture

grasmash commented

I've submitted the MR but I understand that the changes I made don't align with your plan. My approach was to replace existing config fields with a single JSON field. Alternatively, perhaps we could add a field like "Config mode" that lets the the can choose between defining config as JSON or as fields. Fields would then be displayed conditionally based on that decision.

As a matter of personal opinion, I like the simplicity of defining JSON rather than creating many fields. It seems like a lower maintenance effort and more future-proof approach.

ipwa’s picture

ipwa commented
Status: Active » Needs work

Alternatively, perhaps we could add a field like "Config mode" that lets the the user can choose between defining config as JSON or as fields. Fields would be displayed conditionally based on that decision.

That approach sounds good, if you add that to your merge request I'd be happy to review and merge. Thanks for your work it is appreciated!

grasmash opened merge request !14

grasmash closed merge request !13

grasmash reopened merge request !13

endrukk’s picture

endrukk commented
Status: Needs work » Needs review

I created a new merge request which is building on the one in #8. There is an optional Config JSON field in the settings, if it has a valid JSON object, Tiny slider settings are overwritten by the JSON.

endrukk’s picture

endrukk commented

I extended my merge request. There is an Advanced mode toggle which enables the free text field and hides other fields. A slight improvement on the UI.

brooke_heaton’s picture

brooke_heaton commented
Status: Needs review » Needs work

The MR failed and the patch is not applying to 1.1.x-dev.

ipwa’s picture

ipwa commented
Status: Needs work » Needs review

This is the merge request: https://git.drupalcode.org/project/tiny_slider/-/merge_requests/15

ipwa’s picture

ipwa commented
Status: Needs review » Reviewed & tested by the community

  • ipwa committed e4d3c9f0 on 1.1.x authored by endrukk 
    Issue #3382375 by grasmash, ipwa, endrukk: Change setting to accept JSON
ipwa’s picture

ipwa commented
Status: Reviewed & tested by the community » Fixed

Thanks Matthew for getting this started and Endre for bringing it over the line, great contributions!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

peter caritas’s picture

peter caritas commented

I'm attempting to use the advanced mode feature and manually override configuration with json settings. After saving the view, the json settings field is set to null and the view is unchanged. Although, the advanced mode checkbox remains checked. Am I using this correctly?

Here are the json settings I'm adding: 

[
    default_row_class: true,
    center: false,
    mode: carousel,
    prevButton: '.prev',
    nextButton: '.next',
    nav: true,
    navPosition: 'bottom',
    loop: true,
    items: 1,
    gutter: 0,
    slideBy: 1,
    autoplay: true,
    autoplayTimeout: 12000,
    autoplayHoverPause: true,
    itemsDesktop: 1,
    itemsMobile: 1,

    ]
bohemier’s picture

bohemier commented

@peter your json is invalid. Try this:

{
    "default_row_class": true,
    "center": false,
    "mode": "carousel",
    "prevButton": ".prev",
    "nextButton": ".next",
    "nav": true,
    "navPosition": "bottom",
    "loop": true,
    "items": 1,
    "gutter": 0,
    "slideBy": 1,
    "autoplay": true,
    "autoplayTimeout": 12000,
    "autoplayHoverPause": true,
    "itemsDesktop": 1,
    "itemsMobile": 1
}

see https://jsonlint.com/