2

I wrote some code that generates a bunch of html-elements based on a Json-Object. I add it to the page by JQuerys .append() and .after(). It does work perfectly often, but sometimes the outer loop is only executed once and stops at $( '#'+inputname ).entityselector().

function addlinks(qid, prop) {
    html="<fieldset id=\"quickpresets\">" +
         "<legend>Quick Presets (" + prop.name + ")</legend></fieldset>";
     $('.wikibase-statementgrouplistview').first().after( html );
    for( var p = 0; p < prop.defaults.length; p++ ) {
        pid=prop.defaults[p].pid;
        pname=prop.defaults[p].name;
        pvalues=prop.defaults[p].values;
        inputname="input"+pname;
        pclass="addstatement";
        if($('#P'+pid).find(".wikibase-snakview-value a").length !== 0) {
            pclass += " disabled";
        }
        str="<p class='"+pclass+"'>Add "+pname+":";
        for( var i = 0; i < pvalues.length; i++) {
            toqid=pvalues[i].qid;
            toname=pvalues[i].name;
            str += "&nbsp;<a href='javascript:void(0);' onclick=\""+
                   "additemstatement("+qid+","+pid+",'"+pname+"',"+ toqid +",'" + toname+ "')\">" + toname+ "</a>"+
                   "&nbsp;∙";
        }
        str += "<span class=\"quickpresetsinput\"><input id='"+inputname+"'/>&nbsp;";
        str += "<a href=\'javascript:void(0);\' onclick=\""+
                "onselectitem("+qid+","+pid+",'"+pname+"','"+ inputname +"')\">✔</a>";
        str += "</span></p>";
        $('#quickpresets').append( str );

        input = $( '#'+inputname ).entityselector( {
            url: 'https://www.wikidata.org/w/api.php',
            language: mw.config.get('wgUserLanguage')
        } );

    }
}

How do I fix this issue? And what other things are there that I should do to improve this ugly code?

Updates:

  • I get the following error in the console:

TypeError: $(...).entityselector is not a function [Weitere Informationen]

  • The full code can be found here.
  • I have to use ES5.
  • The data is always the same ("hard coded") JSON.
  • See below for the better readable version of Roamer-1888 – which still causes the same bug.
6
  • 2
    Did you try step by step debugging it within a browser dev tools? And in order how to improve: Try to separate your view (html, css) from your logic (javascript code). Commented Dec 27, 2017 at 1:42
  • add complete sections of the code that are interlinked with each other. provide relevant html and css Commented Dec 27, 2017 at 1:42
  • It's hard to know without the rest of the data. For example, could inputname be an invalid name (that has spaces inside or weird unicode)? By "sometimes the outer loop is only executed once" you mean that the HTML is only updated till that point or does the JavaScript stop at that point? What errors are shown in the console? On a side note, you know you need to sanitize the data (if you didn't write it) before adding it to your client's HTML, right? To prevent XSS and all sorts of problems Commented Dec 27, 2017 at 2:05
  • Dude, look into Template literals Commented Dec 27, 2017 at 2:16
  • @k0pernikus: Yes I tried – the bug never occurs when I do that. Commented Dec 27, 2017 at 9:57

1 Answer 1

1

It's not apparent in the code why .entityselector() should throw on some occasions. The most likely reason is that you are trying to invoke the plugin before it is loaded.

In an attempt to fix the issue, you might try initialising all the inputs in one hit instead of individually in the loop.

Also, the code would be made more readable by attaching a fairly simple fragment to the DOM, then immediately adding links and attaching event handlers with jQuery.

Here it is the way I would write it (with a bunch of tidying up) :

function addlinks(qid, prop) {

    // compose and intert fieldset.
    var $fieldset = $("<fieldset><legend>Quick Presets (" + prop.name + ")</legend></fieldset>")
        .insertAfter($('.wikibase-statementgrouplistview').first());

    // loop through prop.defaults
    prop.defaults.forEach(function(dflt) {
        // compose and append a basic fragment ...
        var $fragment = $("<p class='addstatement'>Add " + dflt.pname + ":<span class=\"links\"></span>"
            + "<span class=\"quickpresetsinput\"><input />&nbsp;<a href=\"#\">✔</a></span></p>")
            .appendTo($fieldset);

        // ... then allow jQuery to augment the appended fragment :
        // i) conditionally addClass('disabled')
        if($('#P' + dflt.pid).find(".wikibase-snakview-value a").length !== 0) {
            $fragment.addClass('disabled');
        }

        // ii) loop through dflt.values and add links.
        dflt.values.forEach(function(val) {
            $("<span>&nbsp;<a href=\"#\"></a>&nbsp;∙</span>")
            .appendTo($fragment.find('span.links'))
            .find('a')
            .text(val.name)
            .on('click', function(e) {
                e.preventDefault();
                additemstatement(qid, dflt.pid, dflt.pname, val.qid, val.name);
            });
        });

        // iii) attach click handlers to the quickpresets inputs
        $fragment.find('.quickpresetsinput').find('a').on('click', function(e) {
            e.preventDefault();
            var selection = $(this).prev('input').data('entityselector').selectedEntity();
            additemstatement(qid, dflt.pid, dflt.pname, selection.id.substring(1), selection.label);
        });
    });

    // invoke .entityselector() on all the quickpresets inputs in one hit
    $('.quickpresetsinput input').entityselector({
        'url': 'https://www.wikidata.org/w/api.php',
        'language': mw.config.get('wgUserLanguage')
    });
}

untested except for syntax

That's certainly tidier though without a proper understanding of the original issue, .entityselector() may still throw.

Sign up to request clarification or add additional context in comments.

2 Comments

Your code looks much nicer than mine did! Thank you. Sadly, the bug still occurs. (At least now the rest of the form is always loaded right) Any other ideas?
No further ideas right now. I suspect the problem is not to do with the code itself. Maybe the order of your <script> tags?

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.