Javascript innerHTML not populating div through function

[elfynity1]

Hi everyone, I am so new to Javascript (a php developer). Please can someone show me why my two arrival divs are not populating:

<div class="box" onclick="changeText(this); getDate(arrival1, 2019-12-01);">
	1
</div>

<div class="box" onclick="changeText(this); getDate(arrival2, 2019-12-02);">
	2
</div>

<script>
function changeText(id) {
  id.classList.toggle("box-color");
}

function getDate(arrival, theDate) {
  document.getElementById("arrival").innerHTML="theDate";
}
</script>

<div id="arrival1">
</div>

<div id="arrival2">
</div>

[bwclovis]

So, couple things. First your not passing in an ID to your changeText method. I COULD be wrong but in that syntax I THINK your passing in the entire div (I don’t know because I wouldn’t set it up that way).

Second if it did work, your result would be “theDate” as your passing in as a string.

If I were to set this up it would look something like this:
dynamic date

[elfynity1]

Hi bwclovis, wow, your code works but I have no idea what it all means, I don’t know Javascript syntax well enough yet. All I actually have wanted to do is : I have a dynamically generated php calendar, and I want the user to be able to click on a date (which highlights that box) and then sends that date value to an input value where I can save it to a php POST variable. The user needs to be able to deselect their date too. The above can’t be done in PHP without a page reload. So what seemed simple has become rather complex for me to achieve.

[bwclovis]

No worries, I just updated my code with some comments.

Yeah date pickers are the WORST, lol. In JS world it wouldn’t be to over the top actually. Can they only select one date?

[bwclovis]

I think what you want would be something like this then:

date switching

PLEASE keep in mind this is rough as(if this is a true calendar) to mark it up as a table with all the A11y bells and whistles attached. Crap this is easier in React, lol.

[m3g4p0p]

@elfynity1

I have no idea what it all means, I don’t know Javascript syntax well enough yet

Hi @elfynity1 , it’s actually exactly as in PHP – anything in quotes is a string, and variable names do not have quotes around them (only that in JS you don’t need a sigil). So with

getDate(arrival1, 2019-12-01);

you’re trying to pass a variable named arrival1 (which is not defined) and the expression 2019-12-01 (which evaluates to the number 2006). Conversely, with

document.getElementById("arrival").innerHTML="theDate";

you’re querying for the element with the literal ID "arrival" (rather than the value that the arrival parameter holds), and attempting to assign the string "theDate".

BTW you might also want to check the console of your browser dev tools for errors, like those getting thrown when referencing variables that don’t exist. And as a general note, I would avoid inlining your JS in the HTML as it makes your code hard to read and maintain – note the syntax highlighting in this very post for a start. So better separate logic from markup by using addEventListener() rather than onlick attributes, so you don’t have to check your HTML for JS errors.

[Paul_Wilkins]

Starting with the initial code, inline HTML event attributes are bad, so move those down to actual JavaScript code.

Comments show code before the changes

<!--<div class="box" onclick="changeText(this); getDate(arrival1, 2019-12-01);">-->
<div class="box">
	1
</div>

<!--<div class="box" onclick="changeText(this); getDate(arrival2, 2019-12-02);">-->
<div class="box">
	2
</div>

const boxes = document.querySelectorAll(".box");
boxes[0].addEventListener("click", function (evt) {
    changeText(this); getDate(arrival1, 2019-12-01);
});
boxes[1].addEventListener("click", function (evt) {
    changeText(this); getDate(arrival2, 2019-12-02);
});

Let’s now run the JavaScript code through JSLint to help reveal issues.

Unexpected ‘1’ after ‘0’.

That’s because the 2019-12-01 needs to be in quotes.

    // changeText(this); getDate(arrival1, 2019-12-01);
    changeText(this); getDate(arrival1, "2019-12-01");

Unexpected ‘2’ after ‘0’.

Same issue, different line.

    // changeText(this); getDate(arrival2, 2019-12-02);
    changeText(this); getDate(arrival2, "2019-12-02");

Undeclared ‘document’.

We can tell JSLint that we expect this code to be run in a browser, by adding the following declaration to the top of the JavaScript code:

/*jslint browser */

Unexpected ‘this’.

The this keyword can cause confusion. We can get the element from the event target instead.

    const el = evt.target;
    // changeText(this); getDate(arrival1, "2019-12-01");
    changeText(el); getDate(arrival1, "2019-12-01");

Undeclared ‘arrival1’.

That arrival1 variable is supposed to rever to the div with an id attribute of “arrival”. We can use a local variable to store that.

    const arrival = document.querySelector("#arrival1");
    // changeText(el); getDate(arrival1, "2019-12-01");
    changeText(el); getDate(arrival, "2019-12-01");

Unexpected ‘this’.

We then have duplicate issues with box 2, so we can copy/paste the code and change some 1’s to 2’s.

boxes[0].addEventListener("click", function (evt) {
    const el = evt.target;
    // const arrival = document.querySelector("#arrival1");
    const arrival = document.querySelector("#arrival2");
    // changeText(el); getDate(arrival, "2019-12-01");
    changeText(el); getDate(arrival, "2019-12-02");
});

Make similar functions the same

That copy/paste tells me that we can do this in a simpler way. I’ll use an index variable and get the numbers from that instead.

const boxes = document.querySelectorAll(".box");
let index = 0;
boxes[index].addEventListener("click", function (evt) {
    const el = evt.target;
    const day = index + 1;
    // const arrival = document.querySelector("#arrival1");
    const arrival = document.querySelector("#arrival" + day);
    // changeText(el); getDate(arrival, "2019-12-01");
    changeText(el); getDate(arrival, "2019-12-0" + day);
});
index += 1;
boxes[index].addEventListener("click", function (evt) {
    const el = evt.target;
    // const arrival = document.querySelector("#arrival2");
    const arrival = document.querySelector("#arrival" + day);
    // changeText(el); getDate(arrival, "2019-12-02");
    changeText(el); getDate(arrival, "2019-12-0" + day);
});

Turn into a loop

We can now use that function code as we loop through each of the boxes:

const boxes = document.querySelectorAll(".box");
boxes.forEach(function (box, index) {
    box.addEventListener("click", function (evt) {
        // index += 1;
        const el = evt.target;
        const day = index + 1;
        const arrival = document.querySelector("#arrival" + day);
        changeText(el); getDate(arrival, "2019-12-0" + day);
    });
// index += 1;
// boxes[index].addEventListener("click", function (evt) {
//     const el = evt.target;
//     const arrival = document.querySelector("#arrival" + day);
//     changeText(el); getDate(arrival, "2019-12-0" + day);
// });
});

Expected ‘id’ at column 4, not column 2.

There’s more to fix up. First, indenting with multiples of 4 spaces.

function changeText(id) {
    id.classList.toggle("box-color");
}

function getDate(arrival, theDate) {
    document.getElementById("arrival").innerHTML="theDate";
}

Also, the changeText function uses id for a function parameter, but it’s using classList on an element instead. It’s time to rename that function parameter to el.

// function changeText(id) {
function changeText(el) {
    // id.classList.toggle("box-color");
    el.classList.toggle("box-color");
}

Unused ‘arrival’.

The getDate function doesn’t need to get the element, as we’ve already done that in the event function. We can just use the arrival element that we’ve given to the getDate function.

function getDate(arrival, theDate) {
    // document.getElementById("arrival").innerHTML="theDate";
    arrival.innerHTML="theDate";
}

Unused ‘theDate’.

The “theDate” shouldn’t be in quotes.

function getDate(arrival, theDate) {
    // arrival.innerHTML="theDate";
    arrival.innerHTML = theDate;
}

Also, based on the innerHTML, we are not getting the date, we are setting the date instead. We should rename the function so that it is more consistent with what it actually does.

// function getDate(arrival, theDate) {
function setDate(arrival, theDate) {
    arrival.innerHTML="theDate";
}
...
        changeText(el);
        // getDate(arrival, "2019-12-0" + day);
        setDate(arrival, "2019-12-0" + day);

Expected ‘setDate’ at column 8, not column 24.

You really shouldn’t have multiple statements on the same line either, so we’ll put them on separate lines:

        // changeText(el); setDate(arrival, "2019-12-0" + day);
        changeText(el);
        setDate(arrival, "2019-12-0" + day);

Things now work

And the code now finally works.

function changeText(el) {
    el.classList.toggle("box-color");
}

function setDate(arrival, theDate) {
    arrival.innerHTML = theDate;
}

const boxes = document.querySelectorAll(".box");
boxes.forEach(function (box, index) {
    box.addEventListener("click", function (evt) {
        const el = evt.target;
        const day = index + 1;
        const arrival = document.querySelector("#arrival" + day);
        changeText(el);
        setDate(arrival, "2019-12-0" + day);
    });
});

But, does the code do what you want to achieve?

Edit fiddle - JSFiddle - Code Playground

[elfynity1]

Wow everyone, I am so very appreciative of your indepth replies. I am actually so inspired to learn Javascript. I have been meaning to for a while and just working through your examples shows me what I am missing out on!

bwclovis, users should be able to select multiple dates by clicking on the box. They would also need to deselect that date or dates if they changed their mind, and each date needs to be saved to a php variable so that I can pass it through post.

m3g4p0p, thank you so much for your explanation. It is very helpful.

Paul_Watkins, thank you so much! It’s going to take a bit of thought to work through understanding your code. I hope I can eventually figure out how to get it to do exactly the way I need to do it. The user would need to be able to deselect dates and choose from any range / number of dates that would output a variable. I guess it would need to be in a while loop to output the arrival divs, because you don’t know how many dates the user will select.

I am going to set aside the beginning of next year to learn Javascript syntax.

So, I know that using inline javascript is not preferred, but I don’t know how else to get this to work. This is my code so far in the actual calendar.php page (it’s obviously just a snippet from the page). So far, it is working like I need it to. Is it terrible javascript coding or can I get away with this for now?

		<?php } else { ?>
		<td onclick="changeText(this); getDate('selectedDate<?php echo $currentDay; ?>', '<?php echo $date; ?>');" class="<?php echo $today; ?>calendar-price">
			<h5><?php echo $currentDay; ?></h5>
			<p>R<?php echo $_SESSION['pricesend']; ?>
		</td>
	<?php }
	
	//incrementing the counters
	$currentDay++;
	$dayOfWeek++;
	
 } // end function build calendar
?>



<input type="text" id="selectedDate3" class="getDate-not-selected" />
<input type="text" id="selectedDate4" class="getDate-not-selected" />
<input type="text" id="selectedDate5" class="getDate-not-selected" />





<script>
function changeText(id) {
	id.classList.toggle("box-color");
}

function getDate(selectedDate, theDate) {
	var x = document.getElementById(selectedDate);
		if (x.value === "") {
			x.value = theDate;
		} else {
			x.value = "";
		};
		
		
						
	var element = document.getElementById(selectedDate);
	element.classList.toggle("getDate-selected");
}			
</script>

[Paul_Wilkins]

@ elfynity1, we find that PHP code results in too much unknown information,

Instead of the PHP code, can you please supply us with the suitable HTML code that PHP renders?

[bwclovis]

So the best way to do this would to be saving the dates to an array and sending them. I did an update so you can see what that looks like (open the console on code pen). It’s close to what Paul has, I just use es5 and tend to call other functions from my listeners, just a habit I got into.

Paul the fact you broke this down in a linter is outstanding and a great way to see why it wasn’t working, kudos sir.