It is possible to reduce the code without losing data

it is possible to reduce the code without losing data ? Or maybe optimize. Here is the code

$('label[for="wb"]').show();
$('label[for="pp"]').show();
$('label[for="ed"]').show();
$('input[name="valuta"]').on('click', function () {
    if ($('input[value="eur"]').is(':checked')) {
        $('input[value="webmoney"]').prop('checked', true);
        $('label[for="wb"]').show();
        $('label[for="pp"]').show();
        $('label[for="ed"]').show();
    }
    if ($('input[value="usd"]').is(':checked')) {
        $('input[value="webmoney"]').prop('checked', true);
        $('label[for="wb"]').show();
        $('label[for="pp"]').show();
        $('label[for="ed"]').show();
    }
    if ($('input[value="rub"]').is(':checked')) {
        $('input[value="webmoney"]').prop('checked', true);
        $('label[for="wb"]').show();
        $('label[for="pp"]').show();
        $('label[for="ed"]').show();
    }
    if ($('input[value="nis"]').is(':checked')) {
        $('input[value="paypal"]').prop('checked', true);
        $('label[for="wb"]').hide();
        $('label[for="pp"]').show();
        $('label[for="ed"]').hide();
    }
});

And here is work example http://jsfiddle.net/Nazaret2005/g8nn3y26/

1 Like

Not really sure what you’re trying to do here, but the JS could be shortened to this without any change in functionality:

$('input[name="valuta"]').on('click', function () {
    $('input[value="webmoney"]').prop('checked', true);
    $('label[for="pp"]').show();
    if ($('input[value="nis"]').is(':checked')) {
        $('label[for="wb"]').hide();
        $('label[for="ed"]').hide();
    } else {
        $('label[for="wb"]').show();
        $('label[for="ed"]').show();       
    }
});
1 Like

I think that what he’s wanting is to remove much of the duplication, perhaps by extracting some of it out to a data format.

This is no proof of concept, but it’s a quick roughing out of a potential direction before I head off:

var paymentMethods = {
        'eur': {
            'webmoney': true,
            'paypal': true,
            ...
        }
    },
    methodLabels = {
        'webmoney': 'wb',
        'paypal': 'pp',
        ...
    }
function updatePaymentMethods(paymentMethods) {
    Object.keys(paymentMethods).forEach(function (method) {
        var isActive = paymentMethods[method];
        $('input[value="' + method + '"]').prop('checked', isActive);
        $('label[for="' + methodLabels[method] + '"]').show(isActive);
    });
}
function updateCurrencyPaymentTypes() {
    var currencies = ['eur', ...];
    currencies.forEach(function (currency) {
        if ($('input[value="' + currency + '"]').is(':checked')) {
            updatePaymentMethods(payentMethods[currency]);
        }
    });
}

$('input[name="valuta"]').on('click', updateCurrencyPaymentTypes);
2 Likes

Thanks for all, Pullo and Paul_Wilkins i love 2 ways, will check them soom )))

From your sample code, it looks like you want the NIS payment currency to show less payment type options, so here’s a way to apply different payment types to different currencies.

Do the currency payment types change depending on if valuta or other gateway types are being used? If not then things can be made even more concise.

Note: I don’t know how you have your form organised, so have created my own test variation.

Here’s the jsfiddle code in which to take a closer look:

var payments = {
    'valuta': {
        methods: {
            'eur': {
                'webmoney': true,
                'paypal': true,
                'ed': true
            },
            'usd': {
                'webmoney': true,
                'paypal': true,
                'ed': true
            },
            'rub': {
                'webmoney': true,
                'paypal': true,
                'ed': true
            },
            'nis': {
                'webmoney': true,
                'paypal': false,
                'ed': false
            }
        }
    },
    'other': {
        methods: {
            'eur': {
                'webmoney': true,
                'paypal': true,
                'ed': true
            },
                'usd': {
                'webmoney': true,
                'paypal': true,
                'ed': false
            },
                'rub': {
                'webmoney': false,
                'paypal': true,
                'ed': true
            },
                'nis': {
                'webmoney': true,
                'paypal': false,
                'ed': true
            }
        }
    }
};

function getLabel(method) {
    return $('[value="' + method + '"]').parents('label');
    }

    function updatePaymentTypes(paymentTypes) {
        Object.keys(paymentTypes).forEach(function (method) {
            var isActive = paymentTypes[method],
                $label = getLabel(method);

            $label.hide();
            $label.toggle(isActive);
        });
    }

    function setDefaultPaymentType() {
        var firstVisibleType = $('[name="type"]:visible:first');
        firstVisibleType.prop('checked', true);
    }

    function updateCurrencyPaymentTypes() {
        var currency = $('input[name="currency"]:checked').val(),
            gateway = $('input[name="gateway"]:checked').val(),
            paymentMethods;

        if (!gateway) {
            $('#paymenttype').hide();
            return;
        }

        $('#paymenttype').show();
        paymentMethods = payments[gateway].methods;

        updatePaymentTypes(paymentMethods[currency]);
        setDefaultPaymentType('type');
    }
    $('#payment')
        .on('click', updateCurrencyPaymentTypes)
        .trigger('click');
2 Likes

Wow, amazing work! Thanks you very much!

Two questions, after i try to little style the form, the last inputs (name=type) does not checked.
And when i add (inputs tag to tag lable it work (show and changes but without checked), but for style i need the inpute before the lable tag (and when i see checked inputs, but all other no work, no changes, just show all ways/options)

To understand this, like this (it show and changes without checked)

<fieldset id="paymenttype">
    <legend>Payment type</legend>
    <div class="pays">
        <label for="wb">
            <input type="radio" id="wb" name="type" value="webmoney">
            <img title="Webmoney" src="http://static.dhkeno.ru/pic/money/pay/webmoney.png">
        </label>
        <label for="pp">
            <input type="radio" id="pp" name="type" value="paypal">
            <img title="Paypal" src="http://static.dhkeno.ru/pic/money/pay/paypal.png">
        </label>
        <label for="ed">
            <input type="radio" id="ed" name="type" value="robokassa">
            <img title="Other" src="http://static.dhkeno.ru/pic/money/pay/robokassa.png">
        </label>
    </div>
</fieldset>

I need it work like this (when i can checked, but changes pay ways not work here)

<fieldset id="paymenttype">
    <legend>Payment type</legend>
    <div class="pays">
        <input type="radio" id="wb" name="type" value="webmoney">
        <label for="wb">
            <img title="Webmoney" src="http://static.dhkeno.ru/pic/money/pay/webmoney.png">
        </label>
        <input type="radio" id="pp" name="type" value="paypal">
        <label for="pp">
            <img title="Paypal" src="http://static.dhkeno.ru/pic/money/pay/paypal.png">
        </label>
        <input type="radio" id="ed" name="type" value="robokassa">
        <label for="ed">
            <img title="Other" src="http://static.dhkeno.ru/pic/money/pay/robokassa.png">
        </label>
    </div>
</fieldset>

Example http://jsfiddle.net/Nazaret2005/awxwsuaw/

Thanks for help again!

For the sake of simplicity, we can rename the data properties from webmoney to wb, so that we can use those to find the names of the appropriate label areas.

    ...
    'eur': {
        'wb': true,
        'pp': true,
        'ed': true
    },
    ...

Previously the code was looking to it’s parents for the label element, whereas with your changes we can use the id to find the appropriate label instead.

$label = $('[for="' + method + '"]');

Because it’s a less complex technique to find the label, you can also remove that getLabel function now.

1 Like

Oh so simple ))) Thanks again!

The setDefaultPaymentType not work for me, how to correct the function like if change the Payment type or Payment gateway that will checked the first input ? thanks )

thats was select first for all (without rule of changes Payment type)

$('.pays input[type=radio]:first').prop('checked', true);

Here is http://jsfiddle.net/Nazaret2005/awxwsuaw/

You’ve hidden those radio buttons so it’s not as easy not to figure out which one should be selected.

What you’ll be wanting to do instead, is to find the first visible label, get the input field related to that, and then make that input field visible.

Here - it’s all nicely laid out in this setDefaultPaymentType function:

function setDefaultPaymentType(section) {
    var firstVisibleLabel = $('label:visible:first', section),
        input = $('#' + firstVisibleLabel.attr('for'));
    input.prop('checked', true);
}

setDefaultPaymentType('.pays');

Hopefully still nice and simple, for simple is hard to break.

1 Like

Thank you it work, but now i can not change the checked input )))

(Look at dollar or euro)

We’ll put a condition around setting the default. It should only occur if the visible labels have no selected inputs.

if (noVisiblySelectedInputs('.pays')) {
    setDefaultPaymentType('.pays');
}

I’ll be back shortly with the noVisiblySelectedInputs function

1 Like

Aha - even better is to not do the above, and make the form updating more specific, so that it triggers only on the currencies and gateway sections.

$('.currencies, #gateways').on('click', updateCurrencyPaymentTypes).trigger('click');

And foreseeing your next comment, moving your submit-showing code to a separate piece just below the above on-click line:

$('.currencies, #gateways').on('click', updateCurrencyPaymentTypes).trigger('click');
$('.pays').on('click', function () {
    if ($('input[name="type"]').is(':checked')) {
        $('#submit').show();
    }
});
1 Like

And - you’ll need to move the submit-hide piece elsewhere, so that if someone chooses a different gateway for example, the submit goes away again until they select a payment option.

Making the change below will resolve that there for you.

if (!currency) {
    $('#gateways').hide();
    $('#paymenttype').hide();
    // $('#submit').hide(); remove this
} else {
    $('#gateways').show();
}
$('#submit').hide(); // add this
1 Like

@Paul_Wilkins - Thank you for all, now work perfect http://jsfiddle.net/Nazaret2005/awxwsuaw/6/ )))

1 Like

I think i do not need this

$('.pays').on('click', function () {
    if ($('input[name="type"]').is(':checked')) {
        $('#submit').show();
    }
});

Just add $(‘#submit’).show(); to end of setDefaultPaymentType function . And $(‘#submit’).hide(); before if (!currency) {

Here is it http://jsfiddle.net/Nazaret2005/awxwsuaw/7/ . And this is work fine . I am right ? )))

Now you are mixing together different aspects of the code, which can lead to greater complexity later on.

Before, it was relatively easy to understand that there are two main triggers for the form. Something happens when the currency or gateway is chosen, and something different needs to occur when the payment type of chosen.

After your change though, it’s not as easy for the code to tell you what’s going on or why. This is something called decoupling, or separation of concerns, and is vital to help keep the complexity down low.

When things get more complex, it becomes even harder for us to understand what the code is trying to achieve. Decoupling attempts to make each piece of code have only one main reason to change, which helps to simplify the maintenance of the code - especially in 6 months or a few years time when you’ve forgotten everything about how currently works.

1 Like

Thanks for the answer, and you are always right! )))