4
\$\begingroup\$

I wanted to use <select multiple> but was annoyed by the poor design on desktops which is why I created a version with dropdown:

convertSelect("001", "Options");

function convertSelect(el_id, name) {
    let el = document.getElementById(el_id),
        opts = Array.from(el.options);

    let input_el = document.createElement('input');
    input_el.setAttribute('id', el_id + '_input');
    input_el.setAttribute('type', 'text');
    input_el.setAttribute('autocomplete', 'off');
    input_el.setAttribute('readonly', 'readonly');
    input_el.addEventListener('focus', () => document.getElementById(el_id + '_span').style.display = "");
    input_el.addEventListener('blur', () => blur(el_id));
    el.parentNode.insertBefore(input_el, el.nextSibling);

    let span_el = document.createElement('span');
    span_el.setAttribute('id', el_id + '_span');
    span_el.setAttribute('style', `min-width:${(input_el.offsetWidth-2)}px;margin-top:${input_el.offsetHeight}px;margin-left:-${input_el.offsetWidth}px;position:absolute;border:1px solid grey;display:none;z-index:9999;text-align:left;background:white;max-height:130px;overflow-y:auto;overflow-x:hidden;`);
    span_el.addEventListener('mouseout', () => blur(el_id));
    span_el.addEventListener('click', () => document.getElementById(el_id + '_input').focus());
    input_el.parentNode.insertBefore(span_el, input_el.nextSibling);

    opts.forEach(opt => {
        let i = opts.indexOf(opt);

        let temp_label = document.createElement('label');
        temp_label.setAttribute('for', el_id + '_' + i);

        let temp_input = document.createElement('input');
        temp_input.setAttribute('style', 'width:auto;');
        temp_input.setAttribute('type', 'checkbox');
        temp_input.setAttribute('id', el_id + '_' + i);
        temp_input.checked = opt.selected;
        temp_input.disabled = opt.disabled || el.disabled;
        temp_input.addEventListener('change', () => check(el_id, name));

        temp_label.appendChild(temp_input);
        temp_label.appendChild(document.createTextNode(opt.textContent));
        span_el.appendChild(temp_label);
    });
    el.style.display = 'none';
    check(el_id, name);
}


function blur(el_id) {
    let el = document.getElementById(el_id);
    clearTimeout(parseInt(el.dataset.timer));
    el.dataset.timer = setTimeout(() => {
        if (document.activeElement.id !== el_id + '_input' && document.activeElement.id !== el_id + '_span')
            document.getElementById(el_id + '_span').style.display = "none";
    }, 200).toString();
}

function check(el_id, name) {
    let el = document.getElementById(el_id),
        opts = Array.from(el.options),
        select_qty = 0,
        select_name;

    opts.forEach(opt => {
        let i = opts.indexOf(opt),
            checkbox = document.getElementById(`${el_id}_${i}`);

        el.options[i].selected = checkbox.checked;
        if (checkbox.checked) {
            select_name = checkbox.parentElement.childNodes[1].textContent;
            select_qty++;
        }
        document.getElementById(`${el_id}_input`).value = select_qty < 1 ? '' : (select_qty > 1 ? `${select_qty} ${name}` : select_name);
    });

    el.dispatchEvent(new Event('change', { 'bubbles': true }));
}
label {
  display: block;
}

input[type="text"]:hover {
  cursor: default;
}
<select id="001" multiple>
  <option value="2">Option Two</option>
  <option value="4">Option Four</option>
  <option value="6">Option Six</option>
  <option value="8" disabled>Disabled Option</option>
</select>

As I'm not very experienced with DOM-Manipulation I would be glad if someone could check my code for any unnecessary complexion, possible simplifications and mistakes. Any improvement tip or critique is helpful!

PS: Especially the blur-function with the timer is bugging me as it feels like a hack.

Edit: The updated version can be found at https://codepen.io/MiXT4PE/pen/OYLRpz

\$\endgroup\$
1
\$\begingroup\$

Overall Feedback

The UI looks good. I think the code is okay, but the number of DOM lookups is higher than it needs to be. Those could be reduced by storing the checkboxes in an array. While browsers have come a long way in terms of efficiency, DOM lookups are still not cheap. Since features like template literals are used, a class or else a simple object could be used to store references to the newly created elements, timers, etc. in arrays, rather than querying the DOM to access elements.


Targeted Feedback

The ES-6 spread syntax can be used instead of using Array.from() to put elements into an array.

Lines like this:

opts = Array.from(el.options)

Can be simplified to just:

opts = [...el.options]

This requires one less function call.


When using Array.forEach() there are more parameters passed to the callback function than just the current element. Both occurrences of

opts.forEach(opt => {
    let i = opts.indexOf(opt);

Could have the call to opts.indexOf() eliminated by utilizing the second parameter instead.

opts.forEach((opt, i) => {

The code uses let for most all variables. Many of those variables never get re-assigned. It is wise to use const for any such variable, to avoid accidental re-assignment.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.