1

Having a very weird issue with a simple div visibility toggle script.

I'm just using javascript to switch a div between 'display: block' and 'display: none' to toggle its visibility. Very routine stuff.

And in general it works, but it always fails on the first click after a fresh page load. Then it works consistently from the second click onward.

No error output on console.

Relevant HTML:

<!DOCTYPE HTML>

<html>
    
<head>
    
<script type="text/javascript" src="res/classes.js"></script>
<script type="text/javascript" src="res/util_c.js"></script>
    
    <script type="text/javascript">

        // load client prefs
        var clientPrefs = new ClientPrefs();
        
        
    </script>

</head>

<body>

<a id="join_show_publist" class="a_btn" onClick="javascript:joinPublistToggle()">View Public Matches</a><br />

<!-- list of public games -->
<div id="join_publist_container" class="ovr">
    <table id="join_publist_listbox">
        
        <tr id="join_publist_listbox_header">
            <td>Table Name</td>
            <td>Open</td>               <!-- open seats remaining at table -->
        </tr>
        
    </table>
    
    <div class="spacer"></div>
    
    <div id="join_savePref_container">
        <input id="join_savePref" type=checkbox onclick="javascript:clientPrefs.joinAlwaysShowPubToggle()" /> 
        <span id="join_savePref_label" onclick="javascript:clientPrefs.joinAlwaysShowPubToggle()">Always show public tables list</span>
    </div>
    
</div>

Relevant CSS:

div.ovr {
    display: none;
}

...and finally in util_c.js:

// toggle visibility of public tables list
function joinPublistToggle() {
    var listContainer = document.getElementById('join_publist_container');
    if (listContainer.style.display == 'none') {
        listContainer.style.display = 'block';
    } else {
        listContainer.style.display = 'none';
    }
}

First click: nothing happens. Second click: the DIV is shown Third click: the DIV is re-hidden etc..

If I put an alert(listContainer.style.display) into the joinPublistToggle function, the alert comes up empty with the first click, then shows 'none' with the second click.

But the CSS specifically sets the display style for that div as 'none' on load. And if I look at that div in the page inspector after a fresh page load the inspector specifically says the div's display property is set to none.

So the issue seems to be that javascript is reading that property as empty even though that property is set as 'none'.

Why would it do that?

1337ingDisorder
  • 821
  • 1
  • 6
  • 17
  • *"And in general it works, but it always fails on the first click after a fresh page load. Then it works consistently from the second click onward."* You are binding three click event handlers, which one are you talking about? The error seems unrelated to that behavior because there is only one place where you reference `ClientPrefs` and that line is only executed *once* on page load. So it seems unlikely that it has anything to do with click events. – Felix Kling Sep 16 '21 at 18:54
  • You are using HTML Doctype 4.01 instead of 5. Is it really necessary ? You can check if the classes.js is loaded before or after your script tag, by adding a console.log in both. – Nice Books Sep 16 '21 at 19:23
  • Thanks @NiceBooks, I actually just copypasted that from an older project haha. I've updated that to just and I'm getting the same results. – 1337ingDisorder Sep 16 '21 at 19:41
  • Look at the console carefully, it doesn't say "Uncaught ReferenceError: ClientPrefs is not defined". Instead, it says "Uncaught Reference Error:joinShowPubList is not defined". –  Sep 16 '21 at 19:42
  • @FelixKling > "You are binding three click event handlers, which one are you talking about?" -- in this line: View Public Matches
     – 1337ingDisorder Sep 16 '21 at 19:43
  • @1337ingDisorder, the code is View Public Matches
    , not View Public Matches
    . joinShowPubList() in no where defined in op's code.     –  Sep 16 '21 at 19:49
  • @TobyHarnish On mine it did literally say "Uncaught ReferenceError: ClientPrefs is not defined" -- however having updated the doctype from 4.01 to 5 at NiceBooks's suggestion I'm no longer getting that error. (But the first click on the anchor that calls javascript:joinPublistToggle() is still being ignored, so I guess FelixKing is correct that it's an unrelated issue -- maybe I should create a second post for that?) – 1337ingDisorder Sep 16 '21 at 19:50
  • @TobyHarnish that line of code is immediately after the tag in OP's code – 1337ingDisorder Sep 16 '21 at 19:52
  • I was using firefox. Perhaps firefox displays the error differently. –  Sep 16 '21 at 19:57
  • I've edited OP to strip out all the class-related stuff, as that was unrelated to the click problem and has been solved by updating the doctype. Still hoping to find out why the first click is being dropped. – 1337ingDisorder Sep 16 '21 at 19:58
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/237192/discussion-between-toby-harnish-and-1337ingdisorder). –  Sep 17 '21 at 00:55

3 Answers3

1

And in general it works, but it always fails on the first click after a fresh page load. Then it works consistently from the second click onward.

I am going to asume when you clicked the link, the checkbox and the table should go away. And when it is clicked again, the table and the checkbox should show. I modified your code and it works for me. for your HTML:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
   "http://www.w3.org/TR/html4/strict.dtd">

<html>
    
<head>
    
<script type="text/javascript" src="classes.js"></script>
<script type="text/javascript" src="util_c.js"></script>
    
    <script type="text/javascript">

        // load client prefs
        var clientPrefs = new ClientPrefs();
        
        
    </script>

</head>

<body>

<a id="join_show_publist" class="a_btn" onClick="javascript:joinPublistToggle()">View Public Matches</a><br />

<!-- list of public games -->
<div id="join_publist_container" class="ovr">
    <table id="join_publist_listbox">
        
        <tr id="join_publist_listbox_header">
            <td>Table Name</td>
            <td>Open</td>               <!-- open seats remaining at table -->
            <td>Starts</td>             <!-- time left until game starts (or "started" if underway) -->
            <td>Timer</td>              <!-- time limit for turns (or "none") -->
            <td>Min</td>                <!-- min players to start the round (or '--' if already underway) -->
            <td>Late</td>               <!-- whether late joiners are allowed at the table -->
            <td>AI</td>                 <!-- whether there are any AI players at the table (and if so, how many)
                                             also colour denotes difficulty: green-easy / yellow-med / red-hard -->
        </tr>
        
        <!-- Generate list via js. Clicking any list entry joins -->
        
    </table>
    
    <div class="spacer"></div>
    
    <div id="join_savePref_container">
        <input id="join_savePref" type=checkbox onclick="javascript:clientPrefs.joinAlwaysShowPubToggle()" /> 
        <span id="join_savePref_label" onclick="javascript:clientPrefs.joinAlwaysShowPubToggle()">Always show public tables list</span>
    </div>
    
</div>

Classes.js:

// ClientPrefs - client-side preferences 
class ClientPrefs {
    
    constructor() {
        
        // JOIN GAME page settings
        this.joinAlwaysShowPub = false;         
        
    }
    
    joinAlwaysShowPub() { return joinAlwaysShowPub; }
    
    joinAlwaysShowPubToggle() {
        
        // toggle setting in memory
        this.joinAlwaysShowPub = !this.joinAlwaysShowPub;
        
        // update checkbox & label
        document.getElementById('join_savePref').checked = this.joinAlwaysShowPub;
        
    }
    
}

And finally your other script:

function joinPublistToggle() {
    var listContainer = document.getElementById('join_publist_container');
    if (listContainer.style.display == 'none') {
        listContainer.style.display = 'block';
    } else {
        listContainer.style.display = 'none';
    }
}

Here are few reasons why your code might not work:

  1. I think the problem is that you mistyped joinPublistToggle() to joinShowPubList.
  2. Your div has a value of nothing for the display property. So, when JS looks at your div, well, the div is not set to none or block, I don't know how to handle it. After you clicked the link a second time, it sets the display in your JS code. So, it knows how to handle it.

Maybe add an display property to your a tag and set it to block so JS know what the property of the style is.

<a id="join_show_publist" class="a_btn" onClick="javascript:joinPublistToggle()" style="display:block;">View Public Matches</a><br />
  • Thanks for that suggestion. I had corrected that shortly after posting, but still getting same results. I've updated OP to reflect the code in place now. – 1337ingDisorder Sep 16 '21 at 20:13
  • Also worth noting I'm getting the same results in chrome and firefox -- both ignore the first click but show the div with the second click. – 1337ingDisorder Sep 16 '21 at 20:13
  • @1337ingDisorder, your div has the value of nothing for the style. Your if...else statement only looks for none or block. So, when it sees "nothing", it doesn't know how to handle it. I have updated my answer to explain it more. –  Sep 16 '21 at 20:44
  • @1337ingDisorder: More precisely: `.style` returns the **inline** style of the element and as mentioned, the element doesn't have an inline style defined. There are a couple of ways to solve this: Explicitly check for an empty value like you did. Add the inline style like Toby did, or use classes to toggle the visibility. – Felix Kling Sep 17 '21 at 13:20
1

style returns the inline style of the element, and your element doesn't have any, which is why listContainer.style.display returns an empty string and the condition fails. It would work if you compared against 'block' instead but it's not really more reliable.

function joinPublistToggle() {
    var listContainer = document.getElementById('join_publist_container');
    if (listContainer.style.display == 'block') {
        listContainer.style.display = 'none';
    } else {
        listContainer.style.display = 'block';
    }
}
https://stackoverflow.com/questions/69213611/js-onclick-event-ignores-first-click-works-for-every-subsequent-click/69224191#

The other answers provide valid solutions, here is another using classes:

CSS:

div.hidden {
    display: none;
}

HTML:

<div id="join_publist_container" class="ovr hidden">

(of course you can also just keep using ovr but I wasn't sure what that's for)

JS:

function joinPublistToggle() {
  document.getElementById('join_publist_container').classList.toggle('hidden');
}
Felix Kling
  • 795,719
  • 175
  • 1,089
  • 1,143
0

This doesn't really answer my question, but I've implemented a simple workaround by adding an OR statement into the JS function:

function joinPublistToggle() {
    var listContainer = document.getElementById('join_publist_container');
    if ( (listContainer.style.display == 'none') || 
         (listContainer.style.display == '' ) ) {
        listContainer.style.display = 'block';
    } else {
        listContainer.style.display = 'none';
    }
}

This doesn't explain why it was behaving so odd, and it isn't a proper solution (as a proper solution would address the cause, not the symptom).

But it works.

I won't mark the post as solved just yet in case any wizards end up reading this and are able to explain why the problem occurred in the first place.

1337ingDisorder
  • 821
  • 1
  • 6
  • 17