1
\$\begingroup\$

I'm trying to display a form where a user can select a specific item from a list or a specific category of items from another list. I want them to select one or the other, not both.

I followed this question which solves how to handle this in the backend.

I still need to figure out how to properly show this in the view. I have the following sample which does what I would like to achieve, but I'm wondering if this is the way to go:

$('.formGroup input:checkbox').on('click', function() {
  var id = $(this).prop('id');
  var groups = $(this).parents('.formGroup').siblings('.formGroup');
  var thisGroup = $(this).parents('.formGroup');

  $('select', thisGroup).prop('disabled', false);

  $.each(groups, function(index, item) {
    var checkbox = $('input:checkbox', item);
    var select = $('select', item);

    checkbox.prop('checked', false);
    select.val(-1).prop('disabled', true);
  })
})
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="formGroup">
  <label>
    <input type="checkbox" value="" id="Option1">Select an Item</label>
  <select disabled="disabled">
    <option value=""></option>
    <option value="1">Item 1</option>
    <option value="2">Item 2</option>
    <option value="3">Item 3</option>
  </select>
</div>
<div class="formGroup">
  <label>
    <input type="checkbox" value="" id="Option2">Or select a Category</label>
  <select disabled="disabled">
    <option value=""></option>
    <option value="1">Category 1</option>
    <option value="2">Category 2</option>
    <option value="3">Category 3</option>
  </select>
</div>

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Possible problem of the current solution

If you check one of the checkboxes, it gets activated and the select-element becomes enabled. If you click the same checkbox again, you only uncheck it, but the select-element stays enabled. I'd say there are two possibilities here:

  • disable the select-element as well
  • don't let the user uncheck the input-element

In the following examples I went with the first option.
As requested in the comments, I've updated the review to work for the second option.

Reduce the number of DOM queries and cache selectors

You didn't say how many of those groups are present or how often a user is going to make this decision. But you do a lot of the same DOM queries in your event handler over and over again.

At first let's store the "group-selector" in a string, to make it re-usebale. Then select all groups:

const selector = '[data-form-group]';
const groups = $(selector);

You can stick to classes if you like. I tend to use data-*-attributes to couple JavaScript.


As we also often need the input- and the select-element in each group, let's store them as well:

groups.each(function() {
    const group = $(this);
    group.data('checkbox', group.find('input'));
    group.data('select', group.find('select'));
});

Now we can access the form elements anytime, without re-querying the DOM.

The actual event handler

Attach the event handler to each input-element, the checkboxes radio-buttons. Then test, whether the current group is "enabled", i.e. the input-element is checked:

If it is checked already

  • do nothing and simply return

If it is not checked

  • enable the select-element
  • run through all other groups and disable them

The final code could look like this:

const selector = '[data-form-group]';
const groups = $(selector);

groups.each(function() {
  const group = $(this);
  group.data('toggle', group.find('input'));
  group.data('options', group.find('select'));

  group.data('toggle').on('change', function() {
    const selectedGroup = $(this).closest(selector);

    if (!selectedGroup.data('toggle').is(':checked')) {
      return;
    }

    selectedGroup.data('options').prop('disabled', false);

    groups.each(function() {
      const current = $(this);

      if (selectedGroup[0] === current[0]) {
        return;
      }

      current.data('toggle').prop('checked', false);
      current.data('options').prop('disabled', true);
    });
  });
});

Try it yourself

\$\endgroup\$
9
  • \$\begingroup\$ I like it. Thanks. One thing i am not sure about is if i would like it more to not have the ability to de-select, so it behaves like a radio button. One of the selection is mandatory so no reason to de-select. I am also considering having a pre-selected option on load. And last thing is that i would remove the selection when another checkbox is selected, as this data is going to be posted and i would not like to send irrelevant data. \$\endgroup\$ Commented Oct 19, 2017 at 13:22
  • \$\begingroup\$ I have this jsfiddle.net/jyparask/9pd45rbx/1 based on your answer. Make note that the last line is only for a show case. In real life scenario i will handle this effect from the back end since it will depend on if it is a new entry so select the first option or an edit so select whichever option the user had selected. \$\endgroup\$ Commented Oct 19, 2017 at 13:55
  • \$\begingroup\$ I've updated the review and also the example at the end of it, to work with radio-buttons and with a pre-selected group. If you don't want to send disabled groups, remove them before sending the form – on submit. I hope this is helpful, @GiannisParaskevopoulos. \$\endgroup\$ Commented Oct 20, 2017 at 8:06
  • \$\begingroup\$ I think the radio buttons seem more natural. Do not get me wrong, i will accept your answer as it really does what i wanted. I do have three comments though. 1. I would rename checkbox in group.data('checkbox' to radio just to make it more obvious (i really got confused at first). 2. I think the part for checking if the radio is checked and return is unnecessary, as radio buttons work this way. 3. I would stick to clearing the selected option when changing the option in order to make clear to the user it won't send this value as well. jsfiddle.net/jyparask/q6ycfce2/1 \$\endgroup\$ Commented Oct 20, 2017 at 8:57
  • \$\begingroup\$ And a bit of question if i may. I see you declare const group and then inside the on click event you declare const group = $(this).closest(selector);. Now, i know the scope is different, but still, for readability's shake, wouldn't it be better if you had named it differently? I myself (no, i do not boast to be a super-duper developer) had to read it twice. \$\endgroup\$ Commented Oct 20, 2017 at 9:05

Your Answer

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

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