Minor: unpositioned extension button in wrong place

[sjernigan]

Describe the bug
According to comment at svg_editor.js:3214
@Property {Integer} [position] The numeric index for placement; defaults to last position (as of the time of extension addition) if not present. For use with {@link http://api.jquery.com/eq/}.

However, I believe this case falls through to line 3276
$(parent).children().last().before(button);

Which adds it in the second to last position. I think this should be
$(parent).children().last().after(button);

[brettz9]

While many of the comments are not canon, and were just my quick attempt to make sense of the arguments (which in this case, it looks like I got wrong), it does seem like the description I have there would be the desired behavior, so I have made the change in master.

However, the current invocation of addExtension (which triggers the extension_added event) occurs asynchronously (for performance reasons) against the list of extensions (including the default extensions unless noDefaultExtensions is passed), so this behavior will most likely not be consistent. We could change the asynchronous Promise behavior, but I'm not sure this use case is compelling enough?

[sjernigan]

Thanks for the before/after fix. I have noticed the inconsistent ordering behavior and had it on my todo list to figure out at some point. Like you, I jotted it down as a problem but didn't prioritize the resolution as it's infrequent and not all that impactful.

[sjernigan]

Perhaps the items could be added to an oversized array asynchronously, only increasing the array size when a collision happens. After all the promises return, the array could be compressed and the items added to #main_menu ul. I'm sure it's not that simple as there are multiple button types being processed.

[sjernigan]

OK, I've got a pretty hacked up version of this repo now but I the following approach seems to work. First, I add a data-position attribute to the div if the extension has a preference. Then after the promises return, I sort the children of all elements with the class tools_panel. To be backwards compatible, I give preference to the items without a position...that is, they will remain unsorted at the top/left. I don't think it's the fastest way to sort this list but it seems to be working for me.

Here's the two code fragments

• Near the lines discussed above

       if ('position' in btn) {
         button.attr('data-position', btn.position)

• And around line 845, I added the following sort loop

  $.each($(".tools_panel"),function(index,list) {
    let switching = true;
    let shouldSwitch = false;
    /* Make a loop that will continue until
    no switching has been done: */
    while (switching) {
      // Start by saying: no switching is done:
      switching = false;
      let i = 0;
      let b = list.getElementsByTagName("DIV");
      // Loop through all list items:
      for (; i < (b.length - 1); i++) {
        // Start by saying there should be no switching:
        shouldSwitch = false;
        /* Check if the next item should
        switch place with the current item: */
        if ((b[i].getAttribute('data-position') != null) &&
            ((b[i + 1].getAttribute('data-position') == null) ||
             (parseInt(b[i].getAttribute('data-position')) > parseInt(b[i + 1].getAttribute('data-position'))))) {
          /* If next item is alphabetically lower than current item,
          mark as a switch and break the loop: */
          shouldSwitch = true;
          break;
        }
      }
      if (shouldSwitch) {
        /* If a switch has been marked, make the switch
        and mark the switch as done: */
        b[i].parentNode.insertBefore(b[i + 1], b[i]);
        switching = true;
      }
    };
  });