I screwed up by being too clever - an exploration

JavaScript
1

I’ve been helping someone by developing some categories/result selection code.

The trouble is, that even though the tests all pass with flying colors, and the tests do seem to all work, the user experience seems to be different from that which is intended.

The problem is that this code gets triggered, even when you are outside of the area, and are clicking things that aren’t checkboxes.

        $(container).on("click", ":checkbox", function (evt) {
            console.log("green area checkbox click");
            return clickHandler(evt);
        });

Some simplified example code is at http://jsfiddle.net/L82sgpcw/32/

Clicking on a category in the green section correctly shows and hides the appropriate result. But, clicking the result close button in the blue area has unexpected behaviour.

The first time you click a close button it actually triggers the green section click code, even though the whole results section is not in the same area as the green section event handler. The second time you click the close button the result properly goes away.

I spent several hours too much of time investigating the cause of the problem. In the end I found that it was caused by an earlier choice being too clever by half.

Please delve into the structured code to see if you can find the cause of the problem.

Why does the close button in the blue results section, trigger a category click event in the completely unrelated green section? I found it to be quite an educational process, and am pondering a solution or two to remedy the issue.

1 Like
2

I didn’t see any <form> tags. My guess is the browser is computing form tags before the first input and after the <button> creating an event area that you didn’t code.

3

I manually removed the form tags while simplifying, as they aren’t relevant to the problem being explored here. That is a clever idea though - I was stuck chasing a few eventual dead-ends when exploring this one too.

Even when form tags are included, the problem remains.

4

I get lost looking at the scroll code in a small pane and will need to copy it into my text editor to read it with any chance of “seeing” something.

Do you have, or could you throw together, some time of event type + target console logger?

I know dev tools can do breakpoints, but I’ve only tried it to see how it works, not to do any actual debugging. Maybe if you stepped through it?

5

When I tried to step through the code earlier, I ended up getting lost in a maze of jQuery code.

I’ll reveal what’s causing the actual problem tomorrow, to give people time to explore should they want to.

6

Yes, I imagine it’s similar to when I look at log files. So much there, most I don’t see why it’s there, and it always feels more serendipitous than skill when I do finally spot a lead.

7

Try the following code…

$(resultsArea).on(“click”, closeButton, function (evt) {
evt.stopPropogation();
uncheckCheckbox(evt);
});

8

That’s an interesting approach, but the categories item remains selected even though its result has been closed.

9

This is because you have also bound a second click event to the same button to un-check the checkbox, which you do by firing a click event at it:

    function uncheckByValue(value) {
        var $checkbox = getCheckboxByValue(value);
        if ($checkbox.is(":checked")) {
            $checkbox.click();
        }
    }

So what you’re actually doing is:
Click checkbox. This sets the checkbox to clicked, and then displays the div.
Click close box. This fires two events:
First event Hides the div.
Second event checks the checkbox’s status; because it’s checked, it clicks the checkbox again.
This fires the checkbox’s click event again. Which redisplays the div…

Why does this not happen the second time? The second time you click the button, the checkbox is not checked, so the second event doesnt fire a click at it.

How do you resolve it? closeClickHandler is entirely redundant, because the act of clicking the checkbox by the other event handles toggling the div. Remove closeClickHandler and its bind.

1 Like
10

While on the surface that seems to work, it does result in the isolated module failing to work, which is not a viable situation.

The actual problem stems from the searchresults code that adds a second close click handler to uncheck the checkbox.

There are two possible solutions that come to mind.

Solution 1: use a Force toggle

We can have the toggleResult function accept a force argument, so that the state of the checkbox determines whether the result is shown or not.

    function toggleResult(result, force) {
        if (force === true) {
          $(result).show();
        } else if (force === false) {
          $(result).hide();
        } else {
          $(result).toggle();
        }
    }
...
        function updateResults(checkbox) {
            var result = $("#" + checkbox.value);
            // resultsController.toggleResult(result);
            resultsController.toggleResult(result, checkbox.checked);
            ...
        }

Sample code using this force toggle code: http://jsfiddle.net/L82sgpcw/67/

Solution 2: use a beforeClose handler

A beforeClose handler means we no longer need to have multiple events being triggered on the close button. We can then toggle the checkbox first, before the result is removed.

    function closeClickHandler(evt, beforeClose) {
        var button = evt.target;
        var id = getName(button);
        if (typeof beforeClose === "function") {
            beforeClose(evt);
        }
        $("#" + id).hide();
    }
...
        $(closeButtonSelector).on("click", function (evt) {
            closeClickHandler(evt, opts.beforeClose);
        });
    }
...
            resultsController.init({
                resultSelector: resultSelector,
                closeButtonSelector: closeButton,
                beforeClose: uncheckCheckbox
            });
            // $(resultsArea).on("click", closeButton, function (evt) {
            //     uncheckCheckbox(evt);
            // });

Sample code for this beforeClose solution is found at http://jsfiddle.net/L82sgpcw/73/

The problem occurred because I doubled up on the close click event, which had be uncertain about it when I was doing it at the time.

I’m leaning closer to the beforeClose solution, as it removes the double event problem, and more clearly expresses the problem that’s needing to be solved.

11

You’re overengineering a problem that shouldn’t exist.

We’ve both come to the same conclusion - the problem is the doubleing up of event triggers. The correct solution to that problem is to REDUCE code, not add more.

If you want to isolate the opening/closing of the div from the checking of the checkbox, neither of your proposed solutions actually do that. Why you would want to do that i’m not entirely sure, but it doesnt do that.

The appropriate solution is to make the close button call the checkboxes, which is the controller of the divs.
The close button isnt the controller, because it cant open the div.

12

While normally I’d agree with this, that then ends up with the results close button needing to know about the categories checkboxes, and the results code then being incapable of working without the checkboxes.

I’m putting together a situation where the checkboxes work all by themself, the results work all by themself, and that they both work well together too.

13

Clearly i’m missing something or not understanding your definitions of items then, because your results dont appear unless the checkboxes get checked. Which means they DONT work all by themselves.

14

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.